diff --git a/docs/config.md b/docs/config.md index 61b49f1bb..0b8798bc9 100644 --- a/docs/config.md +++ b/docs/config.md @@ -28,7 +28,7 @@ The explanation and default value of each configuration item are as follows: # It generates a self-sign certificate unless the following x509_key_pair_streaming are both set. enable_tls_streaming = false - # "plugins.cri.x509_key_pair_streaming" constains a x509 valid key pair to stream with tls. + # "plugins.cri.x509_key_pair_streaming" contains a x509 valid key pair to stream with tls. [plugins.cri.x509_key_pair_streaming] # tls_cert_file is the filepath to the certificate paired with the "tls_key_file" tls_cert_file = "" diff --git a/pkg/server/streaming.go b/pkg/server/streaming.go index 8d732eb60..0ed948617 100644 --- a/pkg/server/streaming.go +++ b/pkg/server/streaming.go @@ -34,6 +34,36 @@ import ( ctrdutil "github.com/containerd/cri/pkg/containerd/util" ) +type streamListenerMode int + +const ( + x509KeyPairTLS streamListenerMode = iota + selfSignTLS + withoutTLS +) + +func getStreamListenerMode(c *criService) (streamListenerMode, error) { + if c.config.EnableTLSStreaming { + if c.config.X509KeyPairStreaming.TLSCertFile != "" && c.config.X509KeyPairStreaming.TLSKeyFile != "" { + return x509KeyPairTLS, nil + } + if c.config.X509KeyPairStreaming.TLSCertFile != "" && c.config.X509KeyPairStreaming.TLSKeyFile == "" { + return -1, errors.New("must set X509KeyPairStreaming.TLSKeyFile") + } + if c.config.X509KeyPairStreaming.TLSCertFile == "" && c.config.X509KeyPairStreaming.TLSKeyFile != "" { + return -1, errors.New("must set X509KeyPairStreaming.TLSCertFile") + } + return selfSignTLS, nil + } + if c.config.X509KeyPairStreaming.TLSCertFile != "" { + return -1, errors.New("X509KeyPairStreaming.TLSCertFile is set but EnableTLSStreaming is not set") + } + if c.config.X509KeyPairStreaming.TLSKeyFile != "" { + return -1, errors.New("X509KeyPairStreaming.TLSKeyFile is set but EnableTLSStreaming is not set") + } + return withoutTLS, nil +} + func newStreamServer(c *criService, addr, port string) (streaming.Server, error) { if addr == "" { a, err := k8snet.ChooseBindAddress(nil) @@ -45,13 +75,12 @@ func newStreamServer(c *criService, addr, port string) (streaming.Server, error) config := streaming.DefaultConfig config.Addr = net.JoinHostPort(addr, port) run := newStreamRuntime(c) - if !c.config.EnableTLSStreaming { - if c.config.X509KeyPairStreaming.TLSCertFile != "" || c.config.X509KeyPairStreaming.TLSKeyFile != "" { - return nil, errors.Errorf("X509KeyPairStreaming.TLSCertFile and/or X509KeyPairStreaming.TLSKeyFile are set but EnableTLSStreaming is not set") - } - return streaming.NewServer(config, run) + tlsMode, err := getStreamListenerMode(c) + if err != nil { + return nil, errors.Wrapf(err, "invalid stream server configuration") } - if c.config.X509KeyPairStreaming.TLSCertFile != "" && c.config.X509KeyPairStreaming.TLSKeyFile != "" { + switch tlsMode { + case x509KeyPairTLS: tlsCert, err := tls.LoadX509KeyPair(c.config.X509KeyPairStreaming.TLSCertFile, c.config.X509KeyPairStreaming.TLSKeyFile) if err != nil { return nil, errors.Wrap(err, "failed to load x509 key pair for stream server") @@ -60,19 +89,21 @@ func newStreamServer(c *criService, addr, port string) (streaming.Server, error) Certificates: []tls.Certificate{tlsCert}, } return streaming.NewServer(config, run) - } else if c.config.X509KeyPairStreaming.TLSCertFile != "" || c.config.X509KeyPairStreaming.TLSKeyFile != "" { - return nil, errors.Errorf("must set both X509KeyPairStreaming.TLSCertFile and X509KeyPairStreaming.TLSKeyFile") + case selfSignTLS: + tlsCert, err := newTLSCert() + if err != nil { + return nil, errors.Wrap(err, "failed to generate tls certificate for stream server") + } + config.TLSConfig = &tls.Config{ + Certificates: []tls.Certificate{tlsCert}, + InsecureSkipVerify: true, + } + return streaming.NewServer(config, run) + case withoutTLS: + return streaming.NewServer(config, run) + default: + return nil, errors.New("invalid configuration for the stream listener") } - // generating self-sign certs - tlsCert, err := newTLSCert() - if err != nil { - return nil, errors.Wrap(err, "failed to generate tls certificate for stream server") - } - config.TLSConfig = &tls.Config{ - Certificates: []tls.Certificate{tlsCert}, - InsecureSkipVerify: true, - } - return streaming.NewServer(config, run) } type streamRuntime struct { diff --git a/pkg/server/streaming_test.go b/pkg/server/streaming_test.go new file mode 100644 index 000000000..a88903d3b --- /dev/null +++ b/pkg/server/streaming_test.go @@ -0,0 +1,153 @@ +/* +Copyright 2017 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package server + +import ( + "testing" + + "github.com/containerd/cri/pkg/config" + "github.com/stretchr/testify/assert" +) + +func TestValidateStreamServer(t *testing.T) { + for desc, test := range map[string]struct { + *criService + tlsMode streamListenerMode + expectErr bool + }{ + "should pass with default withoutTLS": { + criService: &criService{ + config: config.Config{ + PluginConfig: config.DefaultConfig(), + }, + }, + tlsMode: withoutTLS, + expectErr: false, + }, + "should pass with x509KeyPairTLS": { + criService: &criService{ + config: config.Config{ + PluginConfig: config.PluginConfig{ + EnableTLSStreaming: true, + X509KeyPairStreaming: config.X509KeyPairStreaming{ + TLSKeyFile: "non-empty", + TLSCertFile: "non-empty", + }, + }, + }, + }, + tlsMode: x509KeyPairTLS, + expectErr: false, + }, + "should pass with selfSign": { + criService: &criService{ + config: config.Config{ + PluginConfig: config.PluginConfig{ + EnableTLSStreaming: true, + }, + }, + }, + tlsMode: selfSignTLS, + expectErr: false, + }, + "should return error with X509 keypair but not EnableTLSStreaming": { + criService: &criService{ + config: config.Config{ + PluginConfig: config.PluginConfig{ + EnableTLSStreaming: false, + X509KeyPairStreaming: config.X509KeyPairStreaming{ + TLSKeyFile: "non-empty", + TLSCertFile: "non-empty", + }, + }, + }, + }, + tlsMode: -1, + expectErr: true, + }, + "should return error with X509 TLSCertFile empty": { + criService: &criService{ + config: config.Config{ + PluginConfig: config.PluginConfig{ + EnableTLSStreaming: true, + X509KeyPairStreaming: config.X509KeyPairStreaming{ + TLSKeyFile: "non-empty", + TLSCertFile: "", + }, + }, + }, + }, + tlsMode: -1, + expectErr: true, + }, + "should return error with X509 TLSKeyFile empty": { + criService: &criService{ + config: config.Config{ + PluginConfig: config.PluginConfig{ + EnableTLSStreaming: true, + X509KeyPairStreaming: config.X509KeyPairStreaming{ + TLSKeyFile: "", + TLSCertFile: "non-empty", + }, + }, + }, + }, + tlsMode: -1, + expectErr: true, + }, + "should return error without EnableTLSStreaming and only TLSCertFile set": { + criService: &criService{ + config: config.Config{ + PluginConfig: config.PluginConfig{ + EnableTLSStreaming: false, + X509KeyPairStreaming: config.X509KeyPairStreaming{ + TLSKeyFile: "", + TLSCertFile: "non-empty", + }, + }, + }, + }, + tlsMode: -1, + expectErr: true, + }, + "should return error without EnableTLSStreaming and only TLSKeyFile set": { + criService: &criService{ + config: config.Config{ + PluginConfig: config.PluginConfig{ + EnableTLSStreaming: false, + X509KeyPairStreaming: config.X509KeyPairStreaming{ + TLSKeyFile: "non-empty", + TLSCertFile: "", + }, + }, + }, + }, + tlsMode: -1, + expectErr: true, + }, + } { + t.Run(desc, func(t *testing.T) { + tlsMode, err := getStreamListenerMode(test.criService) + if test.expectErr { + assert.Error(t, err) + return + } + assert.NoError(t, err) + assert.Equal(t, test.tlsMode, tlsMode) + }) + } +}