fn: SSL config adjustments (#1160)

SSL related FN_NODE_CERT (and related) settings are
not very clear today. Removing this in favor of a
simple map of tls.Config objects. Three keys are
provided for this map:

TLSGRPCServer
TLSAdminServer
TLSWebServer

which correspond to server TLS settings for the
associated services.

Operators/implementers can further add more
keys to the map and add their own TLS config.

api/agent/lb_agent_test.go

@@ -2,6 +2,7 @@ package agent
 
 import (
 	"context"
+	"crypto/tls"
 	"errors"
 	"fmt"
 	"io"
@@ -27,13 +28,12 @@ type mockRunner struct {
 type mockRunnerPool struct {
 	runners   []pool.Runner
 	generator pool.MTLSRunnerFactory
-	pki       *pool.PKIData
 }
 
 func newMockRunnerPool(rf pool.MTLSRunnerFactory, runnerAddrs []string) *mockRunnerPool {
 	var runners []pool.Runner
 	for _, addr := range runnerAddrs {
-		r, err := rf(addr, "", nil)
+		r, err := rf(addr, nil)
 		if err != nil {
 			continue
 		}
@@ -43,7 +43,6 @@ func newMockRunnerPool(rf pool.MTLSRunnerFactory, runnerAddrs []string) *mockRun
 	return &mockRunnerPool{
 		runners:   runners,
 		generator: rf,
-		pki:       &pool.PKIData{},
 	}
 }
 
@@ -56,7 +55,7 @@ func (rp *mockRunnerPool) Shutdown(context.Context) error {
 }
 
 func NewMockRunnerFactory(sleep time.Duration, maxCalls int32) pool.MTLSRunnerFactory {
-	return func(addr, cn string, pki *pool.PKIData) (pool.Runner, error) {
+	return func(addr string, tlsConf *tls.Config) (pool.Runner, error) {
 		return &mockRunner{
 			sleep:    sleep,
 			maxCalls: maxCalls,
@@ -66,7 +65,7 @@ func NewMockRunnerFactory(sleep time.Duration, maxCalls int32) pool.MTLSRunnerFa
 }
 
 func FaultyRunnerFactory() pool.MTLSRunnerFactory {
-	return func(addr, cn string, pki *pool.PKIData) (pool.Runner, error) {
+	return func(addr string, tlsConf *tls.Config) (pool.Runner, error) {
 		return &mockRunner{
 			addr: addr,
 		}, errors.New("Creation of new runner failed")

api/agent/pure_runner.go

@@ -4,7 +4,6 @@ import (
 	"bytes"
 	"context"
 	"crypto/tls"
-	"crypto/x509"
 	"encoding/hex"
 	"encoding/json"
 	"errors"
@@ -866,26 +865,22 @@ func (pr *pureRunner) Status(ctx context.Context, _ *empty.Empty) (*runner.Runne
 	return pr.handleStatusCall(ctx)
 }
 
-func DefaultPureRunner(cancel context.CancelFunc, addr string, da CallHandler, cert string, key string, ca string) (Agent, error) {
+func DefaultPureRunner(cancel context.CancelFunc, addr string, da CallHandler, tlsCfg *tls.Config) (Agent, error) {
 
 	agent := New(da)
 
 	// WARNING: SSL creds are optional.
-	if cert == "" || key == "" || ca == "" {
+	if tlsCfg == nil {
 		return NewPureRunner(cancel, addr, PureRunnerWithAgent(agent))
 	}
-	return NewPureRunner(cancel, addr, PureRunnerWithAgent(agent), PureRunnerWithSSL(cert, key, ca))
+	return NewPureRunner(cancel, addr, PureRunnerWithAgent(agent), PureRunnerWithSSL(tlsCfg))
 }
 
 type PureRunnerOption func(*pureRunner) error
 
-func PureRunnerWithSSL(cert string, key string, ca string) PureRunnerOption {
+func PureRunnerWithSSL(tlsCfg *tls.Config) PureRunnerOption {
 	return func(pr *pureRunner) error {
-		c, err := createCreds(cert, key, ca)
-		if err != nil {
-			return fmt.Errorf("Failed to create pure runner credentials: %s", err)
-		}
-		pr.creds = c
+		pr.creds = credentials.NewTLS(tlsCfg)
 		return nil
 	}
 }
@@ -961,34 +956,5 @@ func NewPureRunner(cancel context.CancelFunc, addr string, options ...PureRunner
 	return pr, nil
 }
 
-func createCreds(cert string, key string, ca string) (credentials.TransportCredentials, error) {
-	if cert == "" || key == "" || ca == "" {
-		return nil, errors.New("Failed to create credentials, cert/key/ca not provided")
-	}
-
-	// Load the certificates from disk
-	certificate, err := tls.LoadX509KeyPair(cert, key)
-	if err != nil {
-		return nil, fmt.Errorf("Could not load server key pair: %s", err)
-	}
-
-	// Create a certificate pool from the certificate authority
-	certPool := x509.NewCertPool()
-	authority, err := ioutil.ReadFile(ca)
-	if err != nil {
-		return nil, fmt.Errorf("Could not read ca certificate: %s", err)
-	}
-
-	if ok := certPool.AppendCertsFromPEM(authority); !ok {
-		return nil, errors.New("Failed to append client certs")
-	}
-
-	return credentials.NewTLS(&tls.Config{
-		ClientAuth:   tls.RequireAndVerifyClientCert,
-		Certificates: []tls.Certificate{certificate},
-		ClientCAs:    certPool,
-	}), nil
-}
-
 var _ runner.RunnerProtocolServer = &pureRunner{}
 var _ Agent = &pureRunner{}

api/agent/runner_client.go

@@ -2,6 +2,7 @@ package agent
 
 import (
 	"context"
+	"crypto/tls"
 	"encoding/hex"
 	"encoding/json"
 	"errors"
@@ -39,8 +40,8 @@ type gRPCRunner struct {
 	client  pb.RunnerProtocolClient
 }
 
-func SecureGRPCRunnerFactory(addr, runnerCertCN string, pki *pool.PKIData) (pool.Runner, error) {
-	conn, client, err := runnerConnection(addr, runnerCertCN, pki)
+func SecureGRPCRunnerFactory(addr string, tlsConf *tls.Config) (pool.Runner, error) {
+	conn, client, err := runnerConnection(addr, tlsConf)
 	if err != nil {
 		return nil, err
 	}
@@ -59,20 +60,15 @@ func (r *gRPCRunner) Close(context.Context) error {
 	return r.conn.Close()
 }
 
-func runnerConnection(address, runnerCertCN string, pki *pool.PKIData) (*grpc.ClientConn, pb.RunnerProtocolClient, error) {
+func runnerConnection(address string, tlsConf *tls.Config) (*grpc.ClientConn, pb.RunnerProtocolClient, error) {
 
 	ctx := context.Background()
 	logger := common.Logger(ctx).WithField("runner_addr", address)
 	ctx = common.WithLogger(ctx, logger)
 
 	var creds credentials.TransportCredentials
-	if pki != nil {
-		var err error
-		creds, err = grpcutil.CreateCredentials(pki.Cert, pki.Key, pki.Ca, runnerCertCN)
-		if err != nil {
-			logger.WithError(err).Error("Unable to create credentials to connect to runner node")
-			return nil, nil, err
-		}
+	if tlsConf != nil {
+		creds = credentials.NewTLS(tlsConf)
 	}
 
 	// we want to set a very short timeout to fail-fast if something goes wrong

api/agent/static_pool.go

@@ -2,6 +2,7 @@ package agent
 
 import (
 	"context"
+	"crypto/tls"
 
 	pool "github.com/fnproject/fn/api/runnerpool"
 	"github.com/sirupsen/logrus"
@@ -10,20 +11,20 @@ import (
 // manages a single set of runners ignoring lb groups
 type staticRunnerPool struct {
 	generator pool.MTLSRunnerFactory
-	pki       *pool.PKIData // can be nil when running in insecure mode
+	tlsConf   *tls.Config // can be nil when running in insecure mode
 	runnerCN  string
 	runners   []pool.Runner
 }
 
 func DefaultStaticRunnerPool(runnerAddresses []string) pool.RunnerPool {
-	return NewStaticRunnerPool(runnerAddresses, nil, "", SecureGRPCRunnerFactory)
+	return NewStaticRunnerPool(runnerAddresses, nil, SecureGRPCRunnerFactory)
 }
 
-func NewStaticRunnerPool(runnerAddresses []string, pki *pool.PKIData, runnerCN string, runnerFactory pool.MTLSRunnerFactory) pool.RunnerPool {
+func NewStaticRunnerPool(runnerAddresses []string, tlsConf *tls.Config, runnerFactory pool.MTLSRunnerFactory) pool.RunnerPool {
 	logrus.WithField("runners", runnerAddresses).Info("Starting static runner pool")
 	var runners []pool.Runner
 	for _, addr := range runnerAddresses {
-		r, err := runnerFactory(addr, runnerCN, pki)
+		r, err := runnerFactory(addr, tlsConf)
 		if err != nil {
 			logrus.WithError(err).WithField("runner_addr", addr).Warn("Invalid runner")
 			continue
@@ -33,8 +34,7 @@ func NewStaticRunnerPool(runnerAddresses []string, pki *pool.PKIData, runnerCN s
 	}
 	return &staticRunnerPool{
 		runners:   runners,
-		pki:       pki,
-		runnerCN:  runnerCN,
+		tlsConf:   tlsConf,
 		generator: runnerFactory,
 	}
 }

api/agent/static_pool_test.go

@@ -2,6 +2,7 @@ package agent
 
 import (
 	"context"
+	"crypto/tls"
 	"errors"
 	"testing"
 
@@ -9,7 +10,7 @@ import (
 )
 
 func setupStaticPool(runners []string) pool.RunnerPool {
-	return NewStaticRunnerPool(runners, nil, "", mockRunnerFactory)
+	return NewStaticRunnerPool(runners, nil, mockRunnerFactory)
 }
 
 var (
@@ -36,7 +37,7 @@ func (r *mockStaticRunner) Address() string {
 	return r.address
 }
 
-func mockRunnerFactory(addr, cn string, pki *pool.PKIData) (pool.Runner, error) {
+func mockRunnerFactory(addr string, tlsConf *tls.Config) (pool.Runner, error) {
 	return &mockStaticRunner{address: addr}, nil
 }
 

api/common/tls_utils.go

@@ -0,0 +1,96 @@
+package common
+
+import (
+	"crypto/tls"
+	"crypto/x509"
+	"errors"
+	"fmt"
+	"io/ioutil"
+	"os"
+	"path/filepath"
+)
+
+// A simple TLS Config generator using cert/key
+func NewTLSSimple(certPath, keyPath string) (*tls.Config, error) {
+
+	err := checkFile(certPath)
+	if err != nil {
+		return nil, err
+	}
+
+	err = checkFile(keyPath)
+	if err != nil {
+		return nil, err
+	}
+
+	// Load the certificates from disk
+	certificate, err := tls.LoadX509KeyPair(certPath, keyPath)
+	if err != nil {
+		return nil, fmt.Errorf("Could not load server key pair: %s", err)
+	}
+
+	return &tls.Config{
+		Certificates: []tls.Certificate{certificate},
+	}, nil
+}
+
+// Add a Client CA
+func AddClientCA(tlsConf *tls.Config, clientCAPath string) error {
+
+	err := checkFile(clientCAPath)
+	if err != nil {
+		return err
+	}
+	// Create a certificate pool from the certificate authority
+	authority, err := ioutil.ReadFile(clientCAPath)
+	if err != nil {
+		return fmt.Errorf("Could not read client CA (%s) certificate: %s", clientCAPath, err)
+	}
+
+	tlsConf.ClientAuth = tls.RequireAndVerifyClientCert
+	if tlsConf.ClientCAs == nil {
+		tlsConf.ClientCAs = x509.NewCertPool()
+	}
+
+	if ok := tlsConf.ClientCAs.AppendCertsFromPEM(authority); !ok {
+		return errors.New("Failed to append client certs")
+	}
+	return nil
+}
+
+// Add CA
+func AddCA(tlsConf *tls.Config, caPath string) error {
+
+	err := checkFile(caPath)
+	if err != nil {
+		return err
+	}
+
+	ca, err := ioutil.ReadFile(caPath)
+	if err != nil {
+		return fmt.Errorf("could not read ca (%s) certificate: %s", caPath, err)
+	}
+
+	if tlsConf.RootCAs == nil {
+		tlsConf.RootCAs = x509.NewCertPool()
+	}
+
+	// Append the certificates from the CA
+	if ok := tlsConf.RootCAs.AppendCertsFromPEM(ca); !ok {
+		return errors.New("failed to append ca certs")
+	}
+
+	return nil
+}
+
+func checkFile(path string) error {
+	absPath, err := filepath.Abs(path)
+	if err != nil {
+		return fmt.Errorf("Unable to resolve %v for TLS: please specify a valid and readable file", path)
+	}
+	_, err = os.Stat(absPath)
+	if err != nil {
+		return fmt.Errorf("Cannot stat %v for TLS: please specify a valid and readable file", absPath)
+	}
+	return nil
+}

api/runnerpool/runner_pool.go

@@ -2,6 +2,7 @@ package runnerpool
 
 import (
 	"context"
+	"crypto/tls"
 	"io"
 	"net/http"
 
@@ -22,15 +23,8 @@ type RunnerPool interface {
 	Shutdown(ctx context.Context) error
 }
 
-// PKIData encapsulates TLS certificate data
-type PKIData struct {
-	Ca   string
-	Key  string
-	Cert string
-}
-
 // MTLSRunnerFactory represents a factory method for constructing runners using mTLS
-type MTLSRunnerFactory func(addr, certCommonName string, pki *PKIData) (Runner, error)
+type MTLSRunnerFactory func(addr string, tlsConf *tls.Config) (Runner, error)
 
 // RunnerStatus is general information on Runner health as returned by Runner::Status() call
 type RunnerStatus struct {

api/server/server.go

@@ -4,6 +4,7 @@ import (
 	"bufio"
 	"bytes"
 	"context"
+	"crypto/tls"
 	"encoding/base64"
 	"errors"
 	"fmt"
@@ -104,15 +105,6 @@ const (
 	// EnvJaegerURL is the url of a jaeger node to send traces to.
 	EnvJaegerURL = "FN_JAEGER_URL"
 
-	// EnvCert is the certificate used to communicate with other fn nodes.
-	EnvCert = "FN_NODE_CERT"
-
-	// EnvCertKey is the key for the specified cert.
-	EnvCertKey = "FN_NODE_CERT_KEY"
-
-	// EnvCertAuth is the CA for the cert provided.
-	EnvCertAuth = "FN_NODE_CERT_AUTHORITY"
-
 	// EnvRIDHeader is the header name of the incoming request which holds the request ID
 	EnvRIDHeader = "FN_RID_HEADER"
 
@@ -158,6 +150,15 @@ const (
 	ServerTypePureRunner
 )
 
+const (
+	// TLS Configuration for the GRPC service
+	TLSGRPCServer = "TLSgRPCServer"
+	// TLS Configuration for the admin service
+	TLSAdminServer = "TLSAdminServer"
+	// TLS Configuration for the web service
+	TLSWebServer = "TLSWebServer"
+)
+
 func (s NodeType) String() string {
 	switch s {
 	case ServerTypeFull:
@@ -189,14 +190,12 @@ type Server struct {
 	mq              models.MessageQueue
 	logstore        models.LogStore
 	nodeType        NodeType
+	tlsConfigs      map[string]*tls.Config
 	// Agent enqueue  and read stores
 	lbEnqueue              agent.EnqueueDataAccess
 	lbReadAccess           agent.ReadDataAccess
 	noHTTTPTriggerEndpoint bool
 	noHybridAPI            bool
-	cert                   string
-	certKey                string
-	certAuthority          string
 	appListeners           *appListeners
 	routeListeners         *routeListeners
 	fnListeners            *fnListeners
@@ -250,9 +249,7 @@ func NewFromEnv(ctx context.Context, opts ...Option) *Server {
 	opts = append(opts, WithLogURL(getEnv(EnvLogDBURL, "")))
 	opts = append(opts, WithRunnerURL(getEnv(EnvRunnerURL, "")))
 	opts = append(opts, WithType(nodeType))
-	opts = append(opts, WithNodeCert(getEnv(EnvCert, "")))
-	opts = append(opts, WithNodeCertKey(getEnv(EnvCertKey, "")))
-	opts = append(opts, WithNodeCertAuthority(getEnv(EnvCertAuth, "")))
+
 	opts = append(opts, LimitRequestBody(int64(getEnvInt(EnvMaxRequestSize, 0))))
 
 	publicLBURL := getEnv(EnvPublicLoadBalancerURL, "")
@@ -386,56 +383,10 @@ func WithType(t NodeType) Option {
 	}
 }
 
-// WithNodeCert maps EnvNodeCert
-func WithNodeCert(cert string) Option {
+// WithTLS configures a service with a provided TLS configuration
+func WithTLS(service string, tlsCfg *tls.Config) Option {
 	return func(ctx context.Context, s *Server) error {
-		if cert != "" {
-			abscert, err := filepath.Abs(cert)
-			if err != nil {
-				return fmt.Errorf("Unable to resolve %v: please specify a valid and readable cert file", cert)
-			}
-			_, err = os.Stat(abscert)
-			if err != nil {
-				return fmt.Errorf("Cannot stat %v: please specify a valid and readable cert file", abscert)
-			}
-			s.cert = abscert
-		}
-		return nil
-	}
-}
-
-// WithNodeCertKey maps EnvNodeCertKey
-func WithNodeCertKey(key string) Option {
-	return func(ctx context.Context, s *Server) error {
-		if key != "" {
-			abskey, err := filepath.Abs(key)
-			if err != nil {
-				return fmt.Errorf("Unable to resolve %v: please specify a valid and readable cert key file", key)
-			}
-			_, err = os.Stat(abskey)
-			if err != nil {
-				return fmt.Errorf("Cannot stat %v: please specify a valid and readable cert key file", abskey)
-			}
-			s.certKey = abskey
-		}
-		return nil
-	}
-}
-
-// WithNodeCertAuthority maps EnvNodeCertAuthority
-func WithNodeCertAuthority(ca string) Option {
-	return func(ctx context.Context, s *Server) error {
-		if ca != "" {
-			absca, err := filepath.Abs(ca)
-			if err != nil {
-				return fmt.Errorf("Unable to resolve %v: please specify a valid and readable cert authority file", ca)
-			}
-			_, err = os.Stat(absca)
-			if err != nil {
-				return fmt.Errorf("Cannot stat %v: please specify a valid and readable cert authority file", absca)
-			}
-			s.certAuthority = absca
-		}
+		s.tlsConfigs[service] = tlsCfg
 		return nil
 	}
 }
@@ -561,7 +512,7 @@ func WithAgentFromEnv() Option {
 			}
 			grpcAddr := fmt.Sprintf(":%d", s.grpcListenPort)
 			cancelCtx, cancel := context.WithCancel(ctx)
-			prAgent, err := agent.DefaultPureRunner(cancel, grpcAddr, ds, s.cert, s.certKey, s.certAuthority)
+			prAgent, err := agent.DefaultPureRunner(cancel, grpcAddr, ds, s.tlsConfigs[TLSGRPCServer])
 			if err != nil {
 				return err
 			}
@@ -653,6 +604,7 @@ func New(ctx context.Context, opts ...Option) *Server {
 		adminListenPort: DefaultPort,
 		grpcListenPort:  DefaultGRPCPort,
 		lbEnqueue:       agent.NewUnsupportedAsyncEnqueueAccess(),
+		tlsConfigs:      make(map[string]*tls.Config),
 		// Almost everything else is configured through opts (see NewFromEnv for ex.) or below
 	}
 
@@ -983,14 +935,20 @@ func (s *Server) startGears(ctx context.Context, cancel context.CancelFunc) {
 	installChildReaper()
 
 	server := http.Server{
-		Addr:    listen,
-		Handler: &ochttp.Handler{Handler: s.Router},
+		Addr:      listen,
+		Handler:   &ochttp.Handler{Handler: s.Router},
+		TLSConfig: s.tlsConfigs[TLSWebServer],
 
 		// TODO we should set read/write timeouts
 	}
 
 	go func() {
-		err := server.ListenAndServe()
+		var err error
+		if server.TLSConfig != nil {
+			err = server.ListenAndServeTLS("", "")
+		} else {
+			err = server.ListenAndServe()
+		}
 		if err != nil && err != http.ErrServerClosed {
 			logrus.WithError(err).Error("server error")
 			cancel()
@@ -1003,12 +961,18 @@ func (s *Server) startGears(ctx context.Context, cancel context.CancelFunc) {
 		adminListen := fmt.Sprintf(":%d", s.adminListenPort)
 		logrus.WithField("type", s.nodeType).Infof("Fn Admin serving on `%v`", adminListen)
 		adminServer := http.Server{
-			Addr:    adminListen,
-			Handler: &ochttp.Handler{Handler: s.AdminRouter},
+			Addr:      adminListen,
+			Handler:   &ochttp.Handler{Handler: s.AdminRouter},
+			TLSConfig: s.tlsConfigs[TLSAdminServer],
 		}
 
 		go func() {
-			err := adminServer.ListenAndServe()
+			var err error
+			if adminServer.TLSConfig != nil {
+				err = adminServer.ListenAndServeTLS("", "")
+			} else {
+				err = adminServer.ListenAndServe()
+			}
 			if err != nil && err != http.ErrServerClosed {
 				logrus.WithError(err).Error("server error")
 				cancel()