Skip to content

Commit

Permalink
require a TLS client certificate by default
Browse files Browse the repository at this point in the history
This commit changes the `ClientAuth` type from
`RequestClientCert` to `RequireAnyClientCert` by default.

In general, a KES server should demand a client certificate.
Otherwise, a client (the HTTP/TLS stack) may choose to not
send a client certificate - even if one is available. For example,
the HTTP stack may try to be smart and not send a client certificate
if it determines that cannot be validated since its self-signed.

Instead, the KES server's TLS should abort the handshake if the client
does not send a certificate. However, in some cases we cannot enforce
this. In particular, when some APIs should be accessible without TLS
authentication, like `/v1/metrics`. In these cases, we have to make
it optional for clients to send a certificate. However, disabling
auth for some APIs is an advanced use case intended only for users
who are aware of the implications.

Signed-off-by: Andreas Auernhammer <[email protected]>
  • Loading branch information
aead committed Mar 13, 2024
1 parent ca2ed00 commit 55e9e7d
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 3 deletions.
2 changes: 1 addition & 1 deletion cmd/kes/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -336,7 +336,7 @@ func startDevServer(addr string) error {
MinVersion: tls.VersionTLS12,
NextProtos: []string{"h2", "http/1.1"},
Certificates: []tls.Certificate{srvCert},
ClientAuth: tls.RequestClientCert,
ClientAuth: tls.RequireAnyClientCert,
}

ctx, cancel := signal.NotifyContext(context.Background(), syscall.SIGINT, syscall.SIGTERM)
Expand Down
18 changes: 16 additions & 2 deletions kesconf/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -255,11 +255,11 @@ func ymlToServerConfig(y *ymlFile) (*File, error) {
return nil, errors.New("kesconf: invalid tls config: no certificate")
}

clientAuth := tls.RequestClientCert
clientAuth := tls.RequireAnyClientCert
if v := strings.ToLower(y.TLS.ClientAuth.Value); v != "" && v != "on" && v != "off" {
return nil, fmt.Errorf("kesconf: invalid tls config: invalid auth '%s'", y.TLS.ClientAuth)
} else if v == "on" {
clientAuth = tls.VerifyClientCertIfGiven
clientAuth = tls.RequireAndVerifyClientCert
}

for _, proxy := range y.TLS.Proxy.Identities {
Expand Down Expand Up @@ -304,6 +304,20 @@ func ymlToServerConfig(y *ymlFile) (*File, error) {
if api.Timeout.Value < 0 {
return nil, fmt.Errorf("kesconf: invalid timeout '%d' for API '%s'", api.Timeout.Value, path)
}

// If mTLS authentication is disabled for at least one API,
// we must no longer require that a client sends a certificate.
// However, this may cause authentication errors when a client
// (the client's HTTP/TLS stack) does not send a certificate
// for an API that requires authentication.
if api.InsecureSkipAuth.Value {
if clientAuth == tls.RequireAnyClientCert {
clientAuth = tls.RequestClientCert
}
if clientAuth == tls.RequireAndVerifyClientCert {
clientAuth = tls.VerifyClientCertIfGiven
}
}
}

if len(y.Keys) > 0 {
Expand Down

0 comments on commit 55e9e7d

Please sign in to comment.