Skip to content

HMAC key usage in KeyInfo #303

Closed
Closed

Description

@shunkica

I would like to raise an issue regarding the handling of HMAC secret keys in the library. Currently, the library requires placement of the HMAC secret key inside the keyInfoProvider during HMAC signature verification. However, the KeyInfoProvider is meant to contain public keys, not secret keys. In the context of HMAC, getKeyInfo() (or getKeyInfoContent()) is not needed.

Also, putting a HMAC key in the keyInfoProvider during the signing process poses a potential security risk as it could lead to the leakage of the HMAC secret key inside the KeyInfo element.

I propose that the library should adopt a more appropriate approach for HMAC verification. Instead of using the KeyInfoProvider, it would be advisable to use the signingKey property for both HMAC signing and verification, as it ensures consistency in symmetric key usage.

This change would have the additional benefit of KeyInfoProvider no longer having to handle HMAC keys, and parsing solely PEM keys.

Suggested Solution:

  • Update the documentation to emphasize the use of the signingKey property for HMAC verification
    
  • Do not use keyInfoProvider during HMAC signing and verifying
    
  • Fix the HMAC tests
    

Please let me know if any further information is needed.

Activity

  1. cjbarth commented on Jun 10, 2023

    @cjbarth
    Contributor

    I'm not sure that leaking a private HMAC key is actually a problem if we turn the entire KeyInfo class into having only static members because the only place an HMAC key would come into play would be if it were set as a fallback because we can't encode a binary HMAC key into an <X509Certificate /> element (and we can check before embedding too).

    Am I missing something?

  2. shunkica commented on Jun 10, 2023

    @shunkica
    ContributorAuthor

    Your points are well taken, @cjbarth. I do agree that a well-implemented system might not leak an HMAC secret key even if the KeyInfo class were made entirely static. However, my concern stems more from the perspective of a strict separation of concerns and the potential misuse of the system by developers not entirely familiar with its intricacies.

    By including the HMAC secret key in the KeyInfoProvider, the library appears to be conflating the roles of public and secret keys.

    Using the signingKey property for HMAC verification enforces the symmetric nature of HMAC keys and ensures consistency in key usage. Additionally, it would separate the handling of public keys (in KeyInfoProvider) from the handling of secret keys (in the signingKey property), thereby enhancing clarity in key management.

    These are not criticisms of the current library but suggestions to further refine and improve it.

  3. locked and limited conversation to collaborators on Jun 12, 2023
  4. converted this issue into a discussion #306 on Jun 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions