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

Add new API to PKI to list revoked certificates #17779

Merged
merged 3 commits into from
Nov 3, 2022

Conversation

stevendpclark
Copy link
Contributor

A new API that will return the list of serial numbers of revoked certificates on the local cluster.

❯ curl \
    --header "X-Vault-Token: $VAULT_TOKEN" \
    --request LIST \
    http://127.0.0.1:8200/v1/pki_int/certs/revoked | jq
{
  "request_id": "e221b49c-6c28-0b08-30dc-e2dd2f37d077",
  "lease_id": "",
  "renewable": false,
  "lease_duration": 0,
  "data": {
    "keys": [
      "3d:80:91:c3:c2:34:3b:81:69:3d:92:a3:80:69:db:53:04:26:ab:b4"
    ]
  },
  "wrap_info": null,
  "warnings": null,
  "auth": null
}

 - A new API that will return the list of serial numbers of
   revoked certificates on the local cluster.
@stevendpclark stevendpclark added this to the 1.13.0-rc1 milestone Nov 2, 2022
@stevendpclark stevendpclark requested review from kitography, cipherboy and a team November 2, 2022 18:18

| Method | Path |
|:-------|:------------------|
| `LIST` | `/certs/revoked` |
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
| `LIST` | `/certs/revoked` |
| `LIST` | `/certs/revoked` |

I'm sorry, it just annoys me :-D

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops yup will fix this, thanks for catching it!

@@ -1216,3 +1216,12 @@ func (sc *storageContext) writeAutoTidyConfig(config *tidyConfig) error {

return sc.Storage.Put(sc.Context, entry)
}

func (sc *storageContext) listRevokedCerts() ([]string, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Ah cool, we should use this elsewhere eventually :)

requireSuccessNonNilResponse(t, resp, err, "error revoking cert 2")

// Test that we get back the expected serial numbers.
resp, err = CBList(b, s, "certs/revoked")
Copy link
Contributor

Choose a reason for hiding this comment

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

Wanna also make sure we see these serial numbers in the regular list too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

By regular list are you referring to LIST /certs?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yep :)

Copy link
Contributor

Choose a reason for hiding this comment

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

Pefect, looks great!

@stevendpclark stevendpclark merged commit 4df5979 into main Nov 3, 2022
@stevendpclark stevendpclark deleted the stevendpclark/vault-9719-list-revoked-certs branch November 3, 2022 18:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants