Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

make kubernetes_ca_cert optional #238

Merged
merged 18 commits into from
Mar 27, 2024
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@
* `k8s.io/api` v0.29.1 -> v0.29.2
* `k8s.io/apimachinery` v0.29.1 -> v0.29.2

### Improvements

* Make kubernetes_ca_cert optional [GH-238](https://github.com/hashicorp/vault-plugin-auth-kubernetes/pull/238)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is really about having "TLS uses the host's root CA set" if no CA chain is provided - which is a behavioural change.

We should also document the extra validation being done on the provided CA PEM bundle.


## 0.18.0 (Feb 2, 2024)

### Changes
Expand Down
22 changes: 13 additions & 9 deletions backend.go
Original file line number Diff line number Diff line change
Expand Up @@ -433,16 +433,20 @@ func (b *kubeAuthBackend) updateTLSConfig(config *kubeConfig) error {
caCertBytes = []byte(data)
}

certPool := x509.NewCertPool()
if len(caCertBytes) > 0 {
if ok := certPool.AppendCertsFromPEM(caCertBytes); !ok {
b.Logger().Warn("Configured CA PEM data contains no valid certificates, TLS verification will fail")
if len(caCertBytes) == 0 {
b.Logger().Warn("No CA certificates configured, TLS verification will use the system's trust store")
thyton marked this conversation as resolved.
Show resolved Hide resolved
transport, ok := b.httpClient.Transport.(*http.Transport)
if !ok {
// should never happen
return fmt.Errorf("type assertion failed for %T", b.httpClient.Transport)
}
} else {
// provide an empty certPool
b.Logger().Warn("No CA certificates configured, TLS verification will fail")
// TODO: think about supporting host root CA certificates via a configuration toggle,
// in which case RootCAs should be set to nil
transport.TLSClientConfig = getDefaultTLSConfig()
return nil
}

certPool := x509.NewCertPool()
if ok := certPool.AppendCertsFromPEM(caCertBytes); !ok {
b.Logger().Warn("Configured CA PEM data contains no valid certificates, TLS verification will fail")
}

// only refresh the Root CAs if they have changed since the last full update.
Expand Down
17 changes: 17 additions & 0 deletions backend_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,23 @@ func Test_kubeAuthBackend_updateTLSConfig(t *testing.T) {
},
wantErr: false,
},
{
name: "ca-certs-not-set",
httpClient: getDefaultHTTPClient(),
tlsConfig: getDefaultTLSConfig(),
configs: []testConfig{
{
config: &kubeConfig{
DisableLocalCAJwt: true,
},
expectTLSConfig: &tls.Config{
MinVersion: minTLSVersion,
RootCAs: nil,
},
},
},
wantErr: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
Expand Down
14 changes: 5 additions & 9 deletions path_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,10 @@ func pathConfig(b *kubeAuthBackend) *framework.Path {
Type: framework.TypeString,
Description: "Host must be a host string, a host:port pair, or a URL to the base of the Kubernetes API server.",
},

"kubernetes_ca_cert": {
Type: framework.TypeString,
Description: "PEM encoded CA cert for use by the TLS client used to talk with the API.",
Type: framework.TypeString,
Description: `Optional PEM encoded CA cert for use by the TLS client used to talk with the API.
If it is not set and disable_local_ca_jwt is true, the system's trust store will be used.`,
thyton marked this conversation as resolved.
Show resolved Hide resolved
DisplayAttrs: &framework.DisplayAttributes{
Name: "Kubernetes CA Certificate",
},
Expand Down Expand Up @@ -159,18 +159,14 @@ func (b *kubeAuthBackend) pathConfigWrite(ctx context.Context, req *logical.Requ
return logical.ErrorResponse("no host provided"), nil
}

disableLocalJWT := data.Get("disable_local_ca_jwt").(bool)
disableLocalCAJwt := data.Get("disable_local_ca_jwt").(bool)
pemList := data.Get("pem_keys").([]string)
caCert := data.Get("kubernetes_ca_cert").(string)
issuer := data.Get("issuer").(string)
disableIssValidation := data.Get("disable_iss_validation").(bool)
tokenReviewer := data.Get("token_reviewer_jwt").(string)
useAnnotationsAsAliasMetadata := data.Get("use_annotations_as_alias_metadata").(bool)

if disableLocalJWT && caCert == "" {
return logical.ErrorResponse("kubernetes_ca_cert must be given when disable_local_ca_jwt is true"), nil
}

config := &kubeConfig{
PublicKeys: make([]crypto.PublicKey, len(pemList)),
PEMKeys: pemList,
Expand All @@ -179,7 +175,7 @@ func (b *kubeAuthBackend) pathConfigWrite(ctx context.Context, req *logical.Requ
TokenReviewerJWT: tokenReviewer,
Issuer: issuer,
DisableISSValidation: disableIssValidation,
DisableLocalCAJwt: disableLocalJWT,
DisableLocalCAJwt: disableLocalCAJwt,
UseAnnotationsAsAliasMetadata: useAnnotationsAsAliasMetadata,
}

Expand Down
35 changes: 34 additions & 1 deletion path_config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -458,7 +458,7 @@ func TestConfig_LocalCaJWT(t *testing.T) {
DisableLocalCAJwt: false,
},
},
"CA and disable local default": {
"disable local default, CA set": {
config: map[string]interface{}{
"kubernetes_host": "host",
"kubernetes_ca_cert": testCACert,
Expand All @@ -474,6 +474,39 @@ func TestConfig_LocalCaJWT(t *testing.T) {
DisableLocalCAJwt: true,
},
},
"disable local default, JWT set": {
config: map[string]interface{}{
"kubernetes_host": "host",
"token_reviewer_jwt": jwtGoodDataToken,
"disable_local_ca_jwt": true,
},
setupInClusterFiles: true,
expected: &kubeConfig{
PublicKeys: []crypto.PublicKey{},
PEMKeys: []string{},
Host: "host",
CACert: "",
TokenReviewerJWT: jwtGoodDataToken,
DisableISSValidation: true,
DisableLocalCAJwt: true,
},
},
"disable local default, no CA or JWT": {
config: map[string]interface{}{
"kubernetes_host": "host",
"disable_local_ca_jwt": true,
},
setupInClusterFiles: true,
expected: &kubeConfig{
PublicKeys: []crypto.PublicKey{},
PEMKeys: []string{},
Host: "host",
CACert: "",
TokenReviewerJWT: "",
DisableISSValidation: true,
DisableLocalCAJwt: true,
},
},
}

for name, tc := range testCases {
Expand Down
Loading