From 478d794693b3a965e3ba587da2c67e5e1effa418 Mon Sep 17 00:00:00 2001 From: David Collom Date: Tue, 17 Aug 2021 21:47:27 +0100 Subject: [PATCH] fix: Generate TLS Certificates on startup and only keep in memory (#6540) Signed-off-by: David Collom --- .gitignore | 2 - Dockerfile | 2 - Makefile | 9 +-- cmd/argo/commands/server.go | 16 ++--- cmd/argo/commands/server_test.go | 16 +---- util/tls/tls.go | 102 +++++++++++++++++++++++++++++++ util/tls/tls_test.go | 39 ++++++++++++ 7 files changed, 152 insertions(+), 34 deletions(-) create mode 100644 util/tls/tls.go create mode 100644 util/tls/tls_test.go diff --git a/.gitignore b/.gitignore index d879edd26677..fe396556a4af 100644 --- a/.gitignore +++ b/.gitignore @@ -20,8 +20,6 @@ git-ask-pass.sh /workflow-controller /.scannerwork/ /test-results/ -/argo-server.crt -/argo-server.key /package-lock.json /pkg/apiclient/_.secondary.swagger.json /pkg/apiclient/clusterworkflowtemplate/cluster-workflow-template.swagger.json diff --git a/Dockerfile b/Dockerfile index 0800cd693073..3d3a1e6b89ed 100644 --- a/Dockerfile +++ b/Dockerfile @@ -146,8 +146,6 @@ WORKDIR /home/argo COPY hack/ssh_known_hosts /etc/ssh/ COPY hack/nsswitch.conf /etc/ COPY --from=argocli-build /etc/ssl/certs/ca-certificates.crt /etc/ssl/certs/ -COPY --from=argocli-build --chown=8737 /go/src/github.com/argoproj/argo-workflows/argo-server.crt /home/argo/ -COPY --from=argocli-build --chown=8737 /go/src/github.com/argoproj/argo-workflows/argo-server.key /home/argo/ COPY --from=argocli-build /go/src/github.com/argoproj/argo-workflows/dist/argo /bin/ ENTRYPOINT [ "argo" ] diff --git a/Makefile b/Makefile index a952997a0a34..0f3480fda9ee 100644 --- a/Makefile +++ b/Makefile @@ -174,10 +174,10 @@ dist/argo-linux-s390x: GOARGS = GOOS=linux GOARCH=s390x dist/argo-%.gz: dist/argo-% gzip --force --keep dist/argo-$* -dist/argo-%: server/static/files.go argo-server.crt argo-server.key $(CLI_PKGS) go.sum +dist/argo-%: server/static/files.go $(CLI_PKGS) go.sum CGO_ENABLED=0 $(GOARGS) go build -v -ldflags '${LDFLAGS} -extldflags -static' -o $@ ./cmd/argo -dist/argo: server/static/files.go argo-server.crt argo-server.key $(CLI_PKGS) go.sum +dist/argo: server/static/files.go $(CLI_PKGS) go.sum ifeq ($(shell uname -s),Darwin) # if local, then build fast: use CGO and dynamic-linking go build -v -ldflags '${LDFLAGS}' -o $@ ./cmd/argo @@ -185,11 +185,6 @@ else CGO_ENABLED=0 go build -v -ldflags '${LDFLAGS} -extldflags -static' -o $@ ./cmd/argo endif -argo-server.crt: argo-server.key - -argo-server.key: - openssl req -x509 -newkey rsa:4096 -keyout argo-server.key -out argo-server.crt -days 365 -nodes -subj /CN=localhost/O=ArgoProj - argocli-image: .PHONY: clis diff --git a/cmd/argo/commands/server.go b/cmd/argo/commands/server.go index f8f1cc2ac899..f96b59b00ca8 100644 --- a/cmd/argo/commands/server.go +++ b/cmd/argo/commands/server.go @@ -31,6 +31,7 @@ import ( "github.com/argoproj/argo-workflows/v3/server/types" "github.com/argoproj/argo-workflows/v3/util/cmd" "github.com/argoproj/argo-workflows/v3/util/help" + tlsutils "github.com/argoproj/argo-workflows/v3/util/tls" ) func NewServerCommand() *cobra.Command { @@ -99,18 +100,19 @@ See %s`, help.ArgoServer), var tlsConfig *tls.Config if secure { - cer, err := tls.LoadX509KeyPair("argo-server.crt", "argo-server.key") + log.Infof("Generating Self Signed TLS Certificates for Secure Mode") + tlsMinVersion, err := env.GetInt("TLS_MIN_VERSION", tls.VersionTLS12) if err != nil { return err } - tlsMinVersion, err := env.GetInt("TLS_MIN_VERSION", tls.VersionTLS12) + var cer *tls.Certificate + cer, err = tlsutils.GenerateX509KeyPair() if err != nil { return err } tlsConfig = &tls.Config{ - Certificates: []tls.Certificate{cer}, - InsecureSkipVerify: true, - MinVersion: uint16(tlsMinVersion), + Certificates: []tls.Certificate{*cer}, + MinVersion: uint16(tlsMinVersion), } } else { log.Warn("You are running in insecure mode. Learn how to enable transport layer security: https://argoproj.github.io/argo-workflows/tls/") @@ -182,9 +184,7 @@ See %s`, help.ArgoServer), } command.Flags().StringVar(&baseHRef, "basehref", defaultBaseHRef, "Value for base href in index.html. Used if the server is running behind reverse proxy under subpath different from /. Defaults to the environment variable BASE_HREF.") // "-e" for encrypt, like zip - // We default to secure mode if we find certs available, otherwise we default to insecure mode. - _, err := os.Stat("argo-server.crt") - command.Flags().BoolVarP(&secure, "secure", "e", !os.IsNotExist(err), "Whether or not we should listen on TLS.") + command.Flags().BoolVarP(&secure, "secure", "e", true, "Whether or not we should listen on TLS.") command.Flags().BoolVar(&htst, "hsts", true, "Whether or not we should add a HTTP Secure Transport Security header. This only has effect if secure is enabled.") command.Flags().StringArrayVar(&authModes, "auth-mode", []string{"client"}, "API server authentication mode. Any 1 or more length permutation of: client,server,sso") command.Flags().StringVar(&configMap, "configmap", "workflow-controller-configmap", "Name of K8s configmap to retrieve workflow controller configuration") diff --git a/cmd/argo/commands/server_test.go b/cmd/argo/commands/server_test.go index 965322ed6b9b..2aa3dc70cd6d 100644 --- a/cmd/argo/commands/server_test.go +++ b/cmd/argo/commands/server_test.go @@ -1,27 +1,13 @@ package commands import ( - "os" "testing" "github.com/stretchr/testify/assert" ) func TestDefaultSecureMode(t *testing.T) { - // No certs: We should run insecure + // Secure mode by default cmd := NewServerCommand() - assert.Equal(t, "false", cmd.Flag("secure").Value.String()) - - // Clean up and delete tests files - defer func() { - _ = os.Remove("argo-server.crt") - _ = os.Remove("argo-server.key") - }() - - _, _ = os.Create("argo-server.crt") - _, _ = os.Create("argo-server.key") - - // No certs: We should secure - cmd = NewServerCommand() assert.Equal(t, "true", cmd.Flag("secure").Value.String()) } diff --git a/util/tls/tls.go b/util/tls/tls.go new file mode 100644 index 000000000000..227b04e2c9c7 --- /dev/null +++ b/util/tls/tls.go @@ -0,0 +1,102 @@ +package tls + +import ( + "crypto" + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" + "crypto/tls" + "crypto/x509" + "crypto/x509/pkix" + "encoding/pem" + "fmt" + "log" + "math/big" + "net" + "os" + "time" +) + +func pemBlockForKey(priv interface{}) *pem.Block { + switch k := priv.(type) { + case *ecdsa.PrivateKey: + b, err := x509.MarshalECPrivateKey(k) + if err != nil { + log.Fatal(err) + os.Exit(2) + } + return &pem.Block{Type: "EC PRIVATE KEY", Bytes: b} + default: + return nil + } +} + +func generate() ([]byte, crypto.PrivateKey, error) { + hosts := []string{"localhost"} + + var err error + privateKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + if err != nil { + return nil, nil, fmt.Errorf("failed to generate private key: %s", err) + } + + notBefore := time.Now() + notAfter := notBefore.Add(365 * 24 * time.Hour) + + serialNumberLimit := new(big.Int).Lsh(big.NewInt(1), 128) + serialNumber, err := rand.Int(rand.Reader, serialNumberLimit) + if err != nil { + return nil, nil, fmt.Errorf("failed to generate serial number: %s", err) + } + + template := x509.Certificate{ + SerialNumber: serialNumber, + Subject: pkix.Name{ + Organization: []string{"ArgoProj"}, + }, + NotBefore: notBefore, + NotAfter: notAfter, + + KeyUsage: x509.KeyUsageKeyEncipherment | x509.KeyUsageDigitalSignature, + ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth}, + BasicConstraintsValid: true, + } + + for _, h := range hosts { + if ip := net.ParseIP(h); ip != nil { + template.IPAddresses = append(template.IPAddresses, ip) + } else { + template.DNSNames = append(template.DNSNames, h) + } + } + + certBytes, err := x509.CreateCertificate(rand.Reader, &template, &template, &privateKey.PublicKey, privateKey) + if err != nil { + return nil, nil, fmt.Errorf("failed to create certificate: %s", err) + } + return certBytes, privateKey, nil +} + +// generatePEM generates a new certificate and key and returns it as PEM encoded bytes +func generatePEM() ([]byte, []byte, error) { + certBytes, privateKey, err := generate() + if err != nil { + return nil, nil, err + } + certpem := pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: certBytes}) + keypem := pem.EncodeToMemory(pemBlockForKey(privateKey)) + return certpem, keypem, nil +} + +// GenerateX509KeyPair generates a X509 key pair +func GenerateX509KeyPair() (*tls.Certificate, error) { + certpem, keypem, err := generatePEM() + if err != nil { + return nil, err + } + cert, err := tls.X509KeyPair(certpem, keypem) + if err != nil { + return nil, err + } + return &cert, nil +} diff --git a/util/tls/tls_test.go b/util/tls/tls_test.go new file mode 100644 index 000000000000..834a9f7d3e5d --- /dev/null +++ b/util/tls/tls_test.go @@ -0,0 +1,39 @@ +package tls + +import ( + "crypto/x509" + "testing" + "time" + + "github.com/stretchr/testify/assert" +) + +func TestGenerate(t *testing.T) { + t.Run("Create certificate with default options", func(t *testing.T) { + certBytes, privKey, err := generate() + assert.NoError(t, err) + assert.NotNil(t, privKey) + cert, err := x509.ParseCertificate(certBytes) + assert.NoError(t, err) + assert.NotNil(t, cert) + assert.Len(t, cert.DNSNames, 1) + assert.Equal(t, "localhost", cert.DNSNames[0]) + assert.Empty(t, cert.IPAddresses) + assert.LessOrEqual(t, int64(time.Since(cert.NotBefore)), int64(10*time.Second)) + }) +} + +func TestGeneratePEM(t *testing.T) { + t.Run("Create PEM from certficate options", func(t *testing.T) { + cert, key, err := generatePEM() + assert.NoError(t, err) + assert.NotNil(t, cert) + assert.NotNil(t, key) + }) + + t.Run("Create X509KeyPair", func(t *testing.T) { + cert, err := GenerateX509KeyPair() + assert.NoError(t, err) + assert.NotNil(t, cert) + }) +}