fix: allow TLS access on multiple SNI certs (#812)

if GetCertificate() is set never set tls.Certificates

Author: harshavardhana

cmd/console/server.go

@@ -17,15 +17,12 @@
 package main
 
 import (
-	"context"
 	"fmt"
 	"io/ioutil"
 	"path/filepath"
 	"strconv"
 	"time"
 
-	xcerts "github.com/minio/pkg/certs"
-
 	"github.com/go-openapi/loads"
 	"github.com/jessevdk/go-flags"
 	"github.com/minio/cli"
@@ -143,12 +140,8 @@ func loadAllCerts(ctx *cli.Context) error {
 		return fmt.Errorf("unable to create certs CA directory at %s: failed with %w", certs.GlobalCertsCADir.Get(), err)
 	}
 
-	var manager *xcerts.Manager
 	// load the certificates and the CAs
-	restapi.GlobalRootCAs, restapi.GlobalPublicCerts, manager, err = certs.GetAllCertificatesAndCAs()
-	restapi.GlobalTLSCertsManager = &certs.TLSCertsManager{
-		Manager: manager,
-	}
+	restapi.GlobalRootCAs, restapi.GlobalPublicCerts, restapi.GlobalTLSCertsManager, err = certs.GetAllCertificatesAndCAs()
 	if err != nil {
 		return fmt.Errorf("unable to load certificates at %s: failed with %w", certs.GlobalCertsDir.Get(), err)
 	}
@@ -160,7 +153,7 @@ func loadAllCerts(ctx *cli.Context) error {
 		swaggerServerCACertificate := ctx.String("tls-ca")
 		// load tls cert and key from swagger server tls-certificate and tls-key flags
 		if swaggerServerCertificate != "" && swaggerServerCertificateKey != "" {
-			if err = restapi.GlobalTLSCertsManager.AddCertificate(context.Background(), swaggerServerCertificate, swaggerServerCertificateKey); err != nil {
+			if err = restapi.GlobalTLSCertsManager.AddCertificate(swaggerServerCertificate, swaggerServerCertificateKey); err != nil {
 				return err
 			}
 			if x509Certs, err := certs.ParsePublicCertFile(swaggerServerCertificate); err == nil {

pkg/certs/certs.go

@@ -234,7 +234,6 @@ func LoadX509KeyPair(certFile, keyFile string) (tls.Certificate, error) {
 }
 
 func GetTLSConfig() (x509Certs []*x509.Certificate, manager *xcerts.Manager, err error) {
-
 	ctx := context.Background()
 
 	if !(isFile(getPublicCertFile()) && isFile(getPrivateKeyFile())) {
@@ -314,30 +313,17 @@ func GetTLSConfig() (x509Certs []*x509.Certificate, manager *xcerts.Manager, err
 
 func GetAllCertificatesAndCAs() (*x509.CertPool, []*x509.Certificate, *xcerts.Manager, error) {
 	// load all CAs from ~/.console/certs/CAs
-	GlobalRootCAs, err := xcerts.GetRootCAs(GlobalCertsCADir.Get())
+	rootCAs, err := xcerts.GetRootCAs(GlobalCertsCADir.Get())
 	if err != nil {
 		return nil, nil, nil, err
 	}
 	// load all certs from ~/.console/certs
-	globalPublicCerts, globalTLSCertsManager, err := GetTLSConfig()
+	publicCerts, certsManager, err := GetTLSConfig()
 	if err != nil {
 		return nil, nil, nil, err
 	}
-	return GlobalRootCAs, globalPublicCerts, globalTLSCertsManager, nil
-}
-
-// TLSCertsManager custom TLS Manager for SNI support
-type TLSCertsManager struct {
-	*xcerts.Manager
-}
-
-// AddCertificate check if Manager is initialized and then append a new certificate to it
-func (m *TLSCertsManager) AddCertificate(ctx context.Context, publicKey, privateKey string) (err error) {
-	// If Cert Manager is not nil add more certificates
-	if m.Manager != nil {
-		return m.Manager.AddCertificate(publicKey, privateKey)
+	if rootCAs == nil {
+		rootCAs = &x509.CertPool{}
 	}
-	// Initialize cert manager
-	m.Manager, err = xcerts.NewManager(ctx, publicKey, privateKey, LoadX509KeyPair)
-	return err
+	return rootCAs, publicCerts, certsManager, nil
 }

restapi/config.go

@@ -24,7 +24,7 @@ import (
 	"strings"
 	"time"
 
-	"github.com/minio/console/pkg/certs"
+	xcerts "github.com/minio/pkg/certs"
 	"github.com/minio/pkg/env"
 )
 
@@ -276,7 +276,7 @@ var (
 	// GlobalPublicCerts has certificates Console will use to serve clients
 	GlobalPublicCerts []*x509.Certificate
 	// GlobalTLSCertsManager custom TLS Manager for SNI support
-	GlobalTLSCertsManager *certs.TLSCertsManager
+	GlobalTLSCertsManager *xcerts.Manager
 )
 
 // getK8sSAToken assumes the plugin is running inside a k8s pod and extract the current service account from the

restapi/configure_console.go

@@ -21,9 +21,9 @@ package restapi
 import (
 	"bytes"
 	"crypto/tls"
-	"crypto/x509"
 	"io"
 	"io/fs"
+	"log"
 	"net/http"
 	"strings"
 	"time"
@@ -145,24 +145,13 @@ func configureAPI(api *operations.ConsoleAPI) http.Handler {
 
 // The TLS configuration before HTTPS server starts.
 func configureTLS(tlsConfig *tls.Config) {
-	if GlobalRootCAs == nil {
-		GlobalRootCAs = &x509.CertPool{}
-	}
 	// Add the global public crts as part of global root CAs
 	for _, publicCrt := range GlobalPublicCerts {
-		// Add certificates to swagger TLS configuration
-		tlsConfig.Certificates = append(tlsConfig.Certificates, tls.Certificate{
-			Certificate: [][]byte{publicCrt.Raw},
-			Leaf:        publicCrt,
-		})
 		GlobalRootCAs.AddCert(publicCrt)
 	}
 
 	tlsConfig.RootCAs = GlobalRootCAs
-
-	if GlobalTLSCertsManager != nil {
-		tlsConfig.GetCertificate = GlobalTLSCertsManager.GetCertificate
-	}
+	tlsConfig.GetCertificate = GlobalTLSCertsManager.GetCertificate
 }
 
 // The middleware configuration is for the handler executors. These do not apply to the swagger.json document.
@@ -277,9 +266,18 @@ func wrapHandlerSinglePageApplication(h http.Handler) http.HandlerFunc {
 	}
 }
 
+type logWriter struct{}
+
+func (lw logWriter) Write(b []byte) (int, error) {
+	LogError(string(bytes.TrimSuffix(b, []byte("\n"))))
+	return len(b), nil
+}
+
 // As soon as server is initialized but not run yet, this function will be called.
 // If you need to modify a config, store server instance to stop it individually later, this is the place.
 // This function can be called multiple times, depending on the number of serving schemes.
 // scheme value will be set accordingly: "http", "https" or "unix"
-func configureServer(s *http.Server, scheme, addr string) {
+func configureServer(s *http.Server, _, _ string) {
+	// Turn-off random logging by Go internall
+	s.ErrorLog = log.New(&logWriter{}, "", 0)
 }