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

VAULT-8518 Increase HMAC limit to 4096, and limit approle names to the same limit #17768

Merged
merged 3 commits into from
Nov 2, 2022

Conversation

VioletHynes
Copy link
Contributor

As part of #14746, we restricted the length of approle's HMAC to 1024. This had the unfortunate effect of making it so that operations on an approle with such a name would fail.

As part of this change, I've increased the limit slightly, so that it's more permissive to names, but also made approle creation fail if the name passes this limit, so that we cannot get into this state to begin with.

Copy link
Contributor

@mpalmi mpalmi left a comment

Choose a reason for hiding this comment

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

Looks good! Just some minor nits.

builtin/credential/approle/path_role.go Outdated Show resolved Hide resolved
command/agent/approle_end_to_end_test.go Show resolved Hide resolved
command/agent/approle_end_to_end_test.go Show resolved Hide resolved
Copy link
Contributor

@ccapurso ccapurso left a comment

Choose a reason for hiding this comment

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

Looks good!

@VioletHynes VioletHynes merged commit 2ae9835 into main Nov 2, 2022
@VioletHynes VioletHynes deleted the violethynes/VAULT-8518 branch November 2, 2022 14:42
Copy link
Contributor

@hghaf099 hghaf099 left a comment

Choose a reason for hiding this comment

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

I know this is merged, but I figured to ask my question anyways :)

@@ -0,0 +1,3 @@
```release-note:change
auth/approle: Add maximum length of 4096 for approle role_names, as this value results in HMAC calculation
Copy link
Contributor

Choose a reason for hiding this comment

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

I am bit confused by this phrase. Do you mean that HMAC result is of length 4096?

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.

4 participants