Merge pull request #191 from Callisto13/final-auth-fixes

fix: Use Opaque secret for TLS config

api/v1alpha1/microvmcluster_types.go

@@ -38,22 +38,28 @@ type MicrovmClusterSpec struct {
 	// TLSSecretRef is a reference to the name of a secret which contains TLS cert information
 	// for connecting to Flintlock hosts.
 	// The secret should be created in the same namespace as the MicroVMCluster.
-	// The secret should be of type kubernetes.io/tls https://kubernetes.io/docs/concepts/configuration/secret/#tls-secrets
+	// The secret should be of type Opaque
 	// with the addition of a ca.crt key.
 	//
 	// apiVersion: v1
 	// kind: Secret
 	// metadata:
 	// 	name: secret-tls
 	// 	namespace: default  <- same as Cluster
-	// type: kubernetes.io/tls
+	// type: Opaque
 	// data:
 	// 	tls.crt: |
+	// 		-----BEGIN CERTIFICATE-----
 	// 		MIIC2DCCAcCgAwIBAgIBATANBgkqh ...
+	// 		-----END CERTIFICATE-----
 	// 	tls.key: |
+	// 		-----BEGIN EC PRIVATE KEY-----
 	// 		MIIEpgIBAAKCAQEA7yn3bRHQ5FHMQ ...
+	// 		-----END EC PRIVATE KEY-----
 	// 	ca.crt: |
+	// 		-----BEGIN CERTIFICATE-----
 	// 		MIIEpgIBAAKCAQEA7yn3bRHQ5FHMQ ...
+	// 		-----END CERTIFICATE-----
 	// +optional
 	TLSSecretRef string `json:"tlsSecretRef,omitempty"`
 }

api/v1alpha1/types.go

@@ -187,7 +187,7 @@ type Proxy struct {
 
 // TLSConfig represents config for connecting to TLS enabled hosts.
 type TLSConfig struct {
-	Cert   string `json:"cert"`
-	Key    string `json:"key"`
-	CACert string `json:"caCert"`
+	Cert   []byte `json:"cert"`
+	Key    []byte `json:"key"`
+	CACert []byte `json:"caCert"`
 }

config/crd/bases/infrastructure.cluster.x-k8s.io_microvmclusters.yaml

@@ -154,12 +154,13 @@ spec:
                   to the name of a secret which contains TLS cert information for
                   connecting to Flintlock hosts. The secret should be created in the
                   same namespace as the MicroVMCluster. The secret should be of type
-                  kubernetes.io/tls https://kubernetes.io/docs/concepts/configuration/secret/#tls-secrets
-                  with the addition of a ca.crt key. \n apiVersion: v1 kind: Secret
-                  metadata: name: secret-tls namespace: default  <- same as Cluster
-                  type: kubernetes.io/tls data: tls.crt: | MIIC2DCCAcCgAwIBAgIBATANBgkqh
-                  ... tls.key: | MIIEpgIBAAKCAQEA7yn3bRHQ5FHMQ ... ca.crt: | MIIEpgIBAAKCAQEA7yn3bRHQ5FHMQ
-                  ..."
+                  Opaque with the addition of a ca.crt key. \n apiVersion: v1 kind:
+                  Secret metadata: name: secret-tls namespace: default  <- same as
+                  Cluster type: Opaque data: tls.crt: | -----BEGIN CERTIFICATE-----
+                  MIIC2DCCAcCgAwIBAgIBATANBgkqh ... -----END CERTIFICATE----- tls.key:
+                  | -----BEGIN EC PRIVATE KEY----- MIIEpgIBAAKCAQEA7yn3bRHQ5FHMQ ...
+                  -----END EC PRIVATE KEY----- ca.crt: | -----BEGIN CERTIFICATE-----
+                  MIIEpgIBAAKCAQEA7yn3bRHQ5FHMQ ... -----END CERTIFICATE-----"
                 type: string
             required:
             - placement

controllers/microvmmachine_controller.go

@@ -315,7 +315,7 @@ func (r *MicrovmMachineReconciler) getMicrovmService(
 
 	tls, err := machineScope.GetTLSConfig()
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("getting tls config: %w", err)
 	}
 
 	clientOpts := []flclient.Options{

internal/client/auth.go

@@ -6,12 +6,13 @@ import (
 )
 
 type basicAuth struct {
-	token string
+	token           string
+	requireSecurity bool
 }
 
 // Basic creates a basicAuth with a token.
-func Basic(t string) basicAuth { //nolint: revive // this will not be used
-	return basicAuth{token: t}
+func Basic(t string, s bool) basicAuth { //nolint: revive // this will not be used
+	return basicAuth{token: t, requireSecurity: s}
 }
 
 // GetRequestMetadata fullfills the credentials.PerRPCCredentials interface,
@@ -25,6 +26,6 @@ func (b basicAuth) GetRequestMetadata(ctx context.Context, in ...string) (map[st
 }
 
 // GetRequestMetadata fullfills the credentials.PerRPCCredentials interface.
-func (basicAuth) RequireTransportSecurity() bool {
-	return true
+func (b basicAuth) RequireTransportSecurity() bool {
+	return b.requireSecurity
 }

internal/client/client.go

@@ -69,7 +69,11 @@ func NewFlintlockClient(address string, opts ...Options) (microvm.Client, error)
 	}
 
 	if cfg.basicAuthToken != "" {
-		dialOpts = append(dialOpts, grpc.WithPerRPCCredentials(Basic(cfg.basicAuthToken)))
+		dialOpts = append(dialOpts,
+			grpc.WithPerRPCCredentials(
+				Basic(cfg.basicAuthToken, cfg.tls != nil),
+			),
+		)
 	}
 
 	if cfg.proxy != nil {
@@ -102,13 +106,13 @@ func buildConfig(opts ...Options) clientConfig {
 }
 
 func loadTLS(cfg *infrav1.TLSConfig) (credentials.TransportCredentials, error) {
-	certificate, err := tls.LoadX509KeyPair(cfg.Cert, cfg.Key)
+	certificate, err := tls.X509KeyPair(cfg.Cert, cfg.Key)
 	if err != nil {
 		return nil, err
 	}
 
 	capool := x509.NewCertPool()
-	if !capool.AppendCertsFromPEM([]byte(cfg.CACert)) {
+	if !capool.AppendCertsFromPEM(cfg.CACert) {
 		return nil, fmt.Errorf("could not add cert to pool") //nolint: goerr113 // there is no err to wrap
 	}
 

internal/scope/machine.go

@@ -5,7 +5,6 @@ package scope
 
 import (
 	"context"
-	"encoding/base64"
 	"fmt"
 	"hash/crc32"
 	"sort"
@@ -342,42 +341,28 @@ func (m *MachineScope) GetTLSConfig() (*infrav1.TLSConfig, error) {
 		return nil, err
 	}
 
-	cert, err := decode(tlsSecret.Data, tlsCert)
-	if err != nil {
-		return nil, err
+	certBytes, ok := tlsSecret.Data[tlsCert]
+	if !ok {
+		return nil, &tlsError{tlsCert}
 	}
 
-	key, err := decode(tlsSecret.Data, tlsKey)
-	if err != nil {
-		return nil, err
+	keyBytes, ok := tlsSecret.Data[tlsKey]
+	if !ok {
+		return nil, &tlsError{tlsKey}
 	}
 
-	ca, err := decode(tlsSecret.Data, caCert)
-	if err != nil {
-		return nil, err
+	caBytes, ok := tlsSecret.Data[caCert]
+	if !ok {
+		return nil, &tlsError{caCert}
 	}
 
 	return &infrav1.TLSConfig{
-		Cert:   cert,
-		Key:    key,
-		CACert: ca,
+		Cert:   certBytes,
+		Key:    keyBytes,
+		CACert: caBytes,
 	}, nil
 }
 
-func decode(data map[string][]byte, key string) (string, error) {
-	val, ok := data[key]
-	if !ok {
-		return "", &tlsError{key}
-	}
-
-	dec, err := base64.StdEncoding.DecodeString(string(val))
-	if err != nil {
-		return "", err
-	}
-
-	return string(dec), nil
-}
-
 func (m *MachineScope) getFailureDomainFromProviderID(providerID string) string {
 	if providerID == "" {
 		return ""

internal/scope/machine_test.go

@@ -189,9 +189,9 @@ func TestMachineGetTLSConfig(t *testing.T) {
 	otherClusterNoTLS := newMicrovmCluster(clusterName)
 
 	tlsData := map[string][]byte{
-		"tls.crt": []byte("Zm9v"),
-		"tls.key": []byte("YmFy"),
-		"ca.crt":  []byte("YmF6"),
+		"tls.crt": []byte("foo"),
+		"tls.key": []byte("bar"),
+		"ca.crt":  []byte("baz"),
 	}
 	tlsSecret := newSecret(tlsSecretName, tlsData)
 
@@ -215,9 +215,9 @@ func TestMachineGetTLSConfig(t *testing.T) {
 			expected: func(cfg *v1alpha1.TLSConfig, err error) {
 				Expect(err).NotTo(HaveOccurred())
 				Expect(cfg).ToNot(BeNil())
-				Expect(cfg.Cert).To(Equal("foo"))
-				Expect(cfg.Key).To(Equal("bar"))
-				Expect(cfg.CACert).To(Equal("baz"))
+				Expect(cfg.Cert).To(Equal([]byte("foo")))
+				Expect(cfg.Key).To(Equal([]byte("bar")))
+				Expect(cfg.CACert).To(Equal([]byte("baz")))
 			},
 		},
 		{