Skip to content

Cryptography - Received comments about CBC #2494

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

Closed
randomstuff opened this issue Jan 2, 2025 · 4 comments · May be fixed by #2887
Closed

Cryptography - Received comments about CBC #2494

randomstuff opened this issue Jan 2, 2025 · 4 comments · May be fixed by #2887
Assignees
Labels
1) Discussion ongoing Issue is opened and assigned but no clear proposal yet AppendixV Appendix with crypto details Bart Preneel Issues raised from a crypto review by Bart Preneel (received via Aram H) _5.0 - Not blocker This issue does not block 5.0 so if it gets addressed then great, if not then fine.

Comments

@randomstuff
Copy link
Contributor

Currently we have this comment the crypto appendix:

All encrypted messages must be authenticated. Given this, for ANY use of CBC mode there MUST be an associated hashing function or MAC to validate the message. In general, this MUST be applied in the Encrypt-Then-Hash method (but TLS 1.2 uses Hash-Then-Encrypt instead). If this cannot be guaranteed, then CBC MUST NOT be used.

We have received this comment from Bart Preneel:

I would not hide such an important aspect in an appendix. I would also rewrite it as follows:

All encrypted messages must be authenticated. Given this, for ANY use of CBC mode there MUST be an associated hashing function or MAC algorithm to validate the message. In general, this MUST be applied in the Encrypt-Then-Hash method (but TLS 1.2 uses Hash-Then-Encrypt instead). If this cannot be guaranteed, then CBC MUST NOT be used.

The only application where encryption without a MAC algorithm is allowed is disk encryption.

If CBC is used, it shall be guaranteed that the verification of the padding is performed in constant time.

@jmanico
Copy link
Member

jmanico commented Jan 3, 2025

This looks like a very positive change. If this is coming from Bart, I'd suggest we add it to 5.0.

@danielcuthbert
Copy link
Collaborator

Agreed, good addition and ill add it in as part of the batches ill be doing this coming week. Thanks @randomstuff

@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 Bart Preneel Issues raised from a crypto review by Bart Preneel (received via Aram H) labels Jan 5, 2025
@tghosth tghosth added AppendixV Appendix with crypto details _5.0 - Not blocker This issue does not block 5.0 so if it gets addressed then great, if not then fine. and removed V11 (prev V6) _5.0 - prep This needs to be addressed to prepare 5.0 labels Jan 22, 2025
@randomstuff
Copy link
Contributor Author

@danielcuthbert, do you want to propose something on that or shall I make a PR?

@danielcuthbert
Copy link
Collaborator

I'm in transit right now so was planning on doing this on the plane if it had wifi, but if you have spare cycles and can do a PR, that would be super helpful and i'd appreciate the help. thank you

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 AppendixV Appendix with crypto details Bart Preneel Issues raised from a crypto review by Bart Preneel (received via Aram H) _5.0 - Not blocker This issue does not block 5.0 so if it gets addressed then great, if not then fine.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants