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

Cryptography - proposed requirement about MAC verification failure #2499

Open
randomstuff opened this issue Jan 2, 2025 · 6 comments
Open
Assignees
Labels
1) Discussion ongoing Issue is opened and assigned but no clear proposal yet Bart Preneel Issues raised from a crypto review by Bart Preneel (received via Aram H) V6 _5.0 - prep This needs to be addressed to prepare 5.0

Comments

@randomstuff
Copy link
Contributor

Proposed requirement by Bart Preneel:

6.5.6 Verify that if the MAC verification fails, the decryption operation is not started, and if that is not possible, no plaintext is released.

Might be difficult to verify / quite low-level.

Caveat: This is not compatible with MAC-then-encrypt as still used with some TLS 1.2 ciphersuites. (see #2498).

@jmanico
Copy link
Member

jmanico commented Jan 3, 2025 via email

@tghosth tghosth added 1) Discussion ongoing Issue is opened and assigned but no clear proposal yet _5.0 - prep This needs to be addressed to prepare 5.0 labels Jan 5, 2025
@tghosth
Copy link
Collaborator

tghosth commented Jan 5, 2025

I leave this up to @danielcuthbert's judgement, I am not sure about this one.

@tghosth tghosth added the Bart Preneel Issues raised from a crypto review by Bart Preneel (received via Aram H) label Jan 5, 2025
@unprovable
Copy link

I think that the additions from Bart should have [ADDED] in the front, no?

In any case, this is a very sound bit of advice, but is going to be mostly left to TLS, SSH, or whatever other libraries are in play. Both of those will reject connections if verification fails, so it's really a design pattern check for anything custom written by a developer. Perhaps you could make en/de-cryption occur with auth failing in something like JWT?

I think this might need some further digging.

@randomstuff
Copy link
Contributor Author

Perhaps you could make en/de-cryption occur with auth failing in something like JWT?

When using JWT with encryption, the claims which might be useful for "routing" the JWT are expected to be included in the (plaintext) JWT header. So I don't think that this should be required.

But actually JWT tends to uses MAC-or-sign-then-ecnrypt (when using encryption).

While syntactically the signing and encryption operations for Nested JWTs may be applied in any order, if both signing and encryption are necessary, normally producers should sign the message and then encrypt the result (thus encrypting the signature). This prevents attacks in which the signature is stripped, leaving just an encrypted message, as well as providing privacy for the signer. Furthermore, signatures over encrypted text are not considered valid in many jurisdictions.

Note that potential concerns about security issues related to the order of signing and encryption operations are already addressed by the underlying JWS and JWE specifications; in particular, because JWE only supports the use of authenticated encryption algorithms, cryptographic concerns about the potential need to sign after encryption that apply in many contexts do not apply to this specification.

Note: they are talking about signature but actually intend to talk about both digital signatures and MAC (AFAIU).

A MAC is verified before decryption of the JWE, however. I think it does help mitigate the issues we are intending to mitigate with 6.5.6. @danielcuthbert?

@tghosth
Copy link
Collaborator

tghosth commented Jan 16, 2025

Again @randomstuff, do you think this is general enough to include? It seems like another example of a requirement to use if you "roll your own crypto"....

@randomstuff
Copy link
Contributor Author

@tghosth, I agree that it is quite low-level and I am not sure we need to include that level of detail in ASVS (as opposed to "crypto security verification standard" 😄).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1) Discussion ongoing Issue is opened and assigned but no clear proposal yet Bart Preneel Issues raised from a crypto review by Bart Preneel (received via Aram H) V6 _5.0 - prep This needs to be addressed to prepare 5.0
Projects
None yet
Development

No branches or pull requests

6 participants