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

Backport PKI Intermediate Revocation bug fix and test improvements #16054

Merged
merged 2 commits into from
Jun 17, 2022

Conversation

cipherboy
Copy link
Contributor

Backport of #16052. However, the test infra in #15999 was also pulled in. Either we had to rewrite the backported test, or pull in most of the updated testing infra. Importing it verbatim as a backport seemed cleaner than a piece-meal backport.

* Refactor role issuance tests to use direct backend

Before:
	github.com/hashicorp/vault/builtin/logical/pki	5.879s

After:
	github.com/hashicorp/vault/builtin/logical/pki	1.063s

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

* Refactor role key bit tests to use direct backend

Also removes redundant cases.

Before:
	github.com/hashicorp/vault/builtin/logical/pki	136.605s

After:

	github.com/hashicorp/vault/builtin/logical/pki	24.713s

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

* Refactor common name test to use direct backend

Before:
	github.com/hashicorp/vault/builtin/logical/pki	4.767s

After:

	github.com/hashicorp/vault/builtin/logical/pki	0.611s

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

* Refactor device cert tests to use direct backend

Before:
	github.com/hashicorp/vault/builtin/logical/pki	4.725s

After:

	github.com/hashicorp/vault/builtin/logical/pki	0.402s

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

* Refactor invalid parameter test to use direct backend

Before:
	github.com/hashicorp/vault/builtin/logical/pki	3.777s

After:
	github.com/hashicorp/vault/builtin/logical/pki	0.021s

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

* Refactor Alt Issuer tests to use direct backend

Before:
	github.com/hashicorp/vault/builtin/logical/pki	4.560s

After:
	github.com/hashicorp/vault/builtin/logical/pki	0.111s

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

* Refactor root idempotency tests to use direct backend

As a result, we've had to import a root cert from elsewhere in the test
suite, rather than using the one off the cluster.

Before:
	github.com/hashicorp/vault/builtin/logical/pki	4.399s

After:
	github.com/hashicorp/vault/builtin/logical/pki	0.523s

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

* Move PKI direct backend helpers to common location

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

* Refactor OID SANs test to direct backend

Before:
	github.com/hashicorp/vault/builtin/logical/pki	5.284s

After:
	github.com/hashicorp/vault/builtin/logical/pki	0.808s

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

* Refactor allowed serial numbers test to direct backend

Before:
	github.com/hashicorp/vault/builtin/logical/pki	4.789s

After:
	github.com/hashicorp/vault/builtin/logical/pki	0.600s

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

* Refactor URI SANs to use direct backend

Before:
	github.com/hashicorp/vault/builtin/logical/pki	4.245s

After:
	github.com/hashicorp/vault/builtin/logical/pki	0.600s

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

* Refactor Full Chain CA tests to direct backend

Before:
	github.com/hashicorp/vault/builtin/logical/pki	14.503s

After:
	github.com/hashicorp/vault/builtin/logical/pki	2.082s

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

* Update Allow Past CA tests to use direct backend

Before:
	github.com/hashicorp/vault/builtin/logical/pki	4.323s

After:
	github.com/hashicorp/vault/builtin/logical/pki	0.322s

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

* Convert existing-key root test to direct backend

Before:
	github.com/hashicorp/vault/builtin/logical/pki	4.430s

After:
	github.com/hashicorp/vault/builtin/logical/pki	0.370s

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

* Refactor CRL enable/disable tests to use direct backend

Before:
	github.com/hashicorp/vault/builtin/logical/pki	5.738s

After:
	github.com/hashicorp/vault/builtin/logical/pki	2.482s

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

* Update intermediate existing key tests to use direct backend

Before:
	github.com/hashicorp/vault/builtin/logical/pki	4.182s

After:
	github.com/hashicorp/vault/builtin/logical/pki	0.416s

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

* Refactor Issuance TTL verification tests to use direct backend

Also shorten sleep duration slightly by precisely calculating it
relative to the actual cert life time.

Before:
	github.com/hashicorp/vault/builtin/logical/pki	19.755s

After:
	github.com/hashicorp/vault/builtin/logical/pki	11.521s

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
* Add test for revocation under intermediate CA

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

* Allow revocation of certs with key-less issuers

In Vault 1.11's multiple issuer functionality, we incorrectly fetched
the full CA signing bundle for validating revocation of leaf certs (when
attempting to prohibit revocation of issuers in the mount). When the
issuer lacked a key (such as the root issuer on an intermediate mount),
this signing bundle creation failed.

Instead of fetching the full CA signing bundle, fetch instead the raw
certutil.CertBundle and parse it (to x509.Certificate form) ourselves.

This manifests as the error on revocation:

> URL: PUT http://127.0.0.1:8200/v1/pki_int/revoke
> * could not fetch the CA certificate for issuer id 156e1b99-4f04-5b5e-0036-cc0422c0c0d3: unable to fetch corresponding key for issuer 156e1b99-4f04-5b5e-0036-cc0422c0c0d3; unable to use this issuer for signing

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
@cipherboy cipherboy requested a review from a team June 17, 2022 22:22
@cipherboy cipherboy requested review from a team and taoism4504 as code owners June 17, 2022 22:22
@cipherboy cipherboy requested review from a team, sarahethompson and modrake and removed request for a team June 17, 2022 22:22
@cipherboy cipherboy changed the base branch from main to release/1.11.x June 17, 2022 22:22
@cipherboy cipherboy requested review from stevendpclark and a team and removed request for a team, taoism4504, sarahethompson and modrake June 17, 2022 22:22
@cipherboy cipherboy enabled auto-merge (squash) June 17, 2022 22:26
@cipherboy cipherboy merged commit ea296cc into release/1.11.x Jun 17, 2022
@cipherboy cipherboy deleted the cipherboy-backport-bug-fix-pki branch December 1, 2022 14:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants