feat: support multiple certificates in coder server and helm (#4150)

This commit is contained in:
Dean Sheather 2022-10-04 21:45:21 +10:00 committed by GitHub
parent a1056bfa2a
commit 6325a9ea91
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 294 additions and 78 deletions

View File

@ -5,7 +5,6 @@ import (
"crypto/tls"
"crypto/x509"
"database/sql"
"encoding/pem"
"errors"
"fmt"
"io"
@ -106,11 +105,11 @@ func Server(newAPI func(context.Context, *coderd.Options) (*coderd.API, error))
telemetryEnable bool
telemetryTraceEnable bool
telemetryURL string
tlsCertFile string
tlsCertFiles []string
tlsClientCAFile string
tlsClientAuth string
tlsEnable bool
tlsKeyFile string
tlsKeyFiles []string
tlsMinVersion string
tunnel bool
traceEnable bool
@ -221,7 +220,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 = configureServerTLS(listener, tlsMinVersion, tlsClientAuth, tlsCertFiles, tlsKeyFiles, tlsClientCAFile)
if err != nil {
return xerrors.Errorf("configure tls: %w", err)
}
@ -842,8 +841,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", "",
@ -851,8 +850,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,
@ -1040,7 +1039,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) != 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")
}
if len(tlsKeyFiles) == 0 {
return nil, xerrors.New("--tls-key-file is required when tls is enabled")
}
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 configureServerTLS(listener net.Listener, tlsMinVersion, tlsClientAuth string, tlsCertFiles, tlsKeyFiles []string, tlsClientCAFile string) (net.Listener, error) {
tlsConfig := &tls.Config{
MinVersion: tls.VersionTLS12,
}
@ -1072,36 +1096,31 @@ 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")
certs, err := loadCertificates(tlsCertFiles, tlsKeyFiles)
if err != nil {
return nil, xerrors.Errorf("load certificates: %w", err)
}
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
}
certPEMBlock, err := os.ReadFile(tlsCertFile)
if err != nil {
return nil, xerrors.Errorf("read file %q: %w", tlsCertFile, err)
}
keyPEMBlock, err := os.ReadFile(tlsKeyFile)
if err != nil {
return nil, xerrors.Errorf("read file %q: %w", tlsKeyFile, 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
}
// Expensively check which certificate matches the client hello.
for _, cert := range certs {
cert := cert
if err := hi.SupportsCertificate(&cert); err == nil {
return &cert, nil
}
}
certPool := x509.NewCertPool()
certPool.AppendCertsFromPEM(certPEMBlock)
tlsConfig.RootCAs = certPool
// 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
}
if tlsClientCAFile != "" {
caPool := x509.NewCertPool()

View File

@ -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, "")

8
helm/templates/NOTES.txt Normal file
View File

@ -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! :)

View File

@ -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 }}
@ -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
@ -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 }}

View File

@ -1,3 +1,4 @@
{{- include "coder.verifyDeprecated" . -}}
---
apiVersion: v1
kind: ServiceAccount
@ -37,14 +38,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 }}
@ -62,16 +56,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 }}

View File

@ -47,12 +47,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.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
# "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.