Detect Vault 1.11+ import, update default issuer #15253

cipherboy · 2022-11-03T19:49:18Z

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>

Description

Background: https://support.hashicorp.com/hc/en-us/articles/11308460105491

Consul used to rely on implicit issuer selection when calling Vault endpoints to issue new CSRs. Vault 1.11+ changed that behavior, which caused Consul to check the wrong (previous) issuer when renewing its Intermediate CA. This patch allows Consul to explicitly set a default issuer when it detects that the response from Vault is 1.11+.

Testing & Reproduction steps

Tested manually in K8s setup and local unit tests using different versions of Vault

Links

https://support.hashicorp.com/hc/en-us/articles/11308460105491

Vault's PR to add multiple issuer support in PKI: hashicorp/vault#15277

Vault's PR to add flag to opt-in to previous behavior: hashicorp/vault#17824

PR Checklist

updated test coverage
external facing docs updated
not a security concern

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>

agent/connect/ca/provider_vault.go

cipherboy · 2022-11-15T17:54:37Z

@kisunji Hope that helps, happy to answer questions if you've got some more. :-)

agent/connect/ca/provider_vault.go

cipherboy · 2022-11-15T21:11:01Z

agent/connect/ca/provider_vault.go

+//
+// [/intermediate/set-signed]: https://developer.hashicorp.com/vault/api-docs/secret/pki#import-ca-certificates-and-keys
+// [/intermediate/generate/internal]: https://developer.hashicorp.com/vault/api-docs/secret/pki#generate-intermediate-csr
+func (v *VaultProvider) setDefaultIntermediateIssuer(vaultResp *vaultapi.Secret, keyId string) error {


Looking good, like this refactor!

agent/connect/ca/provider_vault.go

cipherboy

I can't really ACK it since its my PR, but the changes look good from a Vault perspective, thank you!

kisunji · 2022-11-17T17:57:14Z

agent/connect/ca/provider_vault_test.go

@@ -902,6 +902,29 @@ func TestVaultProvider_ReconfigureIntermediateTTL(t *testing.T) {
 	require.Equal(t, 333*3600, mountConfig.MaxLeaseTTL)
 }

+func TestVaultCAProvider_GenerateIntermediate(t *testing.T) {


Unsure if Vault tests will run so I'm pasting local findings here:

Vault 1.10.8 without changes (expected to pass in Vault <1.11)

=== RUN TestVaultCAProvider_RenewIntermediate [INFO] freeport: detected ephemeral port range of [49152, 65535] [INFO] freeport: reducing max blocks from 30 to 26 to avoid the ephemeral port range [DEBUG] freeport: Test "TestVaultCAProvider_RenewIntermediate" took ports [25001 25002] [INFO] agent/connect/ca: testing with vault server version: 1.10.8 [DEBUG] freeport: Test "TestVaultCAProvider_RenewIntermediate" returned ports [25001 25002] --- PASS: TestVaultCAProvider_RenewIntermediate (3.33s) PASS ok github.com/hashicorp/consul/agent/connect/ca 3.681s

Vault 1.10.8 with changes (regression test)

=== RUN TestVaultCAProvider_RenewIntermediate [INFO] freeport: detected ephemeral port range of [49152, 65535] [INFO] freeport: reducing max blocks from 30 to 26 to avoid the ephemeral port range [DEBUG] freeport: Test "TestVaultCAProvider_RenewIntermediate" took ports [20501 20502] [INFO] agent/connect/ca: testing with vault server version: 1.10.8 [DEBUG] freeport: Test "TestVaultCAProvider_RenewIntermediate" returned ports [20501 20502] --- PASS: TestVaultCAProvider_RenewIntermediate (1.33s) PASS ok github.com/hashicorp/consul/agent/connect/ca 1.687s

Vault 1.11.0 without changes

=== RUN TestVaultCAProvider_RenewIntermediate [INFO] freeport: detected ephemeral port range of [49152, 65535] [INFO] freeport: reducing max blocks from 30 to 26 to avoid the ephemeral port range [DEBUG] freeport: Test "TestVaultCAProvider_RenewIntermediate" took ports [32501 32502] [INFO] agent/connect/ca: testing with vault server version: 1.11.0 provider_vault_test.go:918: Error Trace: /Users/chriskim/code/consul/agent/connect/ca/provider_vault_test.go:918 Error: Should not be: "-----BEGIN CERTIFICATE-----\nMIICMDCCAdWgAwIBAgIUE/pOjHz3fDoy9Fgwbw4DYt+8CwAwCgYIKoZIzj0EAwIw\nMDEuMCwGA1UEAxMlcHJpLWhoZDgxdWFjLnZhdWx0LmNhLjExMTExMTExLmNvbnN1\nbDAeFw0yMjExMTcxNzQyMjBaFw0yMzExMTcxNzQyNTBaMDAxLjAsBgNVBAMTJXBy\naS0xanRyZDZ3Yi52YXVsdC5jYS4xMTExMTExMS5jb25zdWwwWTATBgcqhkjOPQIB\nBggqhkjOPQMBBwNCAATKxc/IyicWyhgazqFINH2LHxTPwV6/oyzJJL8ZUqie2VHX\nWQvuVm+SQ7YHT8Hv2/wd9Ji43OIqF/D2iZWX1F9Po4HMMIHJMA4GA1UdDwEB/wQE\nAwIBBjAPBgNVHRMBAf8EBTADAQH/MB0GA1UdDgQWBBT3BpRIx1gItM2dvzxCxk6K\nGZK+NTAfBgNVHSMEGDAWgBS4Klwhx8S2YHuEwR2pZSQM/S2jMTBmBgNVHREEXzBd\ngiVwcmktMWp0cmQ2d2IudmF1bHQuY2EuMTExMTExMTEuY29uc3VshjRzcGlmZmU6\nLy8xMTExMTExMS0yMjIyLTMzMzMtNDQ0NC01NTU1NTU1NTU1NTUuY29uc3VsMAoG\nCCqGSM49BAMCA0kAMEYCIQCFguARdz2ebI4Qz48tmuXp1/VgE94u+8pJK4wuMYAe\nZgIhAM//Dqv3ofZrmRtJbIx6VgjV15C9KqVOQUhwMlRcTalY\n-----END CERTIFICATE-----\n" Test: TestVaultCAProvider_RenewIntermediate [DEBUG] freeport: Test "TestVaultCAProvider_RenewIntermediate" returned ports [32501 32502] --- FAIL: TestVaultCAProvider_RenewIntermediate (1.82s) FAIL FAIL github.com/hashicorp/consul/agent/connect/ca 2.215s FAIL

Vault 1.11.0 with changes

=== RUN TestVaultCAProvider_RenewIntermediate [INFO] freeport: detected ephemeral port range of [49152, 65535] [INFO] freeport: reducing max blocks from 30 to 26 to avoid the ephemeral port range [DEBUG] freeport: Test "TestVaultCAProvider_RenewIntermediate" took ports [29501 29502] [INFO] agent/connect/ca: testing with vault server version: 1.11.0 [DEBUG] freeport: Test "TestVaultCAProvider_RenewIntermediate" returned ports [29501 29502] --- PASS: TestVaultCAProvider_RenewIntermediate (1.30s) PASS ok github.com/hashicorp/consul/agent/connect/ca 1.660s

Also tested with Vault 1.12.1 with same results as 1.11.0

Thank you for including this detail :)

kschoche · 2022-11-17T19:29:56Z

agent/connect/ca/provider_vault_test.go

+
+	orig, err := provider.ActiveIntermediate()
+	require.NoError(t, err)
+


Wow you weren't joking this did turn out to be easier than we thought! Nice work!

Consul used to rely on implicit issuer selection when calling Vault endpoints to issue new CSRs. Vault 1.11+ changed that behavior, which caused Consul to check the wrong (previous) issuer when renewing its Intermediate CA. This patch allows Consul to explicitly set a default issuer when it detects that the response from Vault is 1.11+. Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com> Co-authored-by: Chris S. Kim <ckim@hashicorp.com>

kisunji · 2022-11-17T21:46:38Z

Backports:
1.12.x: #15432
1.13.x: #15436
1.14.x: #15437

Consul used to rely on implicit issuer selection when calling Vault endpoints to issue new CSRs. Vault 1.11+ changed that behavior, which caused Consul to check the wrong (previous) issuer when renewing its Intermediate CA. This patch allows Consul to explicitly set a default issuer when it detects that the response from Vault is 1.11+. Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com> Co-authored-by: Chris S. Kim <ckim@hashicorp.com>

…5437)" This reverts commit 84838e5. It will not be part of the 1.14.1 patch release.

… issuer (#15661) The fix outlined and merged in #15253 fixed the issue as it occurs in the primary DC. There is a similar issue that arises when vault is used as the Connect CA in a secondary datacenter that is fixed by this PR. Additionally: this PR adds support to run the existing suite of vault related integration tests against the last 4 versions of vault (1.9, 1.10, 1.11, 1.12)

… issuer The fix outlined and merged in #15253 fixed the issue as it occurs in the primary DC. There is a similar issue that arises when vault is used as the Connect CA in a secondary datacenter that is fixed by this PR. Additionally: this PR adds support to run the existing suite of vault related integration tests against the last 4 versions of vault (1.9, 1.10, 1.11, 1.12)

… issuer (#15682) The fix outlined and merged in #15253 fixed the issue as it occurs in the primary DC. There is a similar issue that arises when vault is used as the Connect CA in a secondary datacenter that is fixed by this PR. Additionally: this PR adds support to run the existing suite of vault related integration tests against the last 4 versions of vault (1.9, 1.10, 1.11, 1.12) Co-authored-by: R.B. Boyer <4903+rboyer@users.noreply.github.com>

…date default issuer into release/1.12.x (#15681) * Detect Vault 1.11+ import in secondary datacenters and update default issuer The fix outlined and merged in #15253 fixed the issue as it occurs in the primary DC. There is a similar issue that arises when vault is used as the Connect CA in a secondary datacenter that is fixed by this PR. Additionally: this PR adds support to run the existing suite of vault related integration tests against the last 4 versions of vault (1.9, 1.10, 1.11, 1.12) * include testutil.RunStep * any -> interface{} Co-authored-by: R.B. Boyer <4903+rboyer@users.noreply.github.com> Co-authored-by: R.B. Boyer <rb@hashicorp.com>

Detect Vault 1.11+ import, update default issuer

eeed745

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>

github-actions bot added the theme/connect Anything related to Consul Connect, Service Mesh, Side Car Proxies label Nov 3, 2022

More commentary

1fd1f89

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>

kisunji reviewed Nov 15, 2022

View reviewed changes

agent/connect/ca/provider_vault.go Outdated Show resolved Hide resolved

kisunji reviewed Nov 15, 2022

View reviewed changes

agent/connect/ca/provider_vault.go Show resolved Hide resolved

kisunji reviewed Nov 15, 2022

View reviewed changes

agent/connect/ca/provider_vault.go Outdated Show resolved Hide resolved

Merge branch 'main' into fix-issuer-growing-list-maybe-from-vault

067113d

Chris S. Kim added 3 commits November 15, 2022 15:49

Refactored setting default intermediate issuer

7074c98

Fix commit message

e117b64

Merge branch 'main' into fix-issuer-growing-list-maybe-from-vault

0bd2b7d

cipherboy commented Nov 15, 2022

View reviewed changes

Chris S. Kim added 3 commits November 16, 2022 10:56

Address PR feedback

313c025

Minor formatting cleanup

9b8613f

Merge branch 'main' into fix-issuer-growing-list-maybe-from-vault

80ae5c5

cipherboy commented Nov 16, 2022

View reviewed changes

agent/connect/ca/provider_vault.go Show resolved Hide resolved

cipherboy commented Nov 16, 2022

View reviewed changes

Chris S. Kim added 2 commits November 16, 2022 15:00

Merge branch 'main' into fix-issuer-growing-list-maybe-from-vault

1ff1189

Add unit test

7625c05

kisunji reviewed Nov 17, 2022

View reviewed changes

Remove unnecessary line

37e0216

kisunji requested review from kschoche, a team and kyhavlov November 17, 2022 17:59

Merge branch 'main' into fix-issuer-growing-list-maybe-from-vault

7647728

jkirschner-hashicorp mentioned this pull request Nov 17, 2022

Vault 1.11 Multi-Issuer CA breaks Connect CA Intermediate CAs (<-> Vault Provider) #15217

Closed

kschoche reviewed Nov 17, 2022

View reviewed changes

Add changelog

90124a8

kisunji added backport/1.12 labels Nov 17, 2022

kisunji added backport/1.14 theme/consul-vault Relating to Consul & Vault interactions labels Nov 17, 2022

kisunji approved these changes Nov 17, 2022

View reviewed changes

kschoche approved these changes Nov 17, 2022

View reviewed changes

kisunji merged commit 2b90307 into hashicorp:main Nov 17, 2022

hashi-derek added a commit that referenced this pull request Nov 21, 2022

Revert "Detect Vault 1.11+ import, update default issuer (#15253) (#1…

e297e3e

…5437)" This reverts commit 84838e5. It will not be part of the 1.14.1 patch release.

rboyer mentioned this pull request Dec 2, 2022

Detect Vault 1.11+ import in secondary datacenters and update default issuer #15661

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Detect Vault 1.11+ import, update default issuer #15253

Detect Vault 1.11+ import, update default issuer #15253

cipherboy commented Nov 3, 2022 •

edited by kisunji

Loading

cipherboy commented Nov 15, 2022

cipherboy Nov 15, 2022

cipherboy left a comment

kisunji Nov 17, 2022 •

edited

Loading

jkirschner-hashicorp Nov 17, 2022

kschoche Nov 17, 2022

kisunji commented Nov 17, 2022


		orig, err := provider.ActiveIntermediate()
		require.NoError(t, err)

Detect Vault 1.11+ import, update default issuer #15253

Detect Vault 1.11+ import, update default issuer #15253

Conversation

cipherboy commented Nov 3, 2022 • edited by kisunji Loading

Description

Testing & Reproduction steps

Links

PR Checklist

cipherboy commented Nov 15, 2022

cipherboy Nov 15, 2022

Choose a reason for hiding this comment

cipherboy left a comment

Choose a reason for hiding this comment

kisunji Nov 17, 2022 • edited Loading

Choose a reason for hiding this comment

jkirschner-hashicorp Nov 17, 2022

Choose a reason for hiding this comment

kschoche Nov 17, 2022

Choose a reason for hiding this comment

kisunji commented Nov 17, 2022

cipherboy commented Nov 3, 2022 •

edited by kisunji

Loading

kisunji Nov 17, 2022 •

edited

Loading