TLS improvements

internal/command/context/create.go

@@ -1,17 +1,18 @@
 package context
 
 import (
+	"context"
 	"crypto/sha256"
 	"crypto/tls"
 	"crypto/x509"
 	"encoding/pem"
-	"errors"
 	"fmt"
 	"github.com/cirruslabs/orchard/internal/bootstraptoken"
 	"github.com/cirruslabs/orchard/internal/config"
 	"github.com/cirruslabs/orchard/internal/netconstants"
-	"github.com/cirruslabs/orchard/pkg/client"
+	clientpkg "github.com/cirruslabs/orchard/pkg/client"
 	"github.com/manifoldco/promptui"
+	"github.com/pkg/errors"
 	"github.com/spf13/cobra"
 	"net/url"
 	"strconv"
@@ -24,6 +25,7 @@ var bootstrapTokenRaw string
 var serviceAccountName string
 var serviceAccountToken string
 var force bool
+var noPKI bool
 
 func newCreateCommand() *cobra.Command {
 	command := &cobra.Command{
@@ -43,6 +45,10 @@ func newCreateCommand() *cobra.Command {
 		"service account token to use (alternative to --bootstrap-token)")
 	command.PersistentFlags().BoolVar(&force, "force", false,
 		"create the context even if a context with the same name already exists")
+	command.PersistentFlags().BoolVar(&noPKI, "no-pki", false,
+		"do not use the host's root CA set and instead validate the Controller's presented "+
+			"certificate using a bootstrap token (or manually via fingerprint, "+
+			"if no bootstrap token is provided)")
 
 	return command
 }
@@ -53,65 +59,119 @@ func runCreate(cmd *cobra.Command, args []string) error {
 		return err
 	}
 
-	// Establish trust
-	var trustedControllerCertificate *x509.Certificate
+	// If the bootstrap token is present, extract
+	// service account credentials from it
+	// and remember it for further use
+	var bootstrapToken *bootstraptoken.BootstrapToken
 
 	if bootstrapTokenRaw != "" {
-		bootstrapToken, err := bootstraptoken.NewFromString(bootstrapTokenRaw)
+		bootstrapToken, err = bootstraptoken.NewFromString(bootstrapTokenRaw)
 		if err != nil {
 			return err
 		}
 
-		serviceAccountName = bootstrapToken.ServiceAccountName()
-		serviceAccountToken = bootstrapToken.ServiceAccountToken()
-		trustedControllerCertificate = bootstrapToken.Certificate()
-	} else {
-		trustedControllerCertificate, err = probeControllerCertificate(controllerURL)
-		if err != nil {
-			return err
+		if serviceAccountName == "" {
+			serviceAccountName = bootstrapToken.ServiceAccountName()
+		}
+		if serviceAccountToken == "" {
+			serviceAccountToken = bootstrapToken.ServiceAccountToken()
 		}
 	}
 
-	client, err := client.New(
-		client.WithAddress(controllerURL.String()),
-		client.WithTrustedCertificate(trustedControllerCertificate),
-		client.WithCredentials(serviceAccountName, serviceAccountToken),
-	)
+	trustedCertificate, err := tryToConnectToTheController(cmd.Context(), controllerURL, bootstrapToken)
 	if err != nil {
 		return err
 	}
-	if err := client.Check(cmd.Context()); err != nil {
-		return err
-	}
 
 	// Create and save the context
 	configHandle, err := config.NewHandle()
 	if err != nil {
 		return err
 	}
 
-	certificatePEMBytes := pem.EncodeToMemory(&pem.Block{
-		Type:  "CERTIFICATE",
-		Bytes: trustedControllerCertificate.Raw,
-	})
-
-	return configHandle.CreateContext(contextName, config.Context{
+	newContext := config.Context{
 		URL:                 controllerURL.String(),
-		Certificate:         certificatePEMBytes,
 		ServiceAccountName:  serviceAccountName,
 		ServiceAccountToken: serviceAccountToken,
-	}, force)
+	}
+
+	if trustedCertificate != nil {
+		certificatePEMBytes := pem.EncodeToMemory(&pem.Block{
+			Type:  "CERTIFICATE",
+			Bytes: trustedCertificate.Raw,
+		})
+
+		newContext.Certificate = certificatePEMBytes
+	}
+
+	return configHandle.CreateContext(contextName, newContext, force)
 }
 
-func probeControllerCertificate(controllerURL *url.URL) (*x509.Certificate, error) {
-	// Do not use PKI
-	emptyPool := x509.NewCertPool()
+func tryToConnectToTheController(
+	ctx context.Context,
+	controllerURL *url.URL,
+	bootstrapToken *bootstraptoken.BootstrapToken,
+) (*x509.Certificate, error) {
+	if !noPKI {
+		if err := tryToConnectWithPKI(ctx, controllerURL); err == nil {
+			// Connection successful and no certificate retrieval is needed
+			return nil, nil
+		} else if errors.Is(err, clientpkg.ErrAPI) {
+			// Makes no sense to go any further since it's an upper layer (HTTP, not TLS) error
+			return nil, err
+		}
+	}
 
-	//nolint:gosec // since we're not using PKI, InsecureSkipVerify is a must here
+	return tryToConnectWithTrustedCertificate(ctx, controllerURL, bootstrapToken)
+}
+
+func tryToConnectWithPKI(ctx context.Context, controllerURL *url.URL) error {
+	client, err := clientpkg.New(
+		clientpkg.WithAddress(controllerURL.String()),
+		clientpkg.WithCredentials(serviceAccountName, serviceAccountToken),
+	)
+	if err != nil {
+		return err
+	}
+
+	return client.Check(ctx)
+}
+
+func tryToConnectWithTrustedCertificate(
+	ctx context.Context,
+	controllerURL *url.URL,
+	bootstrapToken *bootstraptoken.BootstrapToken,
+) (*x509.Certificate, error) {
+	// Either (1) retrieve a trusted certificate from the bootstrap token
+	// or (2) retrieve it from the Controller and verify it interactively
+	var trustedControllerCertificate *x509.Certificate
+	var err error
+
+	if bootstrapToken != nil {
+		trustedControllerCertificate = bootstrapToken.Certificate()
+	} else {
+		if trustedControllerCertificate, err = probeControllerCertificate(ctx, controllerURL); err != nil {
+			return nil, err
+		}
+	}
+
+	// Now try again with the trusted certificate
+	client, err := clientpkg.New(
+		clientpkg.WithAddress(controllerURL.String()),
+		clientpkg.WithCredentials(serviceAccountName, serviceAccountToken),
+		clientpkg.WithTrustedCertificate(trustedControllerCertificate),
+	)
+	if err != nil {
+		return nil, err
+	}
+
+	return trustedControllerCertificate, client.Check(ctx)
+}
+
+func probeControllerCertificate(ctx context.Context, controllerURL *url.URL) (*x509.Certificate, error) {
+	//nolint:gosec // without InsecureSkipVerify our VerifyConnection won't be called
 	insecureTLSConfig := &tls.Config{
-		MinVersion:         tls.VersionTLS13,
-		RootCAs:            emptyPool,
-		ServerName:         netconstants.DefaultControllerServerName,
+		MinVersion:         tls.VersionTLS12,
 		InsecureSkipVerify: true,
 	}
 
@@ -123,11 +183,17 @@ func probeControllerCertificate(controllerURL *url.URL) (*x509.Certificate, erro
 				ErrCreateFailed)
 		}
 
-		if len(state.PeerCertificates) != 1 {
-			return fmt.Errorf("%w: controller presented %d certificate(s), expected only one",
-				ErrCreateFailed, len(state.PeerCertificates))
+		if len(state.PeerCertificates) == 0 {
+			return fmt.Errorf("%w: controller presented no certificates, expected at least one",
+				ErrCreateFailed)
 		}
 
+		// According to TLS 1.2[1] and TLS 1.3[2] specs:
+		//
+		// "The sender's certificate MUST come first in the list."
+		//
+		// [1]: https://www.rfc-editor.org/rfc/rfc5246#section-7.4.2
+		// [2]: https://www.rfc-editor.org/rfc/rfc8446#section-4.4.2
 		controllerCert = state.PeerCertificates[0]
 		controllerCertFingerprint := sha256.Sum256(controllerCert.Raw)
 		formattedControllerCertFingerprint := formatFingerprint(controllerCertFingerprint[:])
@@ -177,7 +243,11 @@ func probeControllerCertificate(controllerURL *url.URL) (*x509.Certificate, erro
 		}
 	}
 
-	conn, err := tls.Dial("tcp", controllerURL.Host, insecureTLSConfig)
+	dialer := tls.Dialer{
+		Config: insecureTLSConfig,
+	}
+
+	conn, err := dialer.DialContext(ctx, "tcp", controllerURL.Host)
 	if err != nil {
 		return nil, err
 	}

internal/command/controller/init.go

@@ -27,7 +27,7 @@ func FindControllerCertificate(dataDir *controller.DataDir) (controllerCert tls.
 		if err = checkBothCertAndKeyAreSpecified(); err != nil {
 			return controllerCert, err
 		}
-		return tls.LoadX509KeyPair(controllerCertPath, controllerCertPath)
+		return tls.LoadX509KeyPair(controllerCertPath, controllerKeyPath)
 	} else if !dataDir.ControllerCertificateExists() {
 		// otherwise, generate a self-signed certificate if it's not already present
 		controllerCert, err = GenerateSelfSignedControllerCertificate()

internal/command/controller/run.go

@@ -80,7 +80,7 @@ func runController(cmd *cobra.Command, args []string) (err error) {
 		controller.WithDataDir(dataDir),
 		controller.WithLogger(logger),
 		controller.WithTLSConfig(&tls.Config{
-			MinVersion: tls.VersionTLS13,
+			MinVersion: tls.VersionTLS12,
 			Certificates: []tls.Certificate{
 				controllerCert,
 			},

internal/config/context.go

@@ -1,10 +1,9 @@
 package config
 
 import (
-	"crypto/tls"
 	"crypto/x509"
+	"encoding/pem"
 	"fmt"
-	"github.com/cirruslabs/orchard/internal/netconstants"
 )
 
 type Context struct {
@@ -14,20 +13,22 @@ type Context struct {
 	ServiceAccountToken string `yaml:"serviceAccountToken,omitempty"`
 }
 
-func (context *Context) TLSConfig() (*tls.Config, error) {
+func (context *Context) TrustedCertificate() (*x509.Certificate, error) {
 	if len(context.Certificate) == 0 {
 		return nil, nil
 	}
 
-	privatePool := x509.NewCertPool()
+	block, _ := pem.Decode(context.Certificate)
+	if block == nil {
+		return nil, fmt.Errorf("%w: failed to load context's certificate: no PEM data found",
+			ErrConfigReadFailed)
+	}
 
-	if ok := privatePool.AppendCertsFromPEM(context.Certificate); !ok {
-		return nil, fmt.Errorf("%w: failed to load context's certificate", ErrConfigReadFailed)
+	trustedCertificate, err := x509.ParseCertificate(block.Bytes)
+	if err != nil {
+		return nil, fmt.Errorf("%w: failed to load context's certificate: %v",
+			ErrConfigReadFailed, err)
 	}
 
-	return &tls.Config{
-		MinVersion: tls.VersionTLS13,
-		ServerName: netconstants.DefaultControllerServerName,
-		RootCAs:    privatePool,
-	}, nil
+	return trustedCertificate, nil
 }