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

Format certificates to separate out pem and key #86

Merged
merged 6 commits into from
May 19, 2023

Conversation

vickicello
Copy link
Contributor

@vickicello vickicello commented May 15, 2023

Summary

This change adds a new template helper, expandFullChain, that will take a map of secrets and separate out the pem and key file if the secret is a certificate.

Some modules were also updated to address the open dependabot PRs. In order to bump the package versions, we also had to update Go to version 1.17.x.

Manual Testing

A few conditions were tested:

  • expandFullChain template helper works with both PEM and PKCS12 files
  • Disabled secrets will not cause AKVA to error out (GetSecrets has been updated to not pull down disabled secrets by default)
  • A mixture of string secrets + certificate secrets, or just certificate secrets can be pulled down successfully
  • pkcs12 bundle without key: since Azure does not allow you to upload a pkcs12 file without a key, this will never happen
  • stacked pem works

Testing configuration and output:

# akva.yaml

workers:
  -
    resources:
      - kind: all-secrets
        vaultBaseURL: https://my-testing-keyvault.vault.azure.net/
        credential: my-testing-keyvault
    sinks:
      - path: secrets.json
        template: "{{ index .Secrets | expandFullChain | toValues | toJson }}"
    frequency: 10s

# Run AKVA with `go mod download && go build . && ./azure-key-vault-agent -c ./akva.yaml`

# secrets.json - returns all original secrets, plus pem and key separated out for any certificates:

{"test-cert":"...","test-cert.key":"...","test-cert.pem":"...","some-string-secret":"...","different-cert":"...","different-cert.key":"...","different-cert.pem":"..."}

Caveat

expandFullChain is only expected to be used on the all-secrets kind. Like the toValues template helper, expandFullChain will throw an error if you try to use it on an individual secret or certificate. The intended behavior and use is reflected in the README.

@vickicello vickicello changed the title Format all secrets to separate out pem and key Format certificates to separate out pem and key May 16, 2023
Copy link
Contributor

@chrisjohnson chrisjohnson left a comment

Choose a reason for hiding this comment

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

Overall looks fantastic 😍 small question

if *secret.ContentType == "application/x-pkcs12" {
key := cloneSecret(secret)
*key.Value = certutil.PemPrivateKeyFromPkcs12(*secret.Value)
results[secretName+".key"] = secrets.Secret(key)
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 key is already a secrets.Secret -- what happens if you just do results[secretName+".key"] = key?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep that isn't needed anymore - I'll commit that change

@drewby08
Copy link
Contributor

We should also update the README's list of helpers:

Complete List of Cert Helpers:

@vickicello vickicello force-pushed the format-all-secrets branch 3 times, most recently from 34db91d to 938182b Compare May 18, 2023 14:43
Copy link
Contributor

@drewby08 drewby08 left a comment

Choose a reason for hiding this comment

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

Looks great, thank you for the new feature :)

@vickicello vickicello merged commit d250ac8 into master May 19, 2023
1 check passed
@vickicello vickicello deleted the format-all-secrets branch May 19, 2023 18:04
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

3 participants