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

Check cnf claim with CSR fingerprint #1660

Merged
merged 8 commits into from
Jul 24, 2024
Merged

Check cnf claim with CSR fingerprint #1660

merged 8 commits into from
Jul 24, 2024

Conversation

maraino
Copy link
Contributor

@maraino maraino commented Dec 29, 2023

This commit allows tying tokens with the provided CSR or SSH public key. Tokens with a confirmation claim kid (cnf.kid) will validate that the provided fingerprint (kid) matches the CSR or SSH public key.

This check will only be present in JWK and X5C provisioners.

Fixes #1637

Related PRs:

@github-actions github-actions bot added the needs triage Waiting for discussion / prioritization by team label Dec 29, 2023
This commit allows tying tokens with the provided  CSR or SSH public
key. Tokens with a confirmation claim kid (cnf.kid) will validate that
the provided fingerprint (kid) matches the CSR or SSH public key.

This check will only be present in JWK and X5C provisioners.

Fixes #1637
@maraino maraino marked this pull request as ready for review January 5, 2024 23:47
@maraino maraino requested a review from hslatman January 5, 2024 23:47
@hslatman hslatman added this to the v0.26.0 milestone Mar 26, 2024
@hslatman hslatman modified the milestones: v0.26.0, v0.26.1 Mar 29, 2024
authority/provisioner/sign_options.go Outdated Show resolved Hide resolved
authority/provisioner/sign_ssh_options.go Outdated Show resolved Hide resolved
@hslatman hslatman modified the milestones: v0.26.1, v0.26.2 Apr 25, 2024
@hslatman hslatman modified the milestones: v0.26.2, v0.26.3 Jun 17, 2024
@hslatman hslatman modified the milestones: v0.27.0, v0.27.2 Jul 15, 2024
@hslatman hslatman modified the milestones: v0.27.2, v0.27.3 Jul 23, 2024
@maraino maraino requested a review from hslatman July 23, 2024 18:50
}
for name, tt := range tests {
t.Run(name, func(t *testing.T) {
tc := tt(t)
ctx := NewContextWithMethod(context.Background(), SignIdentityMethod)
if opts, err := tc.p.AuthorizeSign(ctx, tc.token); err != nil {
if assert.NotNil(t, tc.err) {
if assert.NotNil(t, tc.err, err.Error()) {
Copy link
Member

Choose a reason for hiding this comment

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

A bit of a weird test case. The tc.err is the expected error, and an assertion on that is unexpected. It was already there, but I don't think this is the right test logic now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It works as expected, showing the err.Error() if the assertion fails, but with ad70982 I've changed this file to use testify's assert and require packages.

authority/tls_test.go Show resolved Hide resolved
@maraino maraino changed the title Check cnf claim with CSR or SSH public key fingerprint Check cnf claim with CSR fingerprint Jul 24, 2024
@maraino maraino requested a review from hslatman July 24, 2024 19:20
hslatman
hslatman previously approved these changes Jul 24, 2024
@maraino maraino merged commit 9c95d34 into master Jul 24, 2024
14 checks passed
@maraino maraino deleted the fix-1637 branch July 24, 2024 19:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs triage Waiting for discussion / prioritization by team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Option to bind JWK provisioner tokens with CSR
2 participants