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 - suggested modification of 6.5.4 #2497

Closed
randomstuff opened this issue Jan 2, 2025 · 13 comments
Closed

Cryptography - suggested modification of 6.5.4 #2497

randomstuff opened this issue Jan 2, 2025 · 13 comments
Assignees
Labels
6) PR awaiting review 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

Current:

6.5.4 [MODIFIED, MOVED FROM 6.2.7] Verify that encrypted data is authenticated via signatures, as well as through authenticated cipher modes or HMAC for protection against unauthorized modification.

Proposed by Bart Preneel:

6.5.4 Verify that authenticated encryption modes are always used, preferably by using an authenticated encryption mode such as mentioned under 6.5.2 or by combining an encryption method with a secure MAC algorithm such as AES-CMAC, HMAC-SHA-2, AES-GMAC, Poly1305-AES.

with comment:

Digital signatures are rarely combined with encryption. I would not use the term signatures for a MAC algorithm.

@randomstuff
Copy link
Contributor Author

OK for me after some wording changes.

Some notes/comment on my own:

  • Some of these schemes are currently not in the crypto appendix, should be add them?
  • Should be add Chacha20-Poly1305 in the list in 6.5.4?
  • Should we have this list here or simply point to the appendix.
  • " combining an encryption method with a secure MAC algorithm such as AES-CMAC, HMAC-SHA-2, AES-GMAC, Poly1305-AES" is somewhat ambiguous because it appears that AES-CMAC and friends are MAC algorithms whereas they are actually a combination of an encryption scheme and a MAC algorithm.

@elarlang elarlang added the V6 label Jan 3, 2025
@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
@danielcuthbert
Copy link
Collaborator

@randomstuff "Some of these schemes are currently not in the crypto appendix, should be add them?" <-- yes please if you have the time or i will do it this week. The appendix I feel is the best place for the list as it allows us to update that more frequently whereas we cant do this easily with core.

@tghosth tghosth added 3) awaiting proposal There is some discussion in issue and reach to some results but it's not concluded with clear propos and removed 1) Discussion ongoing Issue is opened and assigned but no clear proposal yet labels Jan 27, 2025
@tghosth
Copy link
Collaborator

tghosth commented Feb 5, 2025

@randomstuff is there any action on the requirement itself here or should we close this issue in favour of #2513?

@randomstuff
Copy link
Contributor Author

randomstuff commented Feb 5, 2025

#2513 is about listing some missing MAC algorithms (in the appendix) while this one is about rewording the 6.5.4 which is weirdly formulated.

Can we tweak 6.5.4 and 6.5.2 into something like:

6.5.2 Verify that only approved ciphers and modes are used such as AES-256 with GCM.

6.5.4 Verify that encrypted data is protected against unauthorized modification preferably by using an approved authenticated encryption method or by combining an approved encryption method with an approved MAC algorithm.

And maybe have a dedicated section about approved authenticated encryption methods.

Request feedback from @unprovable and @danielcuthbert

@tghosth
Copy link
Collaborator

tghosth commented Feb 6, 2025

6.5.2 Verify that only approved ciphers and modes are used such as AES-256 with GCM.

6.5.4 Verify that encrypted data is protected against unauthorized modification preferably by using an approved authenticated encryption method or by combining an approved encryption method with an approved MAC algorithm.

I think these changes are fine and don't require further discussion.

Have a dedicated section about approved authenticated encryption methods.

What are you proposing for this?

@randomstuff
Copy link
Contributor Author

Have a dedicated section about approved authenticated encryption methods.

Sorry, I meant:

Have a dedicated section in the appendix about approved authenticated encryption methods.

The idea is that is my wording we have:

6.5.4 Verify that encrypted data is protected against unauthorized modification preferably by using an approved authenticated encryption method or by combining an approved encryption method with an approved MAC algorithm.

So we need to actually have some approved authentication encryption methods (especially ChaCha-Poly1305 which is currently nowehere to be found in the appendix).

Something like:

AEAD mechanism Status
AES-GCM approved
AES-CCM approved
ChaCha-Poly1305 approved
Encrypt-then-MAC approved
MAC-then-encrypt legacy

@tghosth
Copy link
Collaborator

tghosth commented Feb 9, 2025

Ok so please open 2 PRs, one for V6 and one for the appendix.

@randomstuff
Copy link
Contributor Author

As discussed in #2590, maybe 6.5.2 should be merged into 6.2.2.

@tghosth
Copy link
Collaborator

tghosth commented Feb 10, 2025

Ok I merged #2597, what else for this issue @randomstuff?

@randomstuff
Copy link
Contributor Author

@tghosth, I've made a PR for the appendix. It currently uses the presentation form of #2573.

@tghosth tghosth added 6) PR awaiting review _5.0 - Not blocker This issue does not block 5.0 so if it gets addressed then great, if not then fine. AppendixV Appendix with crypto details and removed 3) awaiting proposal There is some discussion in issue and reach to some results but it's not concluded with clear propos _5.0 - prep This needs to be addressed to prepare 5.0 V6 labels Feb 11, 2025
@tghosth
Copy link
Collaborator

tghosth commented Feb 12, 2025

Anything else here @randomstuff ?

@randomstuff
Copy link
Contributor Author

I think we are finished with that.

@tghosth tghosth closed this as completed Feb 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6) PR awaiting review 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

No branches or pull requests

4 participants