From 6ebf697b285379a480295c293aca19078ba8f75d Mon Sep 17 00:00:00 2001 From: Alay Patel Date: Mon, 8 Nov 2021 16:41:14 -0500 Subject: [PATCH 1/9] fix godoc for endpoint package Signed-off-by: Alay Patel --- endpoint/endpoint.go | 3 ++- endpoint/loadbalancer/loadbalancer.go | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/endpoint/endpoint.go b/endpoint/endpoint.go index d66c88b..27eca75 100644 --- a/endpoint/endpoint.go +++ b/endpoint/endpoint.go @@ -19,6 +19,7 @@ type Endpoint interface { IngressPort() int32 // IsHealthy returns whether or not all Kube resources used by endpoint are healthy IsHealthy(ctx context.Context, c client.Client) (bool, error) - // MarkForCleanup + // MarkForCleanup adds a label to all the resources created for the endpoint + // Callers are expected to not overwrite MarkForCleanup(ctx context.Context, c client.Client, key, value string) error } diff --git a/endpoint/loadbalancer/loadbalancer.go b/endpoint/loadbalancer/loadbalancer.go index e6d2708..22dabdb 100644 --- a/endpoint/loadbalancer/loadbalancer.go +++ b/endpoint/loadbalancer/loadbalancer.go @@ -28,7 +28,7 @@ type loadBalancer struct { } // AddToScheme should be used as soon as scheme is created to add -// route objects for encoding/decoding +// core objects for encoding/decoding func AddToScheme(scheme *runtime.Scheme) error { return corev1.AddToScheme(scheme) } From 20dc9af482c669744d2fdf99c63a85d98c072421 Mon Sep 17 00:00:00 2001 From: Alay Patel Date: Mon, 8 Nov 2021 16:45:59 -0500 Subject: [PATCH 2/9] add transport interface and stunnel-server implementation Signed-off-by: Alay Patel --- transport/stunnel/server.go | 324 +++++++++++++++++++++++++++++++++++ transport/stunnel/stunnel.go | 22 +++ transport/transport.go | 113 ++++++++++++ 3 files changed, 459 insertions(+) create mode 100644 transport/stunnel/server.go create mode 100644 transport/stunnel/stunnel.go create mode 100644 transport/transport.go diff --git a/transport/stunnel/server.go b/transport/stunnel/server.go new file mode 100644 index 0000000..b0bdf14 --- /dev/null +++ b/transport/stunnel/server.go @@ -0,0 +1,324 @@ +package stunnel + +import ( + "bytes" + "context" + "strconv" + "text/template" + + "github.com/backube/pvc-transfer/endpoint" + "github.com/backube/pvc-transfer/internal/utils" + "github.com/backube/pvc-transfer/transport" + "github.com/go-logr/logr" + corev1 "k8s.io/api/core/v1" + k8serrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + ctrlclient "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" +) + +const ( + stunnelServerConfTemplate = `foreground = yes +pid = +socket = l:TCP_NODELAY=1 +socket = r:TCP_NODELAY=1 +debug = 7 +sslVersion = TLSv1.2 +[rsync] +accept = {{ $.acceptPort }} +connect = {{ $.connectPort }} +key = /etc/stunnel/certs/tls.key +cert = /etc/stunnel/certs/tls.crt +TIMEOUTclose = 0 +` + stunnelConnectPort = 8080 +) + +// AddToScheme should be used as soon as scheme is created to add +// core objects for encoding/decoding +func AddToScheme(scheme *runtime.Scheme) error { + return corev1.AddToScheme(scheme) +} + +// APIsToWatch give a list of APIs to watch if using this package +// to deploy the transport +func APIsToWatch() ([]ctrlclient.Object, error) { + return []ctrlclient.Object{&corev1.Secret{}, &corev1.ConfigMap{}}, nil +} + +type server struct { + logger logr.Logger + listenPort int32 + connectPort int32 + containers []corev1.Container + volumes []corev1.Volume + options *transport.Options + namespacedName types.NamespacedName +} + +func New(ctx context.Context, c ctrlclient.Client, logger logr.Logger, + namespacedName types.NamespacedName, + e endpoint.Endpoint, + options *transport.Options) (transport.Transport, error) { + transportLogger := logger.WithValues("transportServer", namespacedName) + transferPort := e.BackendPort() + + s := &server{ + namespacedName: namespacedName, + options: options, + listenPort: transferPort, + connectPort: stunnelConnectPort, + logger: transportLogger, + } + + err := s.reconcileConfig(ctx, c) + if err != nil { + s.logger.Error(err, "unable to reconcile stunnel server config") + return nil, err + } + + err = s.reconcileSecret(ctx, c) + if err != nil { + s.logger.Error(err, "unable to create stunnel server secret") + return nil, err + } + + s.volumes = s.serverVolumes() + s.containers = s.serverContainers() + + return s, nil +} + +func (s *server) NamespacedName() types.NamespacedName { + return s.namespacedName +} + +func (s *server) ListenPort() int32 { + return s.listenPort +} + +func (s *server) ConnectPort() int32 { + return s.connectPort +} + +func (s *server) Containers() []corev1.Container { + return s.containers +} + +func (s *server) Volumes() []corev1.Volume { + return s.volumes +} + +func (s *server) Type() transport.Type { + return TransportTypeStunnel +} + +func (s *server) Credentials() types.NamespacedName { + return types.NamespacedName{Name: s.prefixedName(stunnelSecret), Namespace: s.NamespacedName().Namespace} +} + +func (s *server) Hostname() string { + return "localhost" +} + +func (s *server) MarkForCleanup(ctx context.Context, c ctrlclient.Client, key, value string) error { + cm := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: s.prefixedName(stunnelConfig), + Namespace: s.NamespacedName().Namespace, + }, + } + err := utils.UpdateWithLabel(ctx, c, cm, key, value) + if err != nil { + return err + } + + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: s.prefixedName(stunnelSecret), + Namespace: s.NamespacedName().Namespace, + }, + } + return utils.UpdateWithLabel(context.TODO(), c, secret, key, value) +} + +func (s *server) reconcileConfig(ctx context.Context, c ctrlclient.Client) error { + ports := map[string]string{ + // acceptPort on which Stunnel service listens on, must connect with endpoint + "acceptPort": strconv.Itoa(int(s.ListenPort())), + // connectPort in the container on which Transfer is listening on + "connectPort": strconv.Itoa(int(s.ConnectPort())), + } + + var stunnelConf bytes.Buffer + stunnelConfTemplate, err := template.New("config").Parse(stunnelServerConfTemplate) + if err != nil { + s.logger.Error(err, "unable to parse stunnel config template") + return err + } + + err = stunnelConfTemplate.Execute(&stunnelConf, ports) + if err != nil { + s.logger.Error(err, "unable to execute stunnel config template") + return err + } + + stunnelConfigMap := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: s.NamespacedName().Namespace, + Name: s.prefixedName(stunnelConfig), + }, + } + + _, err = controllerutil.CreateOrUpdate(ctx, c, stunnelConfigMap, func() error { + stunnelConfigMap.Labels = s.options.Labels + stunnelConfigMap.OwnerReferences = s.options.Owners + + stunnelConfigMap.Data = map[string]string{ + "stunnel.conf": stunnelConf.String(), + } + return nil + }) + return err +} + +func (s *server) prefixedName(name string) string { + return s.namespacedName.Name + "-server-" + name +} + +func (s *server) reconcileSecret(ctx context.Context, c ctrlclient.Client) error { + _, _, found, err := s.getExistingCert(ctx, c) + if found { + return nil + } + + if err != nil { + s.logger.Error(err, "error getting existing ssl certs from secret") + return err + } + + _, newCrt, newKey, err := transport.GenerateSSLCert() + if err != nil { + s.logger.Error(err, "error generating ssl certs for stunnel server") + return err + } + + stunnelSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: s.NamespacedName().Namespace, + Name: s.prefixedName(stunnelSecret), + }, + } + + _, err = controllerutil.CreateOrUpdate(ctx, c, stunnelSecret, func() error { + stunnelSecret.Labels = s.options.Labels + stunnelSecret.OwnerReferences = s.options.Owners + + stunnelSecret.Data = map[string][]byte{ + "tls.crt": newCrt.Bytes(), + "tls.key": newKey.Bytes(), + } + return nil + }) + return err +} + +func (s *server) getExistingCert(ctx context.Context, c ctrlclient.Client) (*bytes.Buffer, *bytes.Buffer, bool, error) { + serverSecret := &corev1.Secret{} + err := c.Get(ctx, types.NamespacedName{ + Namespace: s.namespacedName.Namespace, + Name: s.prefixedName(stunnelSecret), + }, serverSecret) + switch { + case k8serrors.IsNotFound(err): + return nil, nil, false, nil + case err != nil: + return nil, nil, false, err + } + + key, ok := serverSecret.Data["tls.key"] + if !ok { + s.logger.Info("stunnel server secret data missing key tls.key", "secret", types.NamespacedName{ + Namespace: s.namespacedName.Namespace, + Name: s.prefixedName(stunnelSecret), + }) + return nil, nil, false, nil + } + + crt, ok := serverSecret.Data["tls.crt"] + if !ok { + s.logger.Info("stunnel server secret data missing key tls.crt", "secret", types.NamespacedName{ + Namespace: s.namespacedName.Namespace, + Name: s.prefixedName(stunnelSecret), + }) + return nil, nil, false, nil + } + + return bytes.NewBuffer(key), bytes.NewBuffer(crt), false, nil +} + +func (s *server) serverContainers() []corev1.Container { + return []corev1.Container{ + { + Name: Container, + Image: getImage(s.options), + Command: []string{ + "/bin/stunnel", + "/etc/stunnel/stunnel.conf", + }, + Ports: []corev1.ContainerPort{ + { + Name: "stunnel", + Protocol: corev1.ProtocolTCP, + ContainerPort: s.ListenPort(), + }, + }, + VolumeMounts: []corev1.VolumeMount{ + { + Name: s.prefixedName(stunnelConfig), + MountPath: "/etc/stunnel/stunnel.conf", + SubPath: "stunnel.conf", + }, + { + Name: s.prefixedName(stunnelSecret), + MountPath: "/etc/stunnel/certs", + }, + }, + }, + } +} + +func (s *server) serverVolumes() []corev1.Volume { + return []corev1.Volume{ + { + Name: s.prefixedName(stunnelConfig), + VolumeSource: corev1.VolumeSource{ + ConfigMap: &corev1.ConfigMapVolumeSource{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: s.prefixedName(stunnelConfig), + }, + }, + }, + }, + { + Name: s.prefixedName(stunnelSecret), + VolumeSource: corev1.VolumeSource{ + Secret: &corev1.SecretVolumeSource{ + SecretName: s.prefixedName(stunnelSecret), + Items: []corev1.KeyToPath{ + { + Key: "tls.crt", + Path: "tls.crt", + }, + { + Key: "tls.key", + Path: "tls.key", + }, + }, + }, + }, + }, + } +} diff --git a/transport/stunnel/stunnel.go b/transport/stunnel/stunnel.go new file mode 100644 index 0000000..7696c8a --- /dev/null +++ b/transport/stunnel/stunnel.go @@ -0,0 +1,22 @@ +package stunnel + +import "github.com/backube/pvc-transfer/transport" + +const ( + defaultStunnelImage = "quay.io/konveyor/rsync-transfer:latest" + stunnelConfig = "stunnel-config" + stunnelSecret = "stunnel-credentials" +) + +const ( + TransportTypeStunnel = "stunnel" + Container = "stunnel" +) + +func getImage(options *transport.Options) string { + if options.Image == "" { + return defaultStunnelImage + } else { + return options.Image + } +} diff --git a/transport/transport.go b/transport/transport.go new file mode 100644 index 0000000..d1a65ed --- /dev/null +++ b/transport/transport.go @@ -0,0 +1,113 @@ +package transport + +import ( + "bytes" + "context" + "crypto/rand" + "crypto/rsa" + "crypto/x509" + "crypto/x509/pkix" + "encoding/pem" + "math/big" + "time" + + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +type Transport interface { + // NamespacedName returns the namespaced name to identify this transport Transport + NamespacedName() types.NamespacedName + // ListenPort returns a port on which the transport server listens for incomming connections + ListenPort() int32 + // ConnectPort returns the port to connect to transfer server + // Using this the server acts as a client to the transfer relaying all the data + // sent from the transport client + ConnectPort() int32 + // Containers returns a list of containers transfers can add to their server Pods + Containers() []corev1.Container + // Volumes returns a list of volumes transfers have add to their server Pods for getting the configurations + // mounted for the transport containers to work + Volumes() []corev1.Volume + // Type + Type() Type + // Credentials returns the namespaced name of the secret holding credentials for talking to the server + Credentials() types.NamespacedName + // Hostname returns the string to which the transfer will connect to + // in case of a null transport, it will simple relay the endpoint hostname + // in case of a valid transport, it will have a custom hostname where transfers will have to connect to. + Hostname() string + // MarkForCleanup adds a label to all the resources created for the endpoint + // Callers are expected to not overwrite + MarkForCleanup(ctx context.Context, c client.Client, key, value string) error +} + +type Options struct { + Labels map[string]string + Owners []metav1.OwnerReference + Image string + + ProxyURL string + ProxyUsername string + ProxyPassword string + NoVerifyCA bool + CAVerifyLevel string +} + +type Type string + +func GenerateSSLCert() (*bytes.Buffer, *bytes.Buffer, *bytes.Buffer, error) { + caPrivKey, err := rsa.GenerateKey(rand.Reader, 4096) + if err != nil { + return nil, nil, nil, err + } + + subj := pkix.Name{ + CommonName: "backube.dev", + Country: []string{"US"}, + Organization: []string{"Backube"}, + OrganizationalUnit: []string{"Engineering"}, + } + + certTemp := x509.Certificate{ + SerialNumber: big.NewInt(2020), + Subject: subj, + NotBefore: time.Now(), + NotAfter: time.Now().AddDate(10, 0, 0), + IsCA: true, + ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageClientAuth, x509.ExtKeyUsageServerAuth}, + KeyUsage: x509.KeyUsageDigitalSignature | x509.KeyUsageCertSign, + BasicConstraintsValid: true, + } + + caBytes, err := x509.CreateCertificate( + rand.Reader, + &certTemp, + &certTemp, + &caPrivKey.PublicKey, + caPrivKey, + ) + if err != nil { + return nil, nil, nil, err + } + crt := new(bytes.Buffer) + err = pem.Encode(crt, &pem.Block{ + Type: "CERTIFICATE", + Bytes: caBytes, + }) + if err != nil { + return nil, nil, nil, err + } + key := new(bytes.Buffer) + err = pem.Encode(key, &pem.Block{ + Type: "RSA PRIVATE KEY", + Bytes: x509.MarshalPKCS1PrivateKey(caPrivKey), + }) + if err != nil { + return nil, nil, nil, err + } + + return crt, crt, key, nil +} From a1e99e1bf52272ed060d8bf085ee8b746d6b70f3 Mon Sep 17 00:00:00 2001 From: Alay Patel Date: Mon, 8 Nov 2021 17:18:12 -0500 Subject: [PATCH 3/9] add unit tests for transport server Signed-off-by: Alay Patel --- transport/stunnel/server.go | 4 +- transport/stunnel/server_test.go | 403 +++++++++++++++++++++++++++++++ 2 files changed, 405 insertions(+), 2 deletions(-) create mode 100644 transport/stunnel/server_test.go diff --git a/transport/stunnel/server.go b/transport/stunnel/server.go index b0bdf14..b3ef2c9 100644 --- a/transport/stunnel/server.go +++ b/transport/stunnel/server.go @@ -81,7 +81,7 @@ func New(ctx context.Context, c ctrlclient.Client, logger logr.Logger, err = s.reconcileSecret(ctx, c) if err != nil { - s.logger.Error(err, "unable to create stunnel server secret") + s.logger.Error(err, "unable to reconcile stunnel server secret") return nil, err } @@ -256,7 +256,7 @@ func (s *server) getExistingCert(ctx context.Context, c ctrlclient.Client) (*byt return nil, nil, false, nil } - return bytes.NewBuffer(key), bytes.NewBuffer(crt), false, nil + return bytes.NewBuffer(key), bytes.NewBuffer(crt), true, nil } func (s *server) serverContainers() []corev1.Container { diff --git a/transport/stunnel/server_test.go b/transport/stunnel/server_test.go new file mode 100644 index 0000000..7d8f022 --- /dev/null +++ b/transport/stunnel/server_test.go @@ -0,0 +1,403 @@ +package stunnel + +import ( + "context" + "fmt" + "reflect" + "strings" + "testing" + + "github.com/backube/pvc-transfer/endpoint" + "github.com/backube/pvc-transfer/transport" + logrtesting "github.com/go-logr/logr/testing" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + "k8s.io/utils/pointer" + ctrlclient "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" +) + +func fakeClientWithObjects(objs ...ctrlclient.Object) ctrlclient.WithWatch { + scheme := runtime.NewScheme() + AddToScheme(scheme) + return fake.NewClientBuilder().WithScheme(scheme).WithObjects(objs...).Build() +} + +func testOwnerReferences() []metav1.OwnerReference { + return []metav1.OwnerReference{{ + APIVersion: "api.foo", + Kind: "Test", + Name: "bar", + UID: "123", + Controller: pointer.Bool(true), + BlockOwnerDeletion: pointer.Bool(true), + }} +} + +type fakeEndpoint struct { + nn types.NamespacedName + hostname string +} + +func (f fakeEndpoint) NamespacedName() types.NamespacedName { + return f.nn +} + +func (f fakeEndpoint) Hostname() string { + return f.hostname +} + +func (f fakeEndpoint) BackendPort() int32 { + return 1234 +} + +func (f fakeEndpoint) IngressPort() int32 { + return 1234 +} + +func (f fakeEndpoint) IsHealthy(ctx context.Context, c ctrlclient.Client) (bool, error) { + return true, nil +} + +func (f fakeEndpoint) MarkForCleanup(ctx context.Context, c ctrlclient.Client, key, value string) error { + return nil +} + +func newFakeEndpoint() endpoint.Endpoint { + return fakeEndpoint{ + nn: types.NamespacedName{Name: "foo", Namespace: "bar"}, + hostname: "foo.bar", + } +} + +func TestNew(t *testing.T) { + tests := []struct { + name string + namespacedName types.NamespacedName + endpoint endpoint.Endpoint + labels map[string]string + ownerReferences []metav1.OwnerReference + wantErr bool + objects []ctrlclient.Object + }{ + { + name: "test with no stunnel server objects", + namespacedName: types.NamespacedName{Namespace: "bar", Name: "foo"}, + endpoint: newFakeEndpoint(), + labels: map[string]string{"test": "me"}, + ownerReferences: testOwnerReferences(), + wantErr: false, + objects: []ctrlclient.Object{}, + }, + { + name: "test stunnel server, valid secret exists but no configmap", + namespacedName: types.NamespacedName{Namespace: "bar", Name: "foo"}, + endpoint: newFakeEndpoint(), + labels: map[string]string{"test": "me"}, + ownerReferences: testOwnerReferences(), + wantErr: false, + objects: []ctrlclient.Object{ + &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo-server-stunnel-credentials", + Namespace: "bar", + }, + Data: map[string][]byte{"tls.key": []byte(`key`), "tls.crt": []byte(`crt`)}, + }, + }, + }, + { + name: "test stunnel server, invalid secret exists but no configmap", + namespacedName: types.NamespacedName{Namespace: "bar", Name: "foo"}, + endpoint: newFakeEndpoint(), + labels: map[string]string{"test": "me"}, + ownerReferences: testOwnerReferences(), + wantErr: false, + objects: []ctrlclient.Object{ + &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo-server-stunnel-credentials", + Namespace: "bar", + }, + Data: map[string][]byte{"tls.crt": []byte(`crt`)}, + }, + }, + }, + { + name: "test stunnel server, valid configmap but no secret", + namespacedName: types.NamespacedName{Namespace: "bar", Name: "foo"}, + endpoint: newFakeEndpoint(), + labels: map[string]string{"test": "me"}, + ownerReferences: testOwnerReferences(), + wantErr: false, + objects: []ctrlclient.Object{ + &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo-server-stunnel-config", + Namespace: "bar", + }, + Data: map[string]string{"stunnel.conf": "foreground = yes"}, + }, + }, + }, + { + name: "test stunnel server, invalid configmap but no secret", + namespacedName: types.NamespacedName{Namespace: "bar", Name: "foo"}, + endpoint: newFakeEndpoint(), + labels: map[string]string{"test": "me"}, + ownerReferences: testOwnerReferences(), + wantErr: false, + objects: []ctrlclient.Object{ + &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo-server-stunnel-config", + Namespace: "bar", + }, + Data: map[string]string{"stunnel.conf": "foreground = no"}, + }, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + fakeClient := fakeClientWithObjects(tt.objects...) + ctx := context.WithValue(context.Background(), "test", tt.name) + fakeLogger := logrtesting.TestLogger{t} + stunnelServer, err := New(ctx, fakeClient, fakeLogger, tt.namespacedName, tt.endpoint, &transport.Options{Labels: tt.labels, Owners: tt.ownerReferences}) + if (err != nil) != tt.wantErr { + t.Errorf("New() error = %v, wantErr %v", err, tt.wantErr) + return + } + cm := &corev1.ConfigMap{} + err = fakeClient.Get(context.Background(), types.NamespacedName{ + Namespace: "bar", + Name: "foo-server-" + stunnelConfig, + }, cm) + if err != nil { + panic(fmt.Errorf("%#v should not be getting error from fake client", err)) + } + + configdata, ok := cm.Data["stunnel.conf"] + if !ok { + t.Error("unable to find stunnel config data in configmap") + } + if !strings.Contains(configdata, "foreground = yes") { + t.Error("configmap data does not contain the right data") + } + + secret := &corev1.Secret{} + err = fakeClient.Get(context.Background(), types.NamespacedName{ + Namespace: "bar", + Name: "foo-server-" + stunnelSecret, + }, secret) + if err != nil { + panic(fmt.Errorf("%#v should not be getting error from fake client", err)) + } + + _, ok = secret.Data["tls.key"] + if !ok { + t.Error("unable to find tls.key in stunnel secret") + } + + _, ok = secret.Data["tls.crt"] + if !ok { + t.Error("unable to find tls.crt in stunnel secret") + } + + if len(stunnelServer.Volumes()) == 0 { + t.Error("stunnel server volumes not set properly") + } + + if len(stunnelServer.Containers()) == 0 { + t.Error("stunnel server containers not set properly") + } + }) + } +} + +func Test_server_MarkForCleanup(t *testing.T) { + tests := []struct { + name string + namespacedName types.NamespacedName + labels map[string]string + wantErr bool + key string + value string + objects []ctrlclient.Object + }{ + { + name: "test with configmap and secret objects", + namespacedName: types.NamespacedName{Namespace: "bar", Name: "foo"}, + labels: map[string]string{"test": "me"}, + wantErr: false, + key: "cleanup-key", + value: "cleanup-value", + objects: []ctrlclient.Object{ + &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo-server-stunnel-credentials", + Namespace: "bar", + Labels: map[string]string{"test": "me"}, + }, + Data: map[string][]byte{"tls.key": []byte(`key`), "tls.crt": []byte(`crt`)}, + }, + &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo-server-stunnel-config", + Namespace: "bar", + Labels: map[string]string{"test": "me"}, + }, + Data: map[string]string{"stunnel.conf": "foreground = yes"}, + }, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + s := &server{ + logger: logrtesting.TestLogger{t}, + options: &transport.Options{ + Labels: tt.labels, + Owners: testOwnerReferences(), + }, + namespacedName: tt.namespacedName, + } + ctx := context.WithValue(context.Background(), "test", tt.name) + fakeClient := fakeClientWithObjects(tt.objects...) + if err := s.MarkForCleanup(ctx, fakeClient, tt.key, tt.value); (err != nil) != tt.wantErr { + t.Errorf("MarkForCleanup() error = %v, wantErr %v", err, tt.wantErr) + } + + cm := &corev1.ConfigMap{} + err := fakeClient.Get(context.Background(), types.NamespacedName{ + Namespace: "bar", + Name: "foo-server-" + stunnelConfig, + }, cm) + if err != nil { + panic(fmt.Errorf("%#v should not be getting error from fake client", err)) + } + + tt.labels[tt.key] = tt.value + if !reflect.DeepEqual(tt.labels, cm.Labels) { + t.Errorf("labels on configmap = %#v, wanted %#v", cm.Labels, tt.labels) + } + + secret := &corev1.Secret{} + err = fakeClient.Get(context.Background(), types.NamespacedName{ + Namespace: "bar", + Name: "foo-server-" + stunnelSecret, + }, secret) + if err != nil { + panic(fmt.Errorf("%#v should not be getting error from fake client", err)) + } + + if !reflect.DeepEqual(tt.labels, secret.Labels) { + t.Errorf("labels on secret = %#v, wanted %#v", secret.Labels, tt.labels) + } + }) + } +} + +func Test_server_getExistingCert(t *testing.T) { + tests := []struct { + name string + namespacedName types.NamespacedName + labels map[string]string + wantErr bool + wantFound bool + objects []ctrlclient.Object + }{ + { + name: "test with no secret", + namespacedName: types.NamespacedName{Namespace: "bar", Name: "foo"}, + labels: map[string]string{"test": "me"}, + wantErr: false, + wantFound: false, + objects: []ctrlclient.Object{}, + }, + { + name: "test with invalid secret, key missing", + namespacedName: types.NamespacedName{Namespace: "bar", Name: "foo"}, + labels: map[string]string{"test": "me"}, + wantErr: false, + wantFound: false, + objects: []ctrlclient.Object{ + &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo-server-stunnel-credentials", + Namespace: "bar", + Labels: map[string]string{"test": "me"}, + }, + Data: map[string][]byte{"tls.crt": []byte(`crt`)}, + }, + }, + }, + { + name: "test with invalid secret, crt missing", + namespacedName: types.NamespacedName{Namespace: "bar", Name: "foo"}, + labels: map[string]string{"test": "me"}, + wantErr: false, + wantFound: false, + objects: []ctrlclient.Object{ + &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo-server-stunnel-credentials", + Namespace: "bar", + Labels: map[string]string{"test": "me"}, + }, + Data: map[string][]byte{"tls.key": []byte(`key`)}, + }, + }, + }, + { + name: "test with valid secret", + namespacedName: types.NamespacedName{Namespace: "bar", Name: "foo"}, + labels: map[string]string{"test": "me"}, + wantErr: false, + wantFound: true, + objects: []ctrlclient.Object{ + &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo-server-stunnel-credentials", + Namespace: "bar", + Labels: map[string]string{"test": "me"}, + }, + Data: map[string][]byte{"tls.key": []byte(`key`), "tls.crt": []byte(`crt`)}, + }, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + s := &server{ + logger: logrtesting.TestLogger{t}, + namespacedName: tt.namespacedName, + options: &transport.Options{ + Labels: tt.labels, + Owners: testOwnerReferences(), + }, + } + ctx := context.WithValue(context.Background(), "test", tt.name) + key, crt, found, err := s.getExistingCert(ctx, fakeClientWithObjects(tt.objects...)) + if err != nil { + t.Error("found unexpected error", err) + } + if !tt.wantFound && found { + t.Error("found unexpected secret") + } + if tt.wantFound && !found { + t.Error("not found unexpected") + } + + if tt.wantFound && found && key == nil { + t.Error("secret found but empty key, unexpected") + } + + if tt.wantFound && found && crt == nil { + t.Error("secret found but empty crt, unexpected") + } + }) + } +} From 8505c125ef4cd72ce341c623bc7a95c30b320b2f Mon Sep 17 00:00:00 2001 From: Alay Patel Date: Tue, 9 Nov 2021 14:12:45 -0500 Subject: [PATCH 4/9] refactor the functions that can be shared by client and server Signed-off-by: Alay Patel --- transport/stunnel/server.go | 62 ++++------------ transport/stunnel/server_test.go | 108 +--------------------------- transport/stunnel/stunnel.go | 50 ++++++++++++- transport/stunnel/stunnel_test.go | 115 ++++++++++++++++++++++++++++++ 4 files changed, 182 insertions(+), 153 deletions(-) create mode 100644 transport/stunnel/stunnel_test.go diff --git a/transport/stunnel/server.go b/transport/stunnel/server.go index b3ef2c9..8577179 100644 --- a/transport/stunnel/server.go +++ b/transport/stunnel/server.go @@ -11,7 +11,6 @@ import ( "github.com/backube/pvc-transfer/transport" "github.com/go-logr/logr" corev1 "k8s.io/api/core/v1" - k8serrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" @@ -58,7 +57,7 @@ type server struct { namespacedName types.NamespacedName } -func New(ctx context.Context, c ctrlclient.Client, logger logr.Logger, +func NewServer(ctx context.Context, c ctrlclient.Client, logger logr.Logger, namespacedName types.NamespacedName, e endpoint.Endpoint, options *transport.Options) (transport.Transport, error) { @@ -145,23 +144,22 @@ func (s *server) MarkForCleanup(ctx context.Context, c ctrlclient.Client, key, v } func (s *server) reconcileConfig(ctx context.Context, c ctrlclient.Client) error { + stunnelConfTemplate, err := template.New("config").Parse(stunnelServerConfTemplate) + if err != nil { + s.logger.Error(err, "unable to parse stunnel server config template") + return err + } + ports := map[string]string{ // acceptPort on which Stunnel service listens on, must connect with endpoint "acceptPort": strconv.Itoa(int(s.ListenPort())), // connectPort in the container on which Transfer is listening on "connectPort": strconv.Itoa(int(s.ConnectPort())), } - var stunnelConf bytes.Buffer - stunnelConfTemplate, err := template.New("config").Parse(stunnelServerConfTemplate) - if err != nil { - s.logger.Error(err, "unable to parse stunnel config template") - return err - } - err = stunnelConfTemplate.Execute(&stunnelConf, ports) if err != nil { - s.logger.Error(err, "unable to execute stunnel config template") + s.logger.Error(err, "unable to execute stunnel server config template") return err } @@ -189,7 +187,7 @@ func (s *server) prefixedName(name string) string { } func (s *server) reconcileSecret(ctx context.Context, c ctrlclient.Client) error { - _, _, found, err := s.getExistingCert(ctx, c) + _, _, found, err := getExistingCert(ctx, c, s.logger, s.namespacedName, s.secretNameSuffix()) if found { return nil } @@ -208,7 +206,7 @@ func (s *server) reconcileSecret(ctx context.Context, c ctrlclient.Client) error stunnelSecret := &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ Namespace: s.NamespacedName().Namespace, - Name: s.prefixedName(stunnelSecret), + Name: getResourceName(s.namespacedName, s.secretNameSuffix()), }, } @@ -225,38 +223,8 @@ func (s *server) reconcileSecret(ctx context.Context, c ctrlclient.Client) error return err } -func (s *server) getExistingCert(ctx context.Context, c ctrlclient.Client) (*bytes.Buffer, *bytes.Buffer, bool, error) { - serverSecret := &corev1.Secret{} - err := c.Get(ctx, types.NamespacedName{ - Namespace: s.namespacedName.Namespace, - Name: s.prefixedName(stunnelSecret), - }, serverSecret) - switch { - case k8serrors.IsNotFound(err): - return nil, nil, false, nil - case err != nil: - return nil, nil, false, err - } - - key, ok := serverSecret.Data["tls.key"] - if !ok { - s.logger.Info("stunnel server secret data missing key tls.key", "secret", types.NamespacedName{ - Namespace: s.namespacedName.Namespace, - Name: s.prefixedName(stunnelSecret), - }) - return nil, nil, false, nil - } - - crt, ok := serverSecret.Data["tls.crt"] - if !ok { - s.logger.Info("stunnel server secret data missing key tls.crt", "secret", types.NamespacedName{ - Namespace: s.namespacedName.Namespace, - Name: s.prefixedName(stunnelSecret), - }) - return nil, nil, false, nil - } - - return bytes.NewBuffer(key), bytes.NewBuffer(crt), true, nil +func (s *server) secretNameSuffix() string { + return "server-" + stunnelSecret } func (s *server) serverContainers() []corev1.Container { @@ -282,7 +250,7 @@ func (s *server) serverContainers() []corev1.Container { SubPath: "stunnel.conf", }, { - Name: s.prefixedName(stunnelSecret), + Name: getResourceName(s.namespacedName, s.secretNameSuffix()), MountPath: "/etc/stunnel/certs", }, }, @@ -303,10 +271,10 @@ func (s *server) serverVolumes() []corev1.Volume { }, }, { - Name: s.prefixedName(stunnelSecret), + Name: getResourceName(s.namespacedName, s.secretNameSuffix()), VolumeSource: corev1.VolumeSource{ Secret: &corev1.SecretVolumeSource{ - SecretName: s.prefixedName(stunnelSecret), + SecretName: getResourceName(s.namespacedName, s.secretNameSuffix()), Items: []corev1.KeyToPath{ { Key: "tls.crt", diff --git a/transport/stunnel/server_test.go b/transport/stunnel/server_test.go index 7d8f022..6ce9b7a 100644 --- a/transport/stunnel/server_test.go +++ b/transport/stunnel/server_test.go @@ -72,7 +72,7 @@ func newFakeEndpoint() endpoint.Endpoint { } } -func TestNew(t *testing.T) { +func TestNewServer(t *testing.T) { tests := []struct { name string namespacedName types.NamespacedName @@ -165,9 +165,9 @@ func TestNew(t *testing.T) { fakeClient := fakeClientWithObjects(tt.objects...) ctx := context.WithValue(context.Background(), "test", tt.name) fakeLogger := logrtesting.TestLogger{t} - stunnelServer, err := New(ctx, fakeClient, fakeLogger, tt.namespacedName, tt.endpoint, &transport.Options{Labels: tt.labels, Owners: tt.ownerReferences}) + stunnelServer, err := NewServer(ctx, fakeClient, fakeLogger, tt.namespacedName, tt.endpoint, &transport.Options{Labels: tt.labels, Owners: tt.ownerReferences}) if (err != nil) != tt.wantErr { - t.Errorf("New() error = %v, wantErr %v", err, tt.wantErr) + t.Errorf("NewServer() error = %v, wantErr %v", err, tt.wantErr) return } cm := &corev1.ConfigMap{} @@ -299,105 +299,3 @@ func Test_server_MarkForCleanup(t *testing.T) { }) } } - -func Test_server_getExistingCert(t *testing.T) { - tests := []struct { - name string - namespacedName types.NamespacedName - labels map[string]string - wantErr bool - wantFound bool - objects []ctrlclient.Object - }{ - { - name: "test with no secret", - namespacedName: types.NamespacedName{Namespace: "bar", Name: "foo"}, - labels: map[string]string{"test": "me"}, - wantErr: false, - wantFound: false, - objects: []ctrlclient.Object{}, - }, - { - name: "test with invalid secret, key missing", - namespacedName: types.NamespacedName{Namespace: "bar", Name: "foo"}, - labels: map[string]string{"test": "me"}, - wantErr: false, - wantFound: false, - objects: []ctrlclient.Object{ - &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: "foo-server-stunnel-credentials", - Namespace: "bar", - Labels: map[string]string{"test": "me"}, - }, - Data: map[string][]byte{"tls.crt": []byte(`crt`)}, - }, - }, - }, - { - name: "test with invalid secret, crt missing", - namespacedName: types.NamespacedName{Namespace: "bar", Name: "foo"}, - labels: map[string]string{"test": "me"}, - wantErr: false, - wantFound: false, - objects: []ctrlclient.Object{ - &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: "foo-server-stunnel-credentials", - Namespace: "bar", - Labels: map[string]string{"test": "me"}, - }, - Data: map[string][]byte{"tls.key": []byte(`key`)}, - }, - }, - }, - { - name: "test with valid secret", - namespacedName: types.NamespacedName{Namespace: "bar", Name: "foo"}, - labels: map[string]string{"test": "me"}, - wantErr: false, - wantFound: true, - objects: []ctrlclient.Object{ - &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: "foo-server-stunnel-credentials", - Namespace: "bar", - Labels: map[string]string{"test": "me"}, - }, - Data: map[string][]byte{"tls.key": []byte(`key`), "tls.crt": []byte(`crt`)}, - }, - }, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - s := &server{ - logger: logrtesting.TestLogger{t}, - namespacedName: tt.namespacedName, - options: &transport.Options{ - Labels: tt.labels, - Owners: testOwnerReferences(), - }, - } - ctx := context.WithValue(context.Background(), "test", tt.name) - key, crt, found, err := s.getExistingCert(ctx, fakeClientWithObjects(tt.objects...)) - if err != nil { - t.Error("found unexpected error", err) - } - if !tt.wantFound && found { - t.Error("found unexpected secret") - } - if tt.wantFound && !found { - t.Error("not found unexpected") - } - - if tt.wantFound && found && key == nil { - t.Error("secret found but empty key, unexpected") - } - - if tt.wantFound && found && crt == nil { - t.Error("secret found but empty crt, unexpected") - } - }) - } -} diff --git a/transport/stunnel/stunnel.go b/transport/stunnel/stunnel.go index 7696c8a..dfc13ba 100644 --- a/transport/stunnel/stunnel.go +++ b/transport/stunnel/stunnel.go @@ -1,6 +1,16 @@ package stunnel -import "github.com/backube/pvc-transfer/transport" +import ( + "bytes" + "context" + + "github.com/backube/pvc-transfer/transport" + "github.com/go-logr/logr" + corev1 "k8s.io/api/core/v1" + k8serrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/types" + ctrlclient "sigs.k8s.io/controller-runtime/pkg/client" +) const ( defaultStunnelImage = "quay.io/konveyor/rsync-transfer:latest" @@ -20,3 +30,41 @@ func getImage(options *transport.Options) string { return options.Image } } + +func getResourceName(obj types.NamespacedName, suffix string) string { + return obj.Name + "-" + suffix +} + +func getExistingCert(ctx context.Context, c ctrlclient.Client, logger logr.Logger, secretName types.NamespacedName, suffix string) (*bytes.Buffer, *bytes.Buffer, bool, error) { + secret := &corev1.Secret{} + err := c.Get(ctx, types.NamespacedName{ + Namespace: secretName.Namespace, + Name: getResourceName(secretName, suffix), + }, secret) + switch { + case k8serrors.IsNotFound(err): + return nil, nil, false, nil + case err != nil: + return nil, nil, false, err + } + + key, ok := secret.Data["tls.key"] + if !ok { + logger.Info("secret data missing key tls.key", "secret", types.NamespacedName{ + Namespace: secretName.Namespace, + Name: getResourceName(secretName, suffix), + }) + return nil, nil, false, nil + } + + crt, ok := secret.Data["tls.crt"] + if !ok { + logger.Info("secret data missing key tls.crt", "secret", types.NamespacedName{ + Namespace: secretName.Namespace, + Name: getResourceName(secretName, suffix), + }) + return nil, nil, false, nil + } + + return bytes.NewBuffer(key), bytes.NewBuffer(crt), true, nil +} diff --git a/transport/stunnel/stunnel_test.go b/transport/stunnel/stunnel_test.go new file mode 100644 index 0000000..f0676f5 --- /dev/null +++ b/transport/stunnel/stunnel_test.go @@ -0,0 +1,115 @@ +package stunnel + +import ( + "context" + "testing" + + "github.com/backube/pvc-transfer/transport" + logrtesting "github.com/go-logr/logr/testing" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + ctrlclient "sigs.k8s.io/controller-runtime/pkg/client" +) + +func Test_getExistingCert(t *testing.T) { + tests := []struct { + name string + namespacedName types.NamespacedName + labels map[string]string + wantErr bool + wantFound bool + objects []ctrlclient.Object + }{ + { + name: "test with no secret", + namespacedName: types.NamespacedName{Namespace: "bar", Name: "foo"}, + labels: map[string]string{"test": "me"}, + wantErr: false, + wantFound: false, + objects: []ctrlclient.Object{}, + }, + { + name: "test with invalid secret, key missing", + namespacedName: types.NamespacedName{Namespace: "bar", Name: "foo"}, + labels: map[string]string{"test": "me"}, + wantErr: false, + wantFound: false, + objects: []ctrlclient.Object{ + &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo-stunnel-credentials", + Namespace: "bar", + Labels: map[string]string{"test": "me"}, + }, + Data: map[string][]byte{"tls.crt": []byte(`crt`)}, + }, + }, + }, + { + name: "test with invalid secret, crt missing", + namespacedName: types.NamespacedName{Namespace: "bar", Name: "foo"}, + labels: map[string]string{"test": "me"}, + wantErr: false, + wantFound: false, + objects: []ctrlclient.Object{ + &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo-stunnel-credentials", + Namespace: "bar", + Labels: map[string]string{"test": "me"}, + }, + Data: map[string][]byte{"tls.key": []byte(`key`)}, + }, + }, + }, + { + name: "test with valid secret", + namespacedName: types.NamespacedName{Namespace: "bar", Name: "foo"}, + labels: map[string]string{"test": "me"}, + wantErr: false, + wantFound: true, + objects: []ctrlclient.Object{ + &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo-stunnel-credentials", + Namespace: "bar", + Labels: map[string]string{"test": "me"}, + }, + Data: map[string][]byte{"tls.key": []byte(`key`), "tls.crt": []byte(`crt`)}, + }, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + s := &server{ + logger: logrtesting.TestLogger{t}, + namespacedName: tt.namespacedName, + options: &transport.Options{ + Labels: tt.labels, + Owners: testOwnerReferences(), + }, + } + ctx := context.WithValue(context.Background(), "test", tt.name) + key, crt, found, err := getExistingCert(ctx, fakeClientWithObjects(tt.objects...), s.logger, s.namespacedName, stunnelSecret) + if err != nil { + t.Error("found unexpected error", err) + } + if !tt.wantFound && found { + t.Error("found unexpected secret") + } + if tt.wantFound && !found { + t.Error("not found unexpected") + } + + if tt.wantFound && found && key == nil { + t.Error("secret found but empty key, unexpected") + } + + if tt.wantFound && found && crt == nil { + t.Error("secret found but empty crt, unexpected") + } + }) + } +} From 96e9b5846e50396d53edbfa03422030e791f952f Mon Sep 17 00:00:00 2001 From: Alay Patel Date: Fri, 12 Nov 2021 17:08:51 -0500 Subject: [PATCH 5/9] fixups: add comments and documentations Signed-off-by: Alay Patel --- transport/stunnel/server.go | 8 ++++++-- transport/transport.go | 17 ++++++++++++++--- 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/transport/stunnel/server.go b/transport/stunnel/server.go index 8577179..da1c044 100644 --- a/transport/stunnel/server.go +++ b/transport/stunnel/server.go @@ -19,13 +19,17 @@ import ( ) const ( + // TCP_NODELAY=1 bypasses Nagle's Delay algorithm + // this means that the tcp stack does not way of receiving an acc + // before sending the next packet https://en.wikipedia.org/wiki/Nagle%27s_algorithm + // At scale setting/unsetting this option might drive different network characteristics stunnelServerConfTemplate = `foreground = yes pid = socket = l:TCP_NODELAY=1 socket = r:TCP_NODELAY=1 debug = 7 -sslVersion = TLSv1.2 -[rsync] +sslVersion = TLSv1.3 +[transfer] accept = {{ $.acceptPort }} connect = {{ $.connectPort }} key = /etc/stunnel/certs/tls.key diff --git a/transport/transport.go b/transport/transport.go index d1a65ed..599d9d7 100644 --- a/transport/transport.go +++ b/transport/transport.go @@ -17,6 +17,8 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" ) +// Transport exposes the methods required for transfers to add +// a tunneling mechanism for the traffic sent over the network. type Transport interface { // NamespacedName returns the namespaced name to identify this transport Transport NamespacedName() types.NamespacedName @@ -44,15 +46,24 @@ type Transport interface { MarkForCleanup(ctx context.Context, c client.Client, key, value string) error } +// Options allows users of the transport to configure certain field type Options struct { + // Labels will be applied to objects reconciled by the transport Labels map[string]string + // Owners will be applied to all objects reconciled by the transport Owners []metav1.OwnerReference - Image string + // Image allows for specifying the image used for running the transport containers + Image string - ProxyURL string + // ProxyURL is used if the cluster is behind a proxy + ProxyURL string + // ProxyUsername username for connecting to the proxy ProxyUsername string + // ProxyPassword password for connecting to the proxy ProxyPassword string - NoVerifyCA bool + // NoVerifyCA allows you to override verification of TLS certs + NoVerifyCA bool + // CAVerifyLevel the level at which CA certs will be verify if NoVerifyCA is false CAVerifyLevel string } From 402af80e2dc4fd12fc63a13ffd1a700e4f977328 Mon Sep 17 00:00:00 2001 From: Alay Patel Date: Fri, 12 Nov 2021 17:11:29 -0500 Subject: [PATCH 6/9] fixups: context, flip NamespacedName, type cast "stunnel" Co-authored-by: John Strunk Signed-off-by: Alay Patel --- transport/stunnel/server.go | 4 ++-- transport/stunnel/stunnel.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/transport/stunnel/server.go b/transport/stunnel/server.go index da1c044..ad7a1c3 100644 --- a/transport/stunnel/server.go +++ b/transport/stunnel/server.go @@ -144,7 +144,7 @@ func (s *server) MarkForCleanup(ctx context.Context, c ctrlclient.Client, key, v Namespace: s.NamespacedName().Namespace, }, } - return utils.UpdateWithLabel(context.TODO(), c, secret, key, value) + return utils.UpdateWithLabel(ctx, c, secret, key, value) } func (s *server) reconcileConfig(ctx context.Context, c ctrlclient.Client) error { @@ -169,8 +169,8 @@ func (s *server) reconcileConfig(ctx context.Context, c ctrlclient.Client) error stunnelConfigMap := &corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ - Namespace: s.NamespacedName().Namespace, Name: s.prefixedName(stunnelConfig), + Namespace: s.NamespacedName().Namespace, }, } diff --git a/transport/stunnel/stunnel.go b/transport/stunnel/stunnel.go index dfc13ba..eb179c4 100644 --- a/transport/stunnel/stunnel.go +++ b/transport/stunnel/stunnel.go @@ -19,7 +19,7 @@ const ( ) const ( - TransportTypeStunnel = "stunnel" + TransportTypeStunnel transport.Type = "stunnel" Container = "stunnel" ) From e10d983bd19dff5b48889b30cecce289b8da3c63 Mon Sep 17 00:00:00 2001 From: Alay Patel Date: Wed, 17 Nov 2021 22:42:10 -0500 Subject: [PATCH 7/9] add tls authentication for transport Signed-off-by: Alay Patel --- transport/stunnel/server.go | 80 ++++++++++--- transport/stunnel/stunnel.go | 14 ++- transport/tls/certs/generate.go | 168 +++++++++++++++++++++++++++ transport/tls/certs/generate_test.go | 104 +++++++++++++++++ transport/transport.go | 62 ---------- 5 files changed, 347 insertions(+), 81 deletions(-) create mode 100644 transport/tls/certs/generate.go create mode 100644 transport/tls/certs/generate_test.go diff --git a/transport/stunnel/server.go b/transport/stunnel/server.go index ad7a1c3..f301801 100644 --- a/transport/stunnel/server.go +++ b/transport/stunnel/server.go @@ -9,6 +9,7 @@ import ( "github.com/backube/pvc-transfer/endpoint" "github.com/backube/pvc-transfer/internal/utils" "github.com/backube/pvc-transfer/transport" + "github.com/backube/pvc-transfer/transport/tls/certs" "github.com/go-logr/logr" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -20,7 +21,7 @@ import ( const ( // TCP_NODELAY=1 bypasses Nagle's Delay algorithm - // this means that the tcp stack does not way of receiving an acc + // this means that the tcp stack does not wait for receiving an ack // before sending the next packet https://en.wikipedia.org/wiki/Nagle%27s_algorithm // At scale setting/unsetting this option might drive different network characteristics stunnelServerConfTemplate = `foreground = yes @@ -34,6 +35,8 @@ accept = {{ $.acceptPort }} connect = {{ $.connectPort }} key = /etc/stunnel/certs/tls.key cert = /etc/stunnel/certs/tls.crt +CAfile = /etc/stunnel/certs/ca.crt +verify = 2 TIMEOUTclose = 0 ` stunnelConnectPort = 8080 @@ -191,7 +194,7 @@ func (s *server) prefixedName(name string) string { } func (s *server) reconcileSecret(ctx context.Context, c ctrlclient.Client) error { - _, _, found, err := getExistingCert(ctx, c, s.logger, s.namespacedName, s.secretNameSuffix()) + _, _, found, err := getExistingCert(ctx, c, s.logger, s.namespacedName, serverSecretNameSuffix()) if found { return nil } @@ -201,36 +204,77 @@ func (s *server) reconcileSecret(ctx context.Context, c ctrlclient.Client) error return err } - _, newCrt, newKey, err := transport.GenerateSSLCert() + crtBundle, err := certs.New() if err != nil { s.logger.Error(err, "error generating ssl certs for stunnel server") return err } - stunnelSecret := &corev1.Secret{ + crtBundleSecret := &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ Namespace: s.NamespacedName().Namespace, - Name: getResourceName(s.namespacedName, s.secretNameSuffix()), + Name: getResourceName(s.namespacedName, caBundleSecretNameSuffix()), }, } + _, err = controllerutil.CreateOrUpdate(ctx, c, crtBundleSecret, func() error { + crtBundleSecret.Labels = s.options.Labels + crtBundleSecret.OwnerReferences = s.options.Owners + + crtBundleSecret.Data = map[string][]byte{ + "server.crt": crtBundle.ServerCrt.Bytes(), + "server.key": crtBundle.ServerKey.Bytes(), + "client.crt": crtBundle.ClientCrt.Bytes(), + "client.key": crtBundle.ClientKey.Bytes(), + "ca.crt": crtBundle.CACrt.Bytes(), + "ca.key": crtBundle.CAKey.Bytes(), + } + return nil + }) + if err != nil { + return err + } - _, err = controllerutil.CreateOrUpdate(ctx, c, stunnelSecret, func() error { - stunnelSecret.Labels = s.options.Labels - stunnelSecret.OwnerReferences = s.options.Owners + serverSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: s.NamespacedName().Namespace, + Name: getResourceName(s.namespacedName, serverSecretNameSuffix()), + }, + } + _, err = controllerutil.CreateOrUpdate(ctx, c, serverSecret, func() error { + serverSecret.Labels = s.options.Labels + serverSecret.OwnerReferences = s.options.Owners + + serverSecret.Data = map[string][]byte{ + "tls.crt": crtBundle.ServerCrt.Bytes(), + "tls.key": crtBundle.ServerKey.Bytes(), + "ca.crt": crtBundle.CACrt.Bytes(), + } + return nil + }) + if err != nil { + return err + } - stunnelSecret.Data = map[string][]byte{ - "tls.crt": newCrt.Bytes(), - "tls.key": newKey.Bytes(), + clientSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: s.NamespacedName().Namespace, + Name: getResourceName(s.namespacedName, clientSecretNameSuffix()), + }, + } + _, err = controllerutil.CreateOrUpdate(ctx, c, clientSecret, func() error { + clientSecret.Labels = s.options.Labels + clientSecret.OwnerReferences = s.options.Owners + + clientSecret.Data = map[string][]byte{ + "tls.crt": crtBundle.ClientCrt.Bytes(), + "tls.key": crtBundle.ClientKey.Bytes(), + "ca.crt": crtBundle.CACrt.Bytes(), } return nil }) return err } -func (s *server) secretNameSuffix() string { - return "server-" + stunnelSecret -} - func (s *server) serverContainers() []corev1.Container { return []corev1.Container{ { @@ -254,7 +298,7 @@ func (s *server) serverContainers() []corev1.Container { SubPath: "stunnel.conf", }, { - Name: getResourceName(s.namespacedName, s.secretNameSuffix()), + Name: getResourceName(s.namespacedName, serverSecretNameSuffix()), MountPath: "/etc/stunnel/certs", }, }, @@ -275,10 +319,10 @@ func (s *server) serverVolumes() []corev1.Volume { }, }, { - Name: getResourceName(s.namespacedName, s.secretNameSuffix()), + Name: getResourceName(s.namespacedName, serverSecretNameSuffix()), VolumeSource: corev1.VolumeSource{ Secret: &corev1.SecretVolumeSource{ - SecretName: getResourceName(s.namespacedName, s.secretNameSuffix()), + SecretName: getResourceName(s.namespacedName, serverSecretNameSuffix()), Items: []corev1.KeyToPath{ { Key: "tls.crt", diff --git a/transport/stunnel/stunnel.go b/transport/stunnel/stunnel.go index eb179c4..5bb96c5 100644 --- a/transport/stunnel/stunnel.go +++ b/transport/stunnel/stunnel.go @@ -20,9 +20,21 @@ const ( const ( TransportTypeStunnel transport.Type = "stunnel" - Container = "stunnel" + Container = "stunnel" ) +func serverSecretNameSuffix() string { + return "server-" + stunnelSecret +} + +func clientSecretNameSuffix() string { + return "client-" + stunnelSecret +} + +func caBundleSecretNameSuffix() string { + return "ca-bundle-" + stunnelSecret +} + func getImage(options *transport.Options) string { if options.Image == "" { return defaultStunnelImage diff --git a/transport/tls/certs/generate.go b/transport/tls/certs/generate.go new file mode 100644 index 0000000..c69acb3 --- /dev/null +++ b/transport/tls/certs/generate.go @@ -0,0 +1,168 @@ +package certs + +import ( + "bytes" + "crypto/rand" + "crypto/rsa" + "crypto/x509" + "crypto/x509/pkix" + "encoding/pem" + "math/big" + "time" +) + +var ( + keySize = 2048 + defaultCASubject = &pkix.Name{ + Country: []string{"US"}, + Province: []string{"NC"}, + Locality: []string{"RDU"}, + Organization: []string{"Backube"}, + OrganizationalUnit: []string{"Engineering"}, + // This does not have to be a domain name, but certain implementations/configuration + // verify the requests from other side using this and SAN fields. + CommonName: "ca.backube.dev", + } + defaultCrtSubject = &pkix.Name{ + Country: []string{"US"}, + Province: []string{"NC"}, + Locality: []string{"RDU"}, + Organization: []string{"Backube"}, + OrganizationalUnit: []string{"Engineering"}, + CommonName: "cert.backube.dev", + } +) + +// CertificateBundle stores the data used for creating a secret with tls bundle +// that includes a self signed CA (crt and key) as well as client and server certs +// (cert and key). +type CertificateBundle struct { + caRSAKey *rsa.PrivateKey + caCrtTemplate *x509.Certificate + + CACrt *bytes.Buffer + CAKey *bytes.Buffer + ServerCrt *bytes.Buffer + ServerKey *bytes.Buffer + ClientCrt *bytes.Buffer + ClientKey *bytes.Buffer +} + +// New returns CertificateBundle after populating all the public fields. It should +// ideally be persisted in kubernetes objects (secrets) by consumers. If the secret is +// lost or deleted, New should be called again to get a fresh bundle. +func New() (*CertificateBundle, error) { + c := &CertificateBundle{} + var err error + c.CACrt, c.caRSAKey, c.caCrtTemplate, err = GenerateCA(defaultCASubject) + if err != nil { + return nil, err + } + + c.CAKey, err = rsaKeyBytes(c.caRSAKey) + + c.ServerCrt, c.ServerKey, err = Generate(defaultCrtSubject, *c.caCrtTemplate, *c.caRSAKey) + if err != nil { + return nil, err + } + + c.ClientCrt, c.ClientKey, err = Generate(defaultCrtSubject, *c.caCrtTemplate, *c.caRSAKey) + if err != nil { + return nil, err + } + return c, nil +} + +// GenerateCA take a subject and returns caCrt, caKey and caCrtTemplate +// The caKey and caCrtTemplate should be passed into Generate +// along with a similar subject except the CN name should be different from +// the CA. +func GenerateCA(subject *pkix.Name) (caCrt *bytes.Buffer, caKey *rsa.PrivateKey, caCrtTemplate *x509.Certificate, err error) { + if subject == nil { + subject = defaultCASubject + } + caCrtTemplate = &x509.Certificate{ + SerialNumber: big.NewInt(2021), + Subject: *subject, + NotBefore: time.Now(), + NotAfter: time.Now().AddDate(10, 0, 0), + IsCA: true, + ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageClientAuth, x509.ExtKeyUsageServerAuth, x509.ExtKeyUsageAny}, + KeyUsage: x509.KeyUsageDigitalSignature | x509.KeyUsageCertSign, + BasicConstraintsValid: true, + } + caCrt, caKey, err = createCrtKeyPair(caCrtTemplate, nil, nil) + if err != nil { + return + } + return +} + +// Generate takes a subject, caCrtTemplate and caKey and returns crt, key and error +// if error is not nil, do not rely on crt or keys being not nil. +func Generate(subject *pkix.Name, caCrtTemplate x509.Certificate, caKey rsa.PrivateKey) (crt *bytes.Buffer, key *bytes.Buffer, err error) { + crtTemplate := &x509.Certificate{ + SerialNumber: big.NewInt(2020), + Subject: *subject, + NotBefore: time.Now(), + NotAfter: time.Now().AddDate(10, 0, 0), + ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageClientAuth, x509.ExtKeyUsageServerAuth}, + KeyUsage: x509.KeyUsageDigitalSignature | x509.KeyUsageCertSign, + } + + crt, rsaKey, err := createCrtKeyPair(crtTemplate, &caCrtTemplate, &caKey) + if err != nil { + return + } + key, err = rsaKeyBytes(rsaKey) + if err != nil { + return + } + return +} + +func createCrtKeyPair(crtTemplate, parent *x509.Certificate, signer *rsa.PrivateKey) (crt *bytes.Buffer, key *rsa.PrivateKey, err error) { + key, err = rsa.GenerateKey(rand.Reader, keySize) + if err != nil { + return + } + if parent == nil { + parent = crtTemplate + } + if signer == nil { + signer = key + } + + crtBytes, err := x509.CreateCertificate( + rand.Reader, + crtTemplate, + parent, + &key.PublicKey, + signer, + ) + if err != nil { + return + } + + crt = new(bytes.Buffer) + err = pem.Encode(crt, &pem.Block{ + Type: "CERTIFICATE", + Bytes: crtBytes, + }) + if err != nil { + return nil, nil, err + } + return +} + +func rsaKeyBytes(key *rsa.PrivateKey) (keyBytes *bytes.Buffer, err error) { + keyBytes = new(bytes.Buffer) + err = pem.Encode(keyBytes, &pem.Block{ + Type: "RSA PRIVATE KEY", + Bytes: x509.MarshalPKCS1PrivateKey(key), + }) + if err != nil { + return + } + return +} diff --git a/transport/tls/certs/generate_test.go b/transport/tls/certs/generate_test.go new file mode 100644 index 0000000..4d07d23 --- /dev/null +++ b/transport/tls/certs/generate_test.go @@ -0,0 +1,104 @@ +package certs + +import ( + "bytes" + "crypto/x509" + "encoding/pem" + "testing" +) + +func TestNew(t *testing.T) { + tests := []struct { + name string + wantErr bool + }{ + { + name: "test new and verify the CA", + wantErr: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := New() + if (err != nil) != tt.wantErr { + t.Errorf("New() error = %v, wantErr %v", err, tt.wantErr) + return + } + + if got != nil && got.CACrt == nil { + t.Error("ca cert is nil") + return + } + if got != nil && got.CAKey == nil { + t.Error("ca key is nil") + return + } + if got != nil && got.ServerCrt == nil { + t.Error("server crt is nil") + return + } + if got != nil && got.ServerKey == nil { + t.Error("server key is nil") + return + } + if got != nil && got.ClientCrt == nil { + t.Error("client crt is nil") + return + } + if got != nil && got.ClientKey == nil { + t.Error("client key is nil") + return + } + + //if !verifySingedCAFiles() { + // t.Error("client cert is not verified with root CA") + //} + + if !verifySingedCA(got.CACrt, got.ClientCrt) { + t.Error("client cert is not verified with root CA") + } + if !verifySingedCA(got.CACrt, got.ServerCrt) { + t.Error("server cert is not verified with root CA") + } + + got2, err := New() + if (err != nil) != tt.wantErr { + t.Errorf("New() error = %v, wantErr %v", err, tt.wantErr) + return + } + if verifySingedCA(got.CACrt, got2.ClientCrt) { + t.Error("client cert is verified with different root CA") + } + if verifySingedCA(got.CACrt, got2.ServerCrt) { + t.Error("server cert is not verified with different root CA") + } + }) + } +} + +func verifySingedCA(caCrt *bytes.Buffer, crt *bytes.Buffer) bool { + roots := x509.NewCertPool() + ok := roots.AppendCertsFromPEM(caCrt.Bytes()) + if !ok { + panic("failed to parse root certificate") + } + + block, _ := pem.Decode(crt.Bytes()) + if block == nil { + panic("failed to parse certificate") + } + cert, err := x509.ParseCertificate(block.Bytes) + if err != nil { + panic("failed to parse certificate: " + err.Error()) + } + + opts := x509.VerifyOptions{ + Roots: roots, + KeyUsages: []x509.ExtKeyUsage{x509.ExtKeyUsageAny}, + } + + if _, err := cert.Verify(opts); err != nil { + return false + } + return true +} diff --git a/transport/transport.go b/transport/transport.go index 599d9d7..12e83ad 100644 --- a/transport/transport.go +++ b/transport/transport.go @@ -1,15 +1,7 @@ package transport import ( - "bytes" "context" - "crypto/rand" - "crypto/rsa" - "crypto/x509" - "crypto/x509/pkix" - "encoding/pem" - "math/big" - "time" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -68,57 +60,3 @@ type Options struct { } type Type string - -func GenerateSSLCert() (*bytes.Buffer, *bytes.Buffer, *bytes.Buffer, error) { - caPrivKey, err := rsa.GenerateKey(rand.Reader, 4096) - if err != nil { - return nil, nil, nil, err - } - - subj := pkix.Name{ - CommonName: "backube.dev", - Country: []string{"US"}, - Organization: []string{"Backube"}, - OrganizationalUnit: []string{"Engineering"}, - } - - certTemp := x509.Certificate{ - SerialNumber: big.NewInt(2020), - Subject: subj, - NotBefore: time.Now(), - NotAfter: time.Now().AddDate(10, 0, 0), - IsCA: true, - ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageClientAuth, x509.ExtKeyUsageServerAuth}, - KeyUsage: x509.KeyUsageDigitalSignature | x509.KeyUsageCertSign, - BasicConstraintsValid: true, - } - - caBytes, err := x509.CreateCertificate( - rand.Reader, - &certTemp, - &certTemp, - &caPrivKey.PublicKey, - caPrivKey, - ) - if err != nil { - return nil, nil, nil, err - } - crt := new(bytes.Buffer) - err = pem.Encode(crt, &pem.Block{ - Type: "CERTIFICATE", - Bytes: caBytes, - }) - if err != nil { - return nil, nil, nil, err - } - key := new(bytes.Buffer) - err = pem.Encode(key, &pem.Block{ - Type: "RSA PRIVATE KEY", - Bytes: x509.MarshalPKCS1PrivateKey(caPrivKey), - }) - if err != nil { - return nil, nil, nil, err - } - - return crt, crt, key, nil -} From 75c304a1976b1751bbea501d79f9e102cd61adde Mon Sep 17 00:00:00 2001 From: Alay Patel Date: Wed, 17 Nov 2021 23:52:23 -0500 Subject: [PATCH 8/9] update unit tests to check for valid stunnel secret Signed-off-by: Alay Patel --- transport/stunnel/server.go | 11 ++++---- transport/stunnel/server_test.go | 19 +++++++++----- transport/stunnel/stunnel.go | 36 ++++++++++++++++---------- transport/stunnel/stunnel_test.go | 38 ++++++++++++++++++---------- transport/tls/certs/generate.go | 30 ++++++++++++++++++++++ transport/tls/certs/generate_test.go | 38 +++------------------------- 6 files changed, 100 insertions(+), 72 deletions(-) diff --git a/transport/stunnel/server.go b/transport/stunnel/server.go index f301801..58ddeb1 100644 --- a/transport/stunnel/server.go +++ b/transport/stunnel/server.go @@ -194,16 +194,17 @@ func (s *server) prefixedName(name string) string { } func (s *server) reconcileSecret(ctx context.Context, c ctrlclient.Client) error { - _, _, found, err := getExistingCert(ctx, c, s.logger, s.namespacedName, serverSecretNameSuffix()) - if found { - return nil - } - + secretValid, err := isSecretValid(ctx, c, s.logger, s.namespacedName, serverSecretNameSuffix()) if err != nil { s.logger.Error(err, "error getting existing ssl certs from secret") return err } + if secretValid { + s.logger.V(4).Info("found secret with valid certs") + return nil + } + s.logger.Info("generating new certificate bundle") crtBundle, err := certs.New() if err != nil { s.logger.Error(err, "error generating ssl certs for stunnel server") diff --git a/transport/stunnel/server_test.go b/transport/stunnel/server_test.go index 6ce9b7a..07736d9 100644 --- a/transport/stunnel/server_test.go +++ b/transport/stunnel/server_test.go @@ -21,7 +21,7 @@ import ( func fakeClientWithObjects(objs ...ctrlclient.Object) ctrlclient.WithWatch { scheme := runtime.NewScheme() - AddToScheme(scheme) + _ = AddToScheme(scheme) return fake.NewClientBuilder().WithScheme(scheme).WithObjects(objs...).Build() } @@ -57,11 +57,11 @@ func (f fakeEndpoint) IngressPort() int32 { return 1234 } -func (f fakeEndpoint) IsHealthy(ctx context.Context, c ctrlclient.Client) (bool, error) { +func (f fakeEndpoint) IsHealthy(_ context.Context, _ ctrlclient.Client) (bool, error) { return true, nil } -func (f fakeEndpoint) MarkForCleanup(ctx context.Context, c ctrlclient.Client, key, value string) error { +func (f fakeEndpoint) MarkForCleanup(_ context.Context, _ ctrlclient.Client, _, _ string) error { return nil } @@ -164,7 +164,7 @@ func TestNewServer(t *testing.T) { t.Run(tt.name, func(t *testing.T) { fakeClient := fakeClientWithObjects(tt.objects...) ctx := context.WithValue(context.Background(), "test", tt.name) - fakeLogger := logrtesting.TestLogger{t} + fakeLogger := logrtesting.TestLogger{T: t} stunnelServer, err := NewServer(ctx, fakeClient, fakeLogger, tt.namespacedName, tt.endpoint, &transport.Options{Labels: tt.labels, Owners: tt.ownerReferences}) if (err != nil) != tt.wantErr { t.Errorf("NewServer() error = %v, wantErr %v", err, tt.wantErr) @@ -179,11 +179,11 @@ func TestNewServer(t *testing.T) { panic(fmt.Errorf("%#v should not be getting error from fake client", err)) } - configdata, ok := cm.Data["stunnel.conf"] + configData, ok := cm.Data["stunnel.conf"] if !ok { t.Error("unable to find stunnel config data in configmap") } - if !strings.Contains(configdata, "foreground = yes") { + if !strings.Contains(configData, "foreground = yes") { t.Error("configmap data does not contain the right data") } @@ -206,6 +206,11 @@ func TestNewServer(t *testing.T) { t.Error("unable to find tls.crt in stunnel secret") } + _, ok = secret.Data["ca.crt"] + if !ok { + t.Error("unable to find ca.crt in stunnel secret") + } + if len(stunnelServer.Volumes()) == 0 { t.Error("stunnel server volumes not set properly") } @@ -257,7 +262,7 @@ func Test_server_MarkForCleanup(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { s := &server{ - logger: logrtesting.TestLogger{t}, + logger: logrtesting.TestLogger{T: t}, options: &transport.Options{ Labels: tt.labels, Owners: testOwnerReferences(), diff --git a/transport/stunnel/stunnel.go b/transport/stunnel/stunnel.go index 5bb96c5..018d7bd 100644 --- a/transport/stunnel/stunnel.go +++ b/transport/stunnel/stunnel.go @@ -5,6 +5,7 @@ import ( "context" "github.com/backube/pvc-transfer/transport" + "github.com/backube/pvc-transfer/transport/tls/certs" "github.com/go-logr/logr" corev1 "k8s.io/api/core/v1" k8serrors "k8s.io/apimachinery/pkg/api/errors" @@ -47,36 +48,45 @@ func getResourceName(obj types.NamespacedName, suffix string) string { return obj.Name + "-" + suffix } -func getExistingCert(ctx context.Context, c ctrlclient.Client, logger logr.Logger, secretName types.NamespacedName, suffix string) (*bytes.Buffer, *bytes.Buffer, bool, error) { +func isSecretValid(ctx context.Context, c ctrlclient.Client, logger logr.Logger, key types.NamespacedName, suffix string) (bool, error) { secret := &corev1.Secret{} err := c.Get(ctx, types.NamespacedName{ - Namespace: secretName.Namespace, - Name: getResourceName(secretName, suffix), + Namespace: key.Namespace, + Name: getResourceName(key, suffix), }, secret) switch { case k8serrors.IsNotFound(err): - return nil, nil, false, nil + return false, nil case err != nil: - return nil, nil, false, err + return false, err } - key, ok := secret.Data["tls.key"] + _, ok := secret.Data["tls.key"] if !ok { logger.Info("secret data missing key tls.key", "secret", types.NamespacedName{ - Namespace: secretName.Namespace, - Name: getResourceName(secretName, suffix), + Namespace: key.Namespace, + Name: getResourceName(key, suffix), }) - return nil, nil, false, nil + return false, nil } crt, ok := secret.Data["tls.crt"] if !ok { logger.Info("secret data missing key tls.crt", "secret", types.NamespacedName{ - Namespace: secretName.Namespace, - Name: getResourceName(secretName, suffix), + Namespace: key.Namespace, + Name: getResourceName(key, suffix), }) - return nil, nil, false, nil + return false, nil } - return bytes.NewBuffer(key), bytes.NewBuffer(crt), true, nil + ca, ok := secret.Data["ca.crt"] + if !ok { + logger.Info("secret data missing key ca.crt", "secret", types.NamespacedName{ + Namespace: key.Namespace, + Name: getResourceName(key, suffix), + }) + return false, nil + } + + return certs.VerifyCertificate(bytes.NewBuffer(ca), bytes.NewBuffer(crt)) } diff --git a/transport/stunnel/stunnel_test.go b/transport/stunnel/stunnel_test.go index f0676f5..f34796d 100644 --- a/transport/stunnel/stunnel_test.go +++ b/transport/stunnel/stunnel_test.go @@ -5,6 +5,7 @@ import ( "testing" "github.com/backube/pvc-transfer/transport" + "github.com/backube/pvc-transfer/transport/tls/certs" logrtesting "github.com/go-logr/logr/testing" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -12,6 +13,8 @@ import ( ctrlclient "sigs.k8s.io/controller-runtime/pkg/client" ) +var certificateBundle, _ = certs.New() + func Test_getExistingCert(t *testing.T) { tests := []struct { name string @@ -42,7 +45,7 @@ func Test_getExistingCert(t *testing.T) { Namespace: "bar", Labels: map[string]string{"test": "me"}, }, - Data: map[string][]byte{"tls.crt": []byte(`crt`)}, + Data: map[string][]byte{"tls.crt": certificateBundle.ServerCrt.Bytes()}, }, }, }, @@ -59,7 +62,24 @@ func Test_getExistingCert(t *testing.T) { Namespace: "bar", Labels: map[string]string{"test": "me"}, }, - Data: map[string][]byte{"tls.key": []byte(`key`)}, + Data: map[string][]byte{"tls.key": certificateBundle.ServerKey.Bytes()}, + }, + }, + }, + { + name: "test with secret missing ca.crt", + namespacedName: types.NamespacedName{Namespace: "bar", Name: "foo"}, + labels: map[string]string{"test": "me"}, + wantErr: true, + wantFound: false, + objects: []ctrlclient.Object{ + &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo-stunnel-credentials", + Namespace: "bar", + Labels: map[string]string{"test": "me"}, + }, + Data: map[string][]byte{"tls.key": certificateBundle.ServerKey.Bytes(), "tls.crt": certificateBundle.ServerKey.Bytes()}, }, }, }, @@ -67,7 +87,7 @@ func Test_getExistingCert(t *testing.T) { name: "test with valid secret", namespacedName: types.NamespacedName{Namespace: "bar", Name: "foo"}, labels: map[string]string{"test": "me"}, - wantErr: false, + wantErr: true, wantFound: true, objects: []ctrlclient.Object{ &corev1.Secret{ @@ -76,7 +96,7 @@ func Test_getExistingCert(t *testing.T) { Namespace: "bar", Labels: map[string]string{"test": "me"}, }, - Data: map[string][]byte{"tls.key": []byte(`key`), "tls.crt": []byte(`crt`)}, + Data: map[string][]byte{"tls.key": certificateBundle.ServerKey.Bytes(), "tls.crt": certificateBundle.ServerCrt.Bytes(), "ca.crt": certificateBundle.CACrt.Bytes()}, }, }, }, @@ -92,7 +112,7 @@ func Test_getExistingCert(t *testing.T) { }, } ctx := context.WithValue(context.Background(), "test", tt.name) - key, crt, found, err := getExistingCert(ctx, fakeClientWithObjects(tt.objects...), s.logger, s.namespacedName, stunnelSecret) + found, err := isSecretValid(ctx, fakeClientWithObjects(tt.objects...), s.logger, s.namespacedName, stunnelSecret) if err != nil { t.Error("found unexpected error", err) } @@ -102,14 +122,6 @@ func Test_getExistingCert(t *testing.T) { if tt.wantFound && !found { t.Error("not found unexpected") } - - if tt.wantFound && found && key == nil { - t.Error("secret found but empty key, unexpected") - } - - if tt.wantFound && found && crt == nil { - t.Error("secret found but empty crt, unexpected") - } }) } } diff --git a/transport/tls/certs/generate.go b/transport/tls/certs/generate.go index c69acb3..2c6ac58 100644 --- a/transport/tls/certs/generate.go +++ b/transport/tls/certs/generate.go @@ -7,6 +7,7 @@ import ( "crypto/x509" "crypto/x509/pkix" "encoding/pem" + "fmt" "math/big" "time" ) @@ -121,6 +122,35 @@ func Generate(subject *pkix.Name, caCrtTemplate x509.Certificate, caKey rsa.Priv return } +// VerifyCertificate returns true if the crt is signed by the caCrt as the root CA +// with no intermediate DCAs in the chain +func VerifyCertificate(caCrt *bytes.Buffer, crt *bytes.Buffer) (bool, error) { + roots := x509.NewCertPool() + ok := roots.AppendCertsFromPEM(caCrt.Bytes()) + if !ok { + panic("failed to parse root certificate") + } + + block, _ := pem.Decode(crt.Bytes()) + if block == nil { + return false, fmt.Errorf("unable to decode certificate") + } + cert, err := x509.ParseCertificate(block.Bytes) + if err != nil { + return false, fmt.Errorf("failed to parse certificate: %#v", err) + } + + opts := x509.VerifyOptions{ + Roots: roots, + KeyUsages: []x509.ExtKeyUsage{x509.ExtKeyUsageAny}, + } + + if _, err := cert.Verify(opts); err != nil { + return false, nil + } + return true, nil +} + func createCrtKeyPair(crtTemplate, parent *x509.Certificate, signer *rsa.PrivateKey) (crt *bytes.Buffer, key *rsa.PrivateKey, err error) { key, err = rsa.GenerateKey(rand.Reader, keySize) if err != nil { diff --git a/transport/tls/certs/generate_test.go b/transport/tls/certs/generate_test.go index 4d07d23..b72cbea 100644 --- a/transport/tls/certs/generate_test.go +++ b/transport/tls/certs/generate_test.go @@ -1,9 +1,6 @@ package certs import ( - "bytes" - "crypto/x509" - "encoding/pem" "testing" ) @@ -54,10 +51,10 @@ func TestNew(t *testing.T) { // t.Error("client cert is not verified with root CA") //} - if !verifySingedCA(got.CACrt, got.ClientCrt) { + if ok, _ := VerifyCertificate(got.CACrt, got.ClientCrt); !ok { t.Error("client cert is not verified with root CA") } - if !verifySingedCA(got.CACrt, got.ServerCrt) { + if ok, _ := VerifyCertificate(got.CACrt, got.ServerCrt); !ok { t.Error("server cert is not verified with root CA") } @@ -66,39 +63,12 @@ func TestNew(t *testing.T) { t.Errorf("New() error = %v, wantErr %v", err, tt.wantErr) return } - if verifySingedCA(got.CACrt, got2.ClientCrt) { + if ok, _ := VerifyCertificate(got.CACrt, got2.ClientCrt); ok { t.Error("client cert is verified with different root CA") } - if verifySingedCA(got.CACrt, got2.ServerCrt) { + if ok, _ := VerifyCertificate(got.CACrt, got2.ServerCrt); ok { t.Error("server cert is not verified with different root CA") } }) } } - -func verifySingedCA(caCrt *bytes.Buffer, crt *bytes.Buffer) bool { - roots := x509.NewCertPool() - ok := roots.AppendCertsFromPEM(caCrt.Bytes()) - if !ok { - panic("failed to parse root certificate") - } - - block, _ := pem.Decode(crt.Bytes()) - if block == nil { - panic("failed to parse certificate") - } - cert, err := x509.ParseCertificate(block.Bytes) - if err != nil { - panic("failed to parse certificate: " + err.Error()) - } - - opts := x509.VerifyOptions{ - Roots: roots, - KeyUsages: []x509.ExtKeyUsage{x509.ExtKeyUsageAny}, - } - - if _, err := cert.Verify(opts); err != nil { - return false - } - return true -} From 4b9f475655653d5e9f7d52f86bb429f5229f75c9 Mon Sep 17 00:00:00 2001 From: Alay Patel Date: Thu, 18 Nov 2021 11:13:24 -0500 Subject: [PATCH 9/9] stunnel: flip namespace-name, separate func for creating bundle, update MarkForCleanup Signed-off-by: Alay Patel --- transport/stunnel/server.go | 93 +++++++++----------------------- transport/stunnel/server_test.go | 47 ++++++++++++++-- transport/stunnel/stunnel.go | 72 +++++++++++++++++++++++++ 3 files changed, 141 insertions(+), 71 deletions(-) diff --git a/transport/stunnel/server.go b/transport/stunnel/server.go index 58ddeb1..958fc9e 100644 --- a/transport/stunnel/server.go +++ b/transport/stunnel/server.go @@ -141,13 +141,35 @@ func (s *server) MarkForCleanup(ctx context.Context, c ctrlclient.Client, key, v return err } - secret := &corev1.Secret{ + clientSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: getResourceName(s.namespacedName, clientSecretNameSuffix()), + Namespace: s.NamespacedName().Namespace, + }, + } + err = utils.UpdateWithLabel(ctx, c, clientSecret, key, value) + if err != nil { + return err + } + + serverSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: getResourceName(s.namespacedName, serverSecretNameSuffix()), + Namespace: s.NamespacedName().Namespace, + }, + } + err = utils.UpdateWithLabel(ctx, c, serverSecret, key, value) + if err != nil { + return err + } + + crtBundleSecret := &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ - Name: s.prefixedName(stunnelSecret), + Name: getResourceName(s.namespacedName, caBundleSecretNameSuffix()), Namespace: s.NamespacedName().Namespace, }, } - return utils.UpdateWithLabel(ctx, c, secret, key, value) + return utils.UpdateWithLabel(ctx, c, crtBundleSecret, key, value) } func (s *server) reconcileConfig(ctx context.Context, c ctrlclient.Client) error { @@ -210,70 +232,7 @@ func (s *server) reconcileSecret(ctx context.Context, c ctrlclient.Client) error s.logger.Error(err, "error generating ssl certs for stunnel server") return err } - - crtBundleSecret := &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: s.NamespacedName().Namespace, - Name: getResourceName(s.namespacedName, caBundleSecretNameSuffix()), - }, - } - _, err = controllerutil.CreateOrUpdate(ctx, c, crtBundleSecret, func() error { - crtBundleSecret.Labels = s.options.Labels - crtBundleSecret.OwnerReferences = s.options.Owners - - crtBundleSecret.Data = map[string][]byte{ - "server.crt": crtBundle.ServerCrt.Bytes(), - "server.key": crtBundle.ServerKey.Bytes(), - "client.crt": crtBundle.ClientCrt.Bytes(), - "client.key": crtBundle.ClientKey.Bytes(), - "ca.crt": crtBundle.CACrt.Bytes(), - "ca.key": crtBundle.CAKey.Bytes(), - } - return nil - }) - if err != nil { - return err - } - - serverSecret := &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: s.NamespacedName().Namespace, - Name: getResourceName(s.namespacedName, serverSecretNameSuffix()), - }, - } - _, err = controllerutil.CreateOrUpdate(ctx, c, serverSecret, func() error { - serverSecret.Labels = s.options.Labels - serverSecret.OwnerReferences = s.options.Owners - - serverSecret.Data = map[string][]byte{ - "tls.crt": crtBundle.ServerCrt.Bytes(), - "tls.key": crtBundle.ServerKey.Bytes(), - "ca.crt": crtBundle.CACrt.Bytes(), - } - return nil - }) - if err != nil { - return err - } - - clientSecret := &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: s.NamespacedName().Namespace, - Name: getResourceName(s.namespacedName, clientSecretNameSuffix()), - }, - } - _, err = controllerutil.CreateOrUpdate(ctx, c, clientSecret, func() error { - clientSecret.Labels = s.options.Labels - clientSecret.OwnerReferences = s.options.Owners - - clientSecret.Data = map[string][]byte{ - "tls.crt": crtBundle.ClientCrt.Bytes(), - "tls.key": crtBundle.ClientKey.Bytes(), - "ca.crt": crtBundle.CACrt.Bytes(), - } - return nil - }) - return err + return reconcileCertificateSecrets(ctx, c, s.namespacedName, s.options, crtBundle) } func (s *server) serverContainers() []corev1.Container { diff --git a/transport/stunnel/server_test.go b/transport/stunnel/server_test.go index 07736d9..0aed654 100644 --- a/transport/stunnel/server_test.go +++ b/transport/stunnel/server_test.go @@ -248,6 +248,22 @@ func Test_server_MarkForCleanup(t *testing.T) { }, Data: map[string][]byte{"tls.key": []byte(`key`), "tls.crt": []byte(`crt`)}, }, + &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo-client-stunnel-credentials", + Namespace: "bar", + Labels: map[string]string{"test": "me"}, + }, + Data: map[string][]byte{"tls.key": []byte(`key`), "tls.crt": []byte(`crt`)}, + }, + &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo-ca-bundle-stunnel-credentials", + Namespace: "bar", + Labels: map[string]string{"test": "me"}, + }, + Data: map[string][]byte{"tls.key": []byte(`key`), "tls.crt": []byte(`crt`)}, + }, &corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ Name: "foo-server-stunnel-config", @@ -289,17 +305,40 @@ func Test_server_MarkForCleanup(t *testing.T) { t.Errorf("labels on configmap = %#v, wanted %#v", cm.Labels, tt.labels) } - secret := &corev1.Secret{} + secretSecret := &corev1.Secret{} err = fakeClient.Get(context.Background(), types.NamespacedName{ Namespace: "bar", Name: "foo-server-" + stunnelSecret, - }, secret) + }, secretSecret) + if err != nil { + panic(fmt.Errorf("%#v should not be getting error from fake client", err)) + } + if !reflect.DeepEqual(tt.labels, secretSecret.Labels) { + t.Errorf("labels on secretSecret = %#v, wanted %#v", secretSecret.Labels, tt.labels) + } + + clientSecret := &corev1.Secret{} + err = fakeClient.Get(context.Background(), types.NamespacedName{ + Namespace: "bar", + Name: "foo-client-" + stunnelSecret, + }, clientSecret) if err != nil { panic(fmt.Errorf("%#v should not be getting error from fake client", err)) } + if !reflect.DeepEqual(tt.labels, clientSecret.Labels) { + t.Errorf("labels on secretSecret = %#v, wanted %#v", secretSecret.Labels, tt.labels) + } - if !reflect.DeepEqual(tt.labels, secret.Labels) { - t.Errorf("labels on secret = %#v, wanted %#v", secret.Labels, tt.labels) + caBundleSecret := &corev1.Secret{} + err = fakeClient.Get(context.Background(), types.NamespacedName{ + Namespace: "bar", + Name: "foo-ca-bundle-" + stunnelSecret, + }, caBundleSecret) + if err != nil { + panic(fmt.Errorf("%#v should not be getting error from fake client", err)) + } + if !reflect.DeepEqual(tt.labels, clientSecret.Labels) { + t.Errorf("labels on secretSecret = %#v, wanted %#v", secretSecret.Labels, tt.labels) } }) } diff --git a/transport/stunnel/stunnel.go b/transport/stunnel/stunnel.go index 018d7bd..fb372c6 100644 --- a/transport/stunnel/stunnel.go +++ b/transport/stunnel/stunnel.go @@ -3,6 +3,8 @@ package stunnel import ( "bytes" "context" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "github.com/backube/pvc-transfer/transport" "github.com/backube/pvc-transfer/transport/tls/certs" @@ -90,3 +92,73 @@ func isSecretValid(ctx context.Context, c ctrlclient.Client, logger logr.Logger, return certs.VerifyCertificate(bytes.NewBuffer(ca), bytes.NewBuffer(crt)) } + +func reconcileCertificateSecrets(ctx context.Context, + c ctrlclient.Client, + key types.NamespacedName, + options *transport.Options, + crtBundle *certs.CertificateBundle) error { + crtBundleSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: getResourceName(key, caBundleSecretNameSuffix()), + Namespace: key.Namespace, + }, + } + _, err := controllerutil.CreateOrUpdate(ctx, c, crtBundleSecret, func() error { + crtBundleSecret.Labels = options.Labels + crtBundleSecret.OwnerReferences = options.Owners + + crtBundleSecret.Data = map[string][]byte{ + "server.crt": crtBundle.ServerCrt.Bytes(), + "server.key": crtBundle.ServerKey.Bytes(), + "client.crt": crtBundle.ClientCrt.Bytes(), + "client.key": crtBundle.ClientKey.Bytes(), + "ca.crt": crtBundle.CACrt.Bytes(), + "ca.key": crtBundle.CAKey.Bytes(), + } + return nil + }) + if err != nil { + return err + } + + serverSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: getResourceName(key, serverSecretNameSuffix()), + Namespace: key.Namespace, + }, + } + _, err = controllerutil.CreateOrUpdate(ctx, c, serverSecret, func() error { + serverSecret.Labels = options.Labels + serverSecret.OwnerReferences = options.Owners + + serverSecret.Data = map[string][]byte{ + "tls.crt": crtBundle.ServerCrt.Bytes(), + "tls.key": crtBundle.ServerKey.Bytes(), + "ca.crt": crtBundle.CACrt.Bytes(), + } + return nil + }) + if err != nil { + return err + } + + clientSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: getResourceName(key, clientSecretNameSuffix()), + Namespace: key.Namespace, + }, + } + _, err = controllerutil.CreateOrUpdate(ctx, c, clientSecret, func() error { + clientSecret.Labels = options.Labels + clientSecret.OwnerReferences = options.Owners + + clientSecret.Data = map[string][]byte{ + "tls.crt": crtBundle.ClientCrt.Bytes(), + "tls.key": crtBundle.ClientKey.Bytes(), + "ca.crt": crtBundle.CACrt.Bytes(), + } + return nil + }) + return err +}