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 support for a dedicated HMAC type in Transit. #16668

Merged
merged 12 commits into from
Sep 6, 2022
Merged

Add support for a dedicated HMAC type in Transit. #16668

merged 12 commits into from
Sep 6, 2022

Conversation

sgmiller
Copy link
Collaborator

This allows it to be used with BYOK. We also add a key_size parameter for
this type, since HMAC supports any sized key, though we limit it to 256-4096
bits.

@sgmiller sgmiller requested a review from a team August 10, 2022 16:27
sdk/helper/keysutil/policy.go Outdated Show resolved Hide resolved
sdk/helper/keysutil/policy.go Outdated Show resolved Hide resolved
sdk/helper/keysutil/policy.go Show resolved Hide resolved
sgmiller and others added 3 commits August 10, 2022 14:57
Co-authored-by: Matt Schultz <975680+schultz-is@users.noreply.github.com>
Co-authored-by: Matt Schultz <975680+schultz-is@users.noreply.github.com>
Copy link
Contributor

@schultz-is schultz-is left a comment

Choose a reason for hiding this comment

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

Cool new functionality!

@@ -600,7 +601,7 @@ func generateKey(keyType string) (interface{}, error) {
switch keyType {
case "aes128-gcm96":
return uuid.GenerateRandomBytes(16)
case "aes256-gcm96":
case "aes256-gcm96", "hmac":
Copy link
Contributor

@cipherboy cipherboy Aug 16, 2022

Choose a reason for hiding this comment

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

I think we want to actually generate 64 bytes for hmac?

My 2c., but since SHA-256 uses a 512-bit block (which I think is what HMAC is using under the covers, but it isn't clear from my quick glance) -- a 32-byte key would be padded with 32 bytes of zeros, I'd rather we use all the bits since we can.

I'm also generally interested in extensibility. Do you see us adding SHA-512? SHA-3? If so, where/how?

Definitely like this though!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We could add hash function as a sub param. I think I like that rather than a plethora of distinct types.

Co-authored-by: Matt Schultz <975680+schultz-is@users.noreply.github.com>
@sgmiller sgmiller merged commit d6a1ce2 into main Sep 6, 2022
@sgmiller sgmiller deleted the transit-hmac branch September 6, 2022 15:18
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

4 participants