From cbfbc797c4a8e22dcd33f04db741089fec898e37 Mon Sep 17 00:00:00 2001 From: Dean Sheather Date: Thu, 22 Sep 2022 06:20:00 +0000 Subject: [PATCH 1/4] feat: support multiple certificates in coder server and helm --- cli/server.go | 94 ++++++++++++++++++++++++------------- helm/templates/NOTES.txt | 8 ++++ helm/templates/_helpers.tpl | 72 +++++++++++++++++++++++++++- helm/templates/coder.yaml | 24 ++-------- helm/values.yaml | 13 +++-- 5 files changed, 152 insertions(+), 59 deletions(-) create mode 100644 helm/templates/NOTES.txt diff --git a/cli/server.go b/cli/server.go index e8b8a1d1a6d19..4de4940ff7cd1 100644 --- a/cli/server.go +++ b/cli/server.go @@ -5,7 +5,6 @@ import ( "crypto/tls" "crypto/x509" "database/sql" - "encoding/pem" "errors" "fmt" "io" @@ -104,11 +103,11 @@ func Server(newAPI func(context.Context, *coderd.Options) (*coderd.API, error)) tailscaleEnable bool telemetryEnable bool telemetryURL string - tlsCertFile string + tlsCertFiles []string tlsClientCAFile string tlsClientAuth string tlsEnable bool - tlsKeyFile string + tlsKeyFiles []string tlsMinVersion string tunnel bool traceEnable bool @@ -210,7 +209,7 @@ func Server(newAPI func(context.Context, *coderd.Options) (*coderd.API, error)) defer listener.Close() if tlsEnable { - listener, err = configureTLS(listener, tlsMinVersion, tlsClientAuth, tlsCertFile, tlsKeyFile, tlsClientCAFile) + listener, err = configureTLS(listener, tlsMinVersion, tlsClientAuth, tlsCertFiles, tlsKeyFiles, tlsClientCAFile) if err != nil { return xerrors.Errorf("configure tls: %w", err) } @@ -817,8 +816,8 @@ func Server(newAPI func(context.Context, *coderd.Options) (*coderd.API, error)) _ = root.Flags().MarkHidden("telemetry-url") cliflag.BoolVarP(root.Flags(), &tlsEnable, "tls-enable", "", "CODER_TLS_ENABLE", false, "Whether TLS will be enabled.") - cliflag.StringVarP(root.Flags(), &tlsCertFile, "tls-cert-file", "", "CODER_TLS_CERT_FILE", "", - "Path to the certificate for TLS. It requires a PEM-encoded file. "+ + cliflag.StringArrayVarP(root.Flags(), &tlsCertFiles, "tls-cert-file", "", "CODER_TLS_CERT_FILE", []string{}, + "Path to each certificate for TLS. It requires a PEM-encoded file. "+ "To configure the listener to use a CA certificate, concatenate the primary certificate "+ "and the CA certificate together. The primary certificate should appear first in the combined file.") cliflag.StringVarP(root.Flags(), &tlsClientCAFile, "tls-client-ca-file", "", "CODER_TLS_CLIENT_CA_FILE", "", @@ -826,8 +825,8 @@ func Server(newAPI func(context.Context, *coderd.Options) (*coderd.API, error)) cliflag.StringVarP(root.Flags(), &tlsClientAuth, "tls-client-auth", "", "CODER_TLS_CLIENT_AUTH", "request", `Policy the server will follow for TLS Client Authentication. `+ `Accepted values are "none", "request", "require-any", "verify-if-given", or "require-and-verify"`) - cliflag.StringVarP(root.Flags(), &tlsKeyFile, "tls-key-file", "", "CODER_TLS_KEY_FILE", "", - "Path to the private key for the certificate. It requires a PEM-encoded file") + cliflag.StringArrayVarP(root.Flags(), &tlsKeyFiles, "tls-key-file", "", "CODER_TLS_KEY_FILE", []string{}, + "Paths to the private keys for each of the certificates. It requires a PEM-encoded file") cliflag.StringVarP(root.Flags(), &tlsMinVersion, "tls-min-version", "", "CODER_TLS_MIN_VERSION", "tls12", `Minimum supported version of TLS. Accepted values are "tls10", "tls11", "tls12" or "tls13"`) cliflag.BoolVarP(root.Flags(), &tunnel, "tunnel", "", "CODER_TUNNEL", false, @@ -1015,7 +1014,32 @@ func printLogo(cmd *cobra.Command, spooky bool) { _, _ = fmt.Fprintf(cmd.OutOrStdout(), "%s - Remote development on your infrastucture\n", cliui.Styles.Bold.Render("Coder "+buildinfo.Version())) } -func configureTLS(listener net.Listener, tlsMinVersion, tlsClientAuth, tlsCertFile, tlsKeyFile, tlsClientCAFile string) (net.Listener, error) { +func loadCertificates(tlsCertFiles, tlsKeyFiles []string) ([]tls.Certificate, error) { + if len(tlsCertFiles) == 0 { + return nil, xerrors.New("tls-cert-file is required when tls is enabled") + } + if len(tlsKeyFiles) == 0 { + return nil, xerrors.New("tls-key-file is required when tls is enabled") + } + if len(tlsCertFiles) != len(tlsKeyFiles) { + return nil, xerrors.New("tls-cert-file and tls-key-file must be used the same amount of times") + } + + certs := make([]tls.Certificate, len(tlsCertFiles)) + for i := range tlsCertFiles { + certFile, keyFile := tlsCertFiles[i], tlsKeyFiles[i] + cert, err := tls.LoadX509KeyPair(certFile, keyFile) + if err != nil { + return nil, xerrors.Errorf("load TLS key pair %d (%q, %q): %w", i, certFile, keyFile, err) + } + + certs[i] = cert + } + + return certs, nil +} + +func configureTLS(listener net.Listener, tlsMinVersion, tlsClientAuth string, tlsCertFiles, tlsKeyFiles []string, tlsClientCAFile string) (net.Listener, error) { tlsConfig := &tls.Config{ MinVersion: tls.VersionTLS12, } @@ -1047,35 +1071,39 @@ func configureTLS(listener net.Listener, tlsMinVersion, tlsClientAuth, tlsCertFi return nil, xerrors.Errorf("unrecognized tls client auth: %q", tlsClientAuth) } - if tlsCertFile == "" { - return nil, xerrors.New("tls-cert-file is required when tls is enabled") - } - if tlsKeyFile == "" { - return nil, xerrors.New("tls-key-file is required when tls is enabled") - } - - certPEMBlock, err := os.ReadFile(tlsCertFile) - if err != nil { - return nil, xerrors.Errorf("read file %q: %w", tlsCertFile, err) - } - keyPEMBlock, err := os.ReadFile(tlsKeyFile) + certs, err := loadCertificates(tlsCertFiles, tlsKeyFiles) if err != nil { - return nil, xerrors.Errorf("read file %q: %w", tlsKeyFile, err) + return nil, xerrors.Errorf("load certificates: %w", err) } - keyBlock, _ := pem.Decode(keyPEMBlock) - if keyBlock == nil { - return nil, xerrors.New("decoded pem is blank") - } - cert, err := tls.X509KeyPair(certPEMBlock, keyPEMBlock) - if err != nil { - return nil, xerrors.Errorf("create key pair: %w", err) - } - tlsConfig.GetCertificate = func(chi *tls.ClientHelloInfo) (*tls.Certificate, error) { - return &cert, nil + tlsConfig.GetCertificate = func(hi *tls.ClientHelloInfo) (*tls.Certificate, error) { + // If there's only one certificate, return it. + if len(certs) == 1 { + return &certs[0], nil + } + + // Expensively check which certificate matches the client hello. + for _, cert := range certs { + cert := cert + if err := hi.SupportsCertificate(&cert); err == nil { + return &cert, nil + } + } + + // Return the first certificate if we have one, or return nil so the server + // doesn't fail. + if len(certs) > 0 { + return &certs[0], nil + } + return nil, nil //nolint:nilnil } + // Append all certs to the RootCAs list. certPool := x509.NewCertPool() - certPool.AppendCertsFromPEM(certPEMBlock) + for _, c := range certs { + for _, pemBytes := range c.Certificate { + certPool.AppendCertsFromPEM(pemBytes) + } + } tlsConfig.RootCAs = certPool if tlsClientCAFile != "" { diff --git a/helm/templates/NOTES.txt b/helm/templates/NOTES.txt new file mode 100644 index 0000000000000..38b987acbd45b --- /dev/null +++ b/helm/templates/NOTES.txt @@ -0,0 +1,8 @@ +{{- if .Values.coder.tls.secretName }} + +WARN: coder.tls.secretName is deprecated and will be removed in a future + release. Please use coder.tls.secretNames instead. +{{- end }} + +Enjoy Coder! Please create an issue at https://github.com/coder/coder if you run +into any problems! :) diff --git a/helm/templates/_helpers.tpl b/helm/templates/_helpers.tpl index 1dbb6a9647acf..a5960647f03e8 100644 --- a/helm/templates/_helpers.tpl +++ b/helm/templates/_helpers.tpl @@ -37,7 +37,7 @@ Coder Docker image URI */}} {{- define "coder.image" -}} {{- if and (eq .Values.coder.image.tag "") (eq .Chart.AppVersion "0.1.0") -}} -{{ fail "You must specify coder.image.tag if you're installing the Helm chart directly from Git." }} +{{ fail "You must specify the coder.image.tag value if you're installing the Helm chart directly from Git." }} {{- end -}} {{ .Values.coder.image.repo }}:{{ .Values.coder.image.tag | default (printf "v%v" .Chart.AppVersion) }} {{- end }} @@ -81,3 +81,73 @@ Scheme {{- define "coder.scheme" }} {{- include "coder.portName" . | upper -}} {{- end }} + +{{/* +Coder volume definitions. +*/}} +{{- define "coder.volumes" }} +{{- if or .Values.coder.tls.secretNames .Values.coder.tls.secretName }} +volumes: +{{ range $secretName := .Values.coder.tls.secretNames -}} +- name: "tls-{{ $secretName }}" + secret: + secretName: {{ $secretName | quote }} +{{ end -}} +{{- if .Values.coder.tls.secretName -}} +- name: "tls-{{ .Values.coder.tls.secretName }}" + secret: + secretName: {{ .Values.coder.tls.secretName | quote }} +{{- end }} +{{- else }} +volumes: {{ if and (not .Values.coder.tls.secretNames) (not .Values.coder.tls.secretName) }}[]{{ end }} +{{- end }} +{{- end }} + +{{/* +Coder volume mounts. +*/}} +{{- define "coder.volumeMounts" }} +{{- if or .Values.coder.tls.secretNames .Values.coder.tls.secretName }} +volumeMounts: +{{ range $secretName := .Values.coder.tls.secretNames -}} +- name: "tls-{{ $secretName }}" + mountPath: "/etc/ssl/certs/coder/{{ $secretName }}" + readOnly: true +{{ end }} +{{- if .Values.coder.tls.secretName -}} +- name: "tls-{{ .Values.coder.tls.secretName }}" + mountPath: "/etc/ssl/certs/coder/{{ .Values.coder.tls.secretName }}" + readOnly: true +{{- end }} +{{- else }} +volumeMounts: [] +{{- end }} +{{- end }} + +{{/* +Coder TLS environment variables. +*/}} +{{- define "coder.tlsEnv" }} +{{- if or .Values.coder.tls.secretNames .Values.coder.tls.secretName }} +- name: CODER_TLS_ENABLE + value: "true" +- name: CODER_TLS_CERT_FIlE + value: "{{ range $idx, $secretName := .Values.coder.tls.secretNames -}}{{ if $idx }},{{ end }}/etc/ssl/certs/coder/{{ $secretName }}/tls.crt{{- end }}{{ if .Values.coder.tls.secretName -}}/etc/ssl/certs/coder/{{ .Values.coder.tls.secretName }}/tls.crt{{- end }}" +- name: CODER_TLS_KEY_FILE + value: "{{ range $idx, $secretName := .Values.coder.tls.secretNames -}}{{ if $idx }},{{ end }}/etc/ssl/certs/coder/{{ $secretName }}/tls.key{{- end }}{{ if .Values.coder.tls.secretName -}}/etc/ssl/certs/coder/{{ .Values.coder.tls.secretName }}/tls.key{{- end }}" +{{- end }} +{{- end }} + +{{/* +Fail on fully deprecated values or deprecated value combinations. This is +included at the top of coder.yaml. +*/}} +{{- define "coder.verifyDeprecated" }} +{{/* +Deprecated value coder.tls.secretName should not be used alongside new value +coder.tls.secretName. +*/}} +{{- if and .Values.coder.tls.secretName .Values.coder.tls.secretNames }} +{{ fail "You must specify either coder.tls.secretName or coder.tls.secretNames, not both." }} +{{- end }} +{{- end }} diff --git a/helm/templates/coder.yaml b/helm/templates/coder.yaml index 06d455761eab0..71ea15ea18686 100644 --- a/helm/templates/coder.yaml +++ b/helm/templates/coder.yaml @@ -1,3 +1,4 @@ +{{- include "coder.verifyDeprecated" . -}} --- apiVersion: v1 kind: ServiceAccount @@ -36,14 +37,7 @@ spec: env: - name: CODER_ADDRESS value: "0.0.0.0:{{ include "coder.port" . }}" - {{- if .Values.coder.tls.secretName }} - - name: CODER_TLS_ENABLE - value: "true" - - name: CODER_TLS_CERT_FILE - value: /etc/ssl/certs/coder/tls.crt - - name: CODER_TLS_KEY_FILE - value: /etc/ssl/certs/coder/tls.key - {{- end }} + {{- include "coder.tlsEnv" . | nindent 12 }} {{- with .Values.coder.env -}} {{ toYaml . | nindent 12 }} {{- end }} @@ -61,16 +55,6 @@ spec: path: /api/v2/buildinfo port: {{ include "coder.portName" . | quote }} scheme: {{ include "coder.scheme" . | quote }} - {{- if .Values.coder.tls.secretName }} - volumeMounts: - - name: tls - mountPath: /etc/ssl/certs/coder - readOnly: true - {{- end }} + {{- include "coder.volumeMounts" . | nindent 10 }} - {{- if .Values.coder.tls.secretName }} - volumes: - - name: tls - secret: - secretName: {{ .Values.coder.tls.secretName | quote }} - {{- end }} + {{- include "coder.volumes" . | nindent 6 }} diff --git a/helm/values.yaml b/helm/values.yaml index 23ca6cb495daa..317c78829460a 100644 --- a/helm/values.yaml +++ b/helm/values.yaml @@ -43,12 +43,15 @@ coder: # coder.tls -- The TLS configuration for Coder. tls: - # coder.tls.secretName -- The name of the secret containing the TLS - # certificate. The secret should exist in the same namespace as the Helm - # deployment and should be of type "kubernetes.io/tls". The secret will be - # automatically mounted into the pod if specified, and the correct + # coder.tls.secrets -- A list of TLS server certificate secrets to mount + # into the Coder pod. The secrets should exist in the same namespace as the + # Helm deployment and should be of type "kubernetes.io/tls". The secrets + # will be automatically mounted into the pod if specified, and the correct # "CODER_TLS_*" environment variables will be set for you. - secretName: "" + secretNames: [] + # coder.tls.secretName -- Deprecated. Use `coder.tls.secretNames` instead. + # This will be removed in a future release. + # secretName: "" # coder.resources -- The resources to request for Coder. These are optional # and are not set by default. From 2d2df29053b5ba68b1fe79c5625126f649ba53c0 Mon Sep 17 00:00:00 2001 From: Dean Sheather Date: Thu, 29 Sep 2022 08:46:11 +0000 Subject: [PATCH 2/4] chore: multi-cert tests for coder server --- cli/server.go | 14 ++-- cli/server_test.go | 158 +++++++++++++++++++++++++++++++++++++++++---- helm/values.yaml | 2 +- 3 files changed, 153 insertions(+), 21 deletions(-) diff --git a/cli/server.go b/cli/server.go index 4de4940ff7cd1..7c43e73f31467 100644 --- a/cli/server.go +++ b/cli/server.go @@ -1015,14 +1015,14 @@ func printLogo(cmd *cobra.Command, spooky bool) { } func loadCertificates(tlsCertFiles, tlsKeyFiles []string) ([]tls.Certificate, error) { + if len(tlsCertFiles) != len(tlsKeyFiles) { + return nil, xerrors.New("--tls-cert-file and --tls-key-file must be used the same amount of times") + } if len(tlsCertFiles) == 0 { - return nil, xerrors.New("tls-cert-file is required when tls is enabled") + return nil, xerrors.New("--tls-cert-file is required when tls is enabled") } if len(tlsKeyFiles) == 0 { - return nil, xerrors.New("tls-key-file is required when tls is enabled") - } - if len(tlsCertFiles) != len(tlsKeyFiles) { - return nil, xerrors.New("tls-cert-file and tls-key-file must be used the same amount of times") + return nil, xerrors.New("--tls-key-file is required when tls is enabled") } certs := make([]tls.Certificate, len(tlsCertFiles)) @@ -1089,8 +1089,8 @@ func configureTLS(listener net.Listener, tlsMinVersion, tlsClientAuth string, tl } } - // Return the first certificate if we have one, or return nil so the server - // doesn't fail. + // Return the first certificate if we have one, or return nil so the + // server doesn't fail. if len(certs) > 0 { return &certs[0], nil } diff --git a/cli/server_test.go b/cli/server_test.go index c29f4c27274ae..6f4192f42ca8e 100644 --- a/cli/server_test.go +++ b/cli/server_test.go @@ -21,6 +21,7 @@ import ( "runtime" "strconv" "strings" + "sync/atomic" "testing" "time" @@ -240,20 +241,64 @@ func TestServer(t *testing.T) { err := root.ExecuteContext(ctx) require.Error(t, err) }) - t.Run("TLSNoCertFile", func(t *testing.T) { + t.Run("TLSInvalid", func(t *testing.T) { t.Parallel() - ctx, cancelFunc := context.WithCancel(context.Background()) - defer cancelFunc() - root, _ := clitest.New(t, - "server", - "--in-memory", - "--address", ":0", - "--tls-enable", - "--cache-dir", t.TempDir(), - ) - err := root.ExecuteContext(ctx) - require.Error(t, err) + cert1Path, key1Path := generateTLSCertificate(t) + cert2Path, key2Path := generateTLSCertificate(t) + + cases := []struct { + name string + args []string + errContains string + }{ + { + name: "NoCertAndKey", + args: []string{"--tls-enable"}, + errContains: "--tls-cert-file is required when tls is enabled", + }, + { + name: "NoCert", + args: []string{"--tls-enable", "--tls-key-file", key1Path}, + errContains: "--tls-cert-file and --tls-key-file must be used the same amount of times", + }, + { + name: "NoKey", + args: []string{"--tls-enable", "--tls-cert-file", cert1Path}, + errContains: "--tls-cert-file and --tls-key-file must be used the same amount of times", + }, + { + name: "MismatchedCount", + args: []string{"--tls-enable", "--tls-cert-file", cert1Path, "--tls-key-file", key1Path, "--tls-cert-file", cert2Path}, + errContains: "--tls-cert-file and --tls-key-file must be used the same amount of times", + }, + { + name: "MismatchedCertAndKey", + args: []string{"--tls-enable", "--tls-cert-file", cert1Path, "--tls-key-file", key2Path}, + errContains: "load TLS key pair", + }, + } + + for _, c := range cases { + c := c + t.Run(c.name, func(t *testing.T) { + t.Parallel() + ctx, cancelFunc := context.WithCancel(context.Background()) + defer cancelFunc() + + args := []string{ + "server", + "--in-memory", + "--address", ":0", + "--cache-dir", t.TempDir(), + } + args = append(args, c.args...) + root, _ := clitest.New(t, args...) + err := root.ExecuteContext(ctx) + require.Error(t, err) + require.ErrorContains(t, err, c.errContains) + }) + } }) t.Run("TLSValid", func(t *testing.T) { t.Parallel() @@ -293,6 +338,86 @@ func TestServer(t *testing.T) { cancelFunc() require.NoError(t, <-errC) }) + t.Run("TLSValidMultiple", func(t *testing.T) { + t.Parallel() + ctx, cancelFunc := context.WithCancel(context.Background()) + defer cancelFunc() + + cert1Path, key1Path := generateTLSCertificate(t, "alpaca.com") + cert2Path, key2Path := generateTLSCertificate(t, "*.llama.com") + root, cfg := clitest.New(t, + "server", + "--in-memory", + "--address", ":0", + "--tls-enable", + "--tls-cert-file", cert1Path, + "--tls-key-file", key1Path, + "--tls-cert-file", cert2Path, + "--tls-key-file", key2Path, + "--cache-dir", t.TempDir(), + ) + errC := make(chan error, 1) + go func() { + errC <- root.ExecuteContext(ctx) + }() + accessURL := waitAccessURL(t, cfg) + require.Equal(t, "https", accessURL.Scheme) + originalHost := accessURL.Host + + var ( + expectAddr string + dials int64 + ) + client := codersdk.New(accessURL) + client.HTTPClient = &http.Client{ + Transport: &http.Transport{ + DialTLSContext: func(ctx context.Context, network, addr string) (net.Conn, error) { + atomic.AddInt64(&dials, 1) + assert.Equal(t, expectAddr, addr) + + host, _, err := net.SplitHostPort(addr) + require.NoError(t, err) + + // Always connect to the accessURL ip:port regardless of + // hostname. + conn, err := tls.Dial(network, originalHost, &tls.Config{ + MinVersion: tls.VersionTLS12, + //nolint:gosec + InsecureSkipVerify: true, + ServerName: host, + }) + if err != nil { + return nil, err + } + + // We can't call conn.VerifyHostname because it requires + // that the certificates are valid, so we call + // VerifyHostname on the first certificate instead. + require.Len(t, conn.ConnectionState().PeerCertificates, 1) + err = conn.ConnectionState().PeerCertificates[0].VerifyHostname(host) + assert.NoError(t, err, "invalid cert common name") + return conn, nil + }, + }, + } + + // Use the first certificate and hostname. + client.URL.Host = "alpaca.com:443" + expectAddr = "alpaca.com:443" + _, err := client.HasFirstUser(ctx) + require.NoError(t, err) + require.EqualValues(t, 1, atomic.LoadInt64(&dials)) + + // Use the second certificate (wildcard) and hostname. + client.URL.Host = "hi.llama.com:443" + expectAddr = "hi.llama.com:443" + _, err = client.HasFirstUser(ctx) + require.NoError(t, err) + require.EqualValues(t, 2, atomic.LoadInt64(&dials)) + + cancelFunc() + require.NoError(t, <-errC) + }) // This cannot be ran in parallel because it uses a signal. //nolint:paralleltest t.Run("Shutdown", func(t *testing.T) { @@ -480,16 +605,22 @@ func TestServer(t *testing.T) { }) } -func generateTLSCertificate(t testing.TB) (certPath, keyPath string) { +func generateTLSCertificate(t testing.TB, commonName ...string) (certPath, keyPath string) { dir := t.TempDir() + commonNameStr := "localhost" + if len(commonName) > 0 { + commonNameStr = commonName[0] + } privateKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) require.NoError(t, err) template := x509.Certificate{ SerialNumber: big.NewInt(1), Subject: pkix.Name{ Organization: []string{"Acme Co"}, + CommonName: commonNameStr, }, + DNSNames: []string{commonNameStr}, NotBefore: time.Now(), NotAfter: time.Now().Add(time.Hour * 24 * 180), @@ -498,6 +629,7 @@ func generateTLSCertificate(t testing.TB) (certPath, keyPath string) { BasicConstraintsValid: true, IPAddresses: []net.IP{net.ParseIP("127.0.0.1")}, } + derBytes, err := x509.CreateCertificate(rand.Reader, &template, &template, &privateKey.PublicKey, privateKey) require.NoError(t, err) certFile, err := os.CreateTemp(dir, "") diff --git a/helm/values.yaml b/helm/values.yaml index 317c78829460a..4b58b32bdad5a 100644 --- a/helm/values.yaml +++ b/helm/values.yaml @@ -43,7 +43,7 @@ coder: # coder.tls -- The TLS configuration for Coder. tls: - # coder.tls.secrets -- A list of TLS server certificate secrets to mount + # coder.tls.secretNames -- A list of TLS server certificate secrets to mount # into the Coder pod. The secrets should exist in the same namespace as the # Helm deployment and should be of type "kubernetes.io/tls". The secrets # will be automatically mounted into the pod if specified, and the correct From da652ab21422e8e7f2cdff7241d6d9a3b53428a3 Mon Sep 17 00:00:00 2001 From: Dean Sheather Date: Thu, 29 Sep 2022 14:06:45 +0000 Subject: [PATCH 3/4] fixup! chore: multi-cert tests for coder server --- cli/server.go | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/cli/server.go b/cli/server.go index 7c43e73f31467..e7f0143dd35c7 100644 --- a/cli/server.go +++ b/cli/server.go @@ -209,7 +209,7 @@ func Server(newAPI func(context.Context, *coderd.Options) (*coderd.API, error)) defer listener.Close() if tlsEnable { - listener, err = configureTLS(listener, tlsMinVersion, tlsClientAuth, tlsCertFiles, tlsKeyFiles, tlsClientCAFile) + listener, err = configureServerTLS(listener, tlsMinVersion, tlsClientAuth, tlsCertFiles, tlsKeyFiles, tlsClientCAFile) if err != nil { return xerrors.Errorf("configure tls: %w", err) } @@ -1039,7 +1039,7 @@ func loadCertificates(tlsCertFiles, tlsKeyFiles []string) ([]tls.Certificate, er return certs, nil } -func configureTLS(listener net.Listener, tlsMinVersion, tlsClientAuth string, tlsCertFiles, tlsKeyFiles []string, tlsClientCAFile string) (net.Listener, error) { +func configureServerTLS(listener net.Listener, tlsMinVersion, tlsClientAuth string, tlsCertFiles, tlsKeyFiles []string, tlsClientCAFile string) (net.Listener, error) { tlsConfig := &tls.Config{ MinVersion: tls.VersionTLS12, } @@ -1097,15 +1097,6 @@ func configureTLS(listener net.Listener, tlsMinVersion, tlsClientAuth string, tl return nil, nil //nolint:nilnil } - // Append all certs to the RootCAs list. - certPool := x509.NewCertPool() - for _, c := range certs { - for _, pemBytes := range c.Certificate { - certPool.AppendCertsFromPEM(pemBytes) - } - } - tlsConfig.RootCAs = certPool - if tlsClientCAFile != "" { caPool := x509.NewCertPool() data, err := os.ReadFile(tlsClientCAFile) From d4f54486c70ba99f78869e3f7d281320970bc62b Mon Sep 17 00:00:00 2001 From: Dean Sheather Date: Sat, 1 Oct 2022 10:48:11 +0000 Subject: [PATCH 4/4] fixup! chore: multi-cert tests for coder server --- helm/templates/_helpers.tpl | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/helm/templates/_helpers.tpl b/helm/templates/_helpers.tpl index a5960647f03e8..95ba09a88692c 100644 --- a/helm/templates/_helpers.tpl +++ b/helm/templates/_helpers.tpl @@ -46,7 +46,7 @@ Coder Docker image URI Coder listen port (must be > 1024) */}} {{- define "coder.port" }} -{{- if .Values.coder.tls.secretName -}} +{{- if or .Values.coder.tls.secretNames .Values.coder.tls.secretName -}} 8443 {{- else -}} 8080 @@ -57,7 +57,7 @@ Coder listen port (must be > 1024) Coder service port */}} {{- define "coder.servicePort" }} -{{- if .Values.coder.tls.secretName -}} +{{- if or .Values.coder.tls.secretNames .Values.coder.tls.secretName -}} 443 {{- else -}} 80 @@ -68,7 +68,7 @@ Coder service port Port name */}} {{- define "coder.portName" }} -{{- if .Values.coder.tls.secretName -}} +{{- if or .Values.coder.tls.secretNames .Values.coder.tls.secretName -}} https {{- else -}} http @@ -131,7 +131,7 @@ Coder TLS environment variables. {{- if or .Values.coder.tls.secretNames .Values.coder.tls.secretName }} - name: CODER_TLS_ENABLE value: "true" -- name: CODER_TLS_CERT_FIlE +- name: CODER_TLS_CERT_FILE value: "{{ range $idx, $secretName := .Values.coder.tls.secretNames -}}{{ if $idx }},{{ end }}/etc/ssl/certs/coder/{{ $secretName }}/tls.crt{{- end }}{{ if .Values.coder.tls.secretName -}}/etc/ssl/certs/coder/{{ .Values.coder.tls.secretName }}/tls.crt{{- end }}" - name: CODER_TLS_KEY_FILE value: "{{ range $idx, $secretName := .Values.coder.tls.secretNames -}}{{ if $idx }},{{ end }}/etc/ssl/certs/coder/{{ $secretName }}/tls.key{{- end }}{{ if .Values.coder.tls.secretName -}}/etc/ssl/certs/coder/{{ .Values.coder.tls.secretName }}/tls.key{{- end }}"