search

decentralized-identity / didcomm-messaging

https://identity.foundation/didcomm-messaging/spec/
Apache License 2.0
163 stars 57 forks source link

Are AES256-GCM and XC20P content encryption algorithms really needed #213

Closed baha-ai closed 2 years ago

baha-ai commented 3 years ago

It seems the only content encryption algorithms supported by ECDH-1PU draft4 are the AES-CBC+HMAC-SHA ones. See discussion in PR-212.

Even though we can still use AES256-GCM and XC20P for Anoncrypt (ECDH-ES), do we really need to support them?

kdenhartog commented 3 years ago

+1 to raising this question. I think it could be favorable for us to drop these options for simplicity, so I'm +1 to using only AES-CBC+HMAC-SHA options.

@awoie I'd think you'd have an opinion on this as well so tagging you to make you aware of it.

awoie commented 3 years ago

If we cannot support XC20PKW for key wrapping (as defined by draft-amringer-jose-chacha-02) then I'm ok with dropping XC20P for content encryption. Otherwise, I'd be in favour of having an alternative option based on XC20PKW + XC20P to the proposed crypto suites.

awoie commented 3 years ago

It would be nice to get a statement from the ECDH-1PU author on XC20PKW/XC20P.

awoie commented 3 years ago

After reading the response from the 1PU author, I'm ok with dropping XC20P @kdenhartog @Baha-sk .

OR13 commented 3 years ago

I'm +1 to using only AES-CBC+HMAC-SHA options. ... Even though we can still use AES256-GCM and XC20P for Anoncrypt (ECDH-ES), do we really need to support them?

https://www.iana.org/assignments/jose/jose.xhtml#web-signature-encryption-algorithms

I am struggling to understand why this WG keeps trying to make it impossible to use both REGISTERED and APPROVED crypto for didcomm v2.

Certainly nobody should be prevented from using unregistered / experimental / cutting edge crypto...

But making it impossible to implement DIDComm v2 using only off the shelf / widely available tooling seems like either a very clever vendor lock in strategy or terrible security engineering.

Maybe I am misreading this issue...

I want to use DIDComm v2 with only registered and NIST/FIPS approved crypto...how do I do that now, and will I be able to do that in the future?

EDIT:

https://datatracker.ietf.org/doc/html/rfc7518#section-5.1

... "A256CBC-HS512" is registered and "required".

This would appear to answer my question.

Related: https://security.stackexchange.com/questions/234202/performance-comparison-between-aes256-gcm-vs-aes-256-sha-256

additional guidance regarding AES-GCM

Unless you absolutely need AES-GCM, use XChaCha20-Poly1305

I would say a general best practice would be to have one FIPS / NIST / Legacy recommended...

and 1 new age / unregistered cutting edge... and no more...

So a hypothetical recommendation would look like:

  1. ECDH-1PU+A256KW - > A256CBC-HS512
  2. ECDH-ES+A256KW -> A256GCM
  3. ECDH-ES+A256KW -> XC20P
baha-ai commented 3 years ago

I want to use DIDComm v2 with only registered and NIST/FIPS approved crypto...how do I do that now, and will I be able to do that in the future?

Currently you can only do Anoncrypt with registered NIST/FIPS approved crypto 100%. It will require the use of JWS to include a non repudiable sender messages.

The answer to getting non repudiable messages for the recipients only (without embedding JWS in a JWE message) is ECDH-1PU. It uses all the same registered NIST/FIPS approved crypto including KDFs and KW with the exception (additions I would say) of 2 things:

  1. Adds an additional KDF with the sender key and concatenate it with the original kdf.
  2. Includes the encryption tag in the KDF process to protect the message authenticity from unwanted impersonations of the sender by a malicious recipients.

anything else is standard NIST/FIPS/IETF crypto in the 1PU process.

OR13 commented 3 years ago

@Baha-sk thanks for the update, can you provide an example pairing for "Recommended" vs "Registered and Approved".

It sounds like you could drop A256GCM in favor of only A256CBC-HS512 and still have an option that is both "Registered and Approved"... The question would be... how much pain is that for implementers too support / is it supported widely today?

baha-ai commented 3 years ago

It sounds like you could drop A256GCM in favor of only A256CBC-HS512 and still have an option that is both "Registered and Approved"... The question would be... how much pain is that for implementers too support / is it supported widely today?

@OR13 The aim in the docs here is to add minimal required algs for implementations while sticking to the standards/specs as much as possible. For Anoncrypt, content encryption algorithms that can be supported are:

For Authcrypt, only 1 content encryption algorithm is required (only AES-CBC+HMAC_SHA family of algorithms should be allowed for Authcrypt, any other algs should be rejected):

All other algs are optional, it's up to implementations to decide if they need to support them.

From the above algs, all are supported/widely used I would say except for ECDH-1PU+A256KW which is the novelty we're introducing here.

as you linked earlier @OR13 , existing allowed content encryption algorithms in JWE are listed here: https://datatracker.ietf.org/doc/html/rfc7518#section-5.1 and Key wrapping algorithms: https://datatracker.ietf.org/doc/html/rfc7518#section-4.6 (table at the end of the section, we only intend to support ECDH key wrappings, so no Direct mode and no RSAES either)

baha-ai commented 3 years ago

I would actually remove XC20P alg from the list above, since it's not a JWE standard one.

OR13 commented 3 years ago

DIDComm WG

Recommended

Supported

I think the optionality of A256GCM or A256CBC-HS512 for "supported" seems like a mistake.

baha-ai commented 3 years ago

as per https://datatracker.ietf.org/doc/html/rfc7518#section-5.1,

A256GCM is recommended while A256CBC-HS512 is required.

So A256CBC-HS512 is required by both Anoncrypt and Authcrypt, this makes A256GCM as optional for Anoncrypt (and rejected for Authcrypt since it only accepts CBC+HMAC algs).

baha-ai commented 3 years ago

@OR13, maybe Anoncrypt/Authcrypt terminology doesn't make much sense, so here's a summary of key wrapping algs and content encryption ones that can work together:

OR13 commented 3 years ago

@troyronda @Baha-sk can you rephrase your comments in terms of what "DIDComm WG: Recommends vs Supports"

Does my edit above match what you expect?

baha-ai commented 3 years ago

@troyronda @Baha-sk can you rephrase your comments in terms of what "DIDComm WG: Recommends vs Supports"

@OR13 not sure if you posted your comment prior to seeing my previous comment, both comments were less than a minute apart.

But to clarify one more time, when sender info is not revealed, then both A256CBC-HS512 and A256GCM content (or payload) encryption must be supported and for key wrapping only ECDH-ES+A256KW is required.

When sender identity needs to be revealed (to authenticate the sender), then only A256CBC-HS512 is required (AESGCM should be rejected, not optional) and for key wrapping only ECDH-1PU+A256KW is required.

Currently the docs state that XC20P content encryption algorithm should be supported as well. This GH issue was created to discuss if we should remove it (and possibly remove AES256GCM as well) from the list of required algorithms and only stick to A256CBC-HS512.

The DIDcomm docs only mention required algorithms to be NIST/FIPS compliant and must be supported to guarantee interoperability. Optional algorithms are left to the discretion of implementers. They don't guarantee interoperability or compliance, this is why they're not listed in the docs.

OR13 commented 3 years ago

Which of these 2 is more popular for "Sender identity not revealed"

OR

In my experience, A256GCM + ECDH-ES+A256KW would be better to keep and the spec should revised to state:

Sender identity not revealed (no sender key involved in key wrapping for recipients):

Sender identity revealed to the recipients (sender key involved in key wrapping for recipients):

In order to decide this, you would need to sample off the shelf implementations and see how many support each option.

Only popular options => Less options => More interop...

sometimes, we will need to do "Unpopular, but better" => less options => more interop.

baha-ai commented 3 years ago

As far as AFGO is concerned, we support A256GCM, AES-CBC+HMAC-SHA and also the experimental XC20P encryption type. The CBC-HMAC constraint comes from the 1PU draft when revealing the sender's identity is needed. Other than this constraint, nothing stops us from listing the required algs for anonymous encryption.

We can think of popularity or better security (separate HMAC digest in CBC+HMAC tend to be more secure?), but we should aim for the least common denominator of algs to increase DIDcomm interop for both anonymous and sender authenticated encryptions.

kdenhartog commented 3 years ago

I'm +1 to removing A256GCM and XC20P for simplicity because neither one of them gives us a compact committing with an AEAD scheme.

With that in mind, I would like for us to explore if there's any other optional symmetrical ciphers that we could use that meet the AEAD CC requirement. If we're left with only one (AES-CBC-HS512) then that's good enough for a v2 baseline and we can add additional cryptographic ciphers later on as extensions.

OR13 commented 3 years ago

you generally want to avoid a "only 1 option scenario" in crypto... because sometimes that options gets broken.

I would recommend you pick something old that has stood the test of time, and something new that has desirably properties, and support both.

baha-ai commented 3 years ago

For now ECDH-1PU only supports the AES-CBC+HMAC-SHA family of algorithms which leaves with 3 algs:

In the future, if the list of required algs gets updated in rfc7518 (or in ECDH-1PU draft), we can probably add new algs here as well. Or we can try experimental/newer compactly committing AEAD algs (is XChaca+Blake2b one of them ? or AES_CTR+HMAC_SHA which is not in RFC7518 either) but can't commit to making them mandatory unless they pass NIST/FIPS approval (which we know will not happen soon enough for the life of DIDComm V2).

Again, the goal is to have a minimum number of encryption algs required for agent interoperability in DIDComm V2. For now, it seems AES256_CBC+HMAC_SHA512 (aka A256CBC-HS512) is the only viable envelope encryption algorithm that meets all the needed security criteria:

kdenhartog commented 3 years ago

you generally want to avoid a "only 1 option scenario" in crypto... because sometimes that options gets broken.

I would recommend you pick something old that has stood the test of time, and something new that has desirably properties, and support both.

I agree with the sentiment, but haven't had time to evaluate the other options to make sure they meet our security needs. Maybe we can rely upon Yolan from SICPA to assisst with that.

For me personally, I think it's fine if we have one required NIST approved implementation and one non-NIST approved. There's inherent political aspects at play when it comes to algorithm selection and placing a requirement that all algorithms have to be NIST approved introduces an overly western bias.

For example, with TLS it's common that Russian servers don't rely upon NIST based algorithms. An example of their cipher used is documented here: https://tools.ietf.org/id/draft-chudov-cryptopro-cptls-04.html I believe this is what @vimmerru was trying to point out in some of the other issues he opened.

My takeaway from this is that:

  1. Cryptographic agility is a MUST (even if we choose to support only one profile of crypto for now)
  2. Overly agile crypto harms interoperability so there must be a balance
  3. It would be a nice to have to integrate another crypto profile as an optional alternative to demonstrate cryptographic agility and make it a point of consideration for implementors so they don't logically bind the crypto to other parts of their implementation.
TelegramSam commented 3 years ago

The point I want to emphasize:

Cryptographic agility is a MUST (even if we choose to support only one profile of crypto for now)

This is done by specifying our cryptographic options, (which technically allows many options), but restricting the allowed options in the spec. this allows later expansion / change / movement to better crypto when it happens.

AnomalRoil commented 3 years ago

I fully agree that cryptographic agility, in the sense of "it must be fairly easy to replace a given algorithm by another without having to change too much of the code base" is a must nowadays, especially since we can already foresee a future where NIST might be requiring "post-quantum" algorithms instead of the current ones.

I also agree it doesn't hurt to have a NIST/FIPS approved algorithm, but it shouldn't close the door to algorithms that are unrelated to NIST in the case we have users that are wary of NIST "random" constants... So having "optional" algorithm recommendations sounds like a good idea.

On the flip side, having too many algorithm in the spec might be making DIDComm implementation unnecessarily complex.

We might take the PGP approach: have a "fallback mandatory" algorithm that all impl are supposed to support to ensure interoperability, and then have optional ones, or new ones added through later RFC processes or extensions.

But in any cases we might want to come up with some proposals to solve this question rather sooner than later in order to have a final spec asap :)

RE:

not recommended since 16 bytes encryption keys are weak

Well, currently we have X25519 as a key agreement process which is "only" featuring ~126 bits or security, so I don't think we should really limit ourselves to 256 bits schemes on the symmetric side.

TelegramSam commented 2 years ago

Discussed 20220110. We need a discussion of algorithms that end up in the spec.

TelegramSam commented 2 years ago

Discussed WG 20220131. Survey of existing implementations to see if they support it or want to. Folks can always exceed what is required by the spec, and use it with others that do. Daniel will pursue.

dhh1128 commented 2 years ago

Tagging the following people, each of whom is a significant contributor to an implementation of a library that implements DIDComm v2: @simonas-notcat, @mirceanis, @brianorwhatever, @vimmerru, @ashcherbakov, @moopli, @35359595, @ademcaglin, @andrewwhitehead, @andkononykhin, @asmarty, @spivachuk

It's been proposed that the DIDComm v2 spec should NOT require support for AES256-GCM and XP20P. I have the following crisp question for each of you:

Does your implementation already support these two algorithms? If not, do you want to add it, or would you vote against the requirement because you don't want to accept a new requirement?

(For the guys from DSR: I mainly want to know if we already have support; I assume you don't want to add new features unless you're paid to do so... :-)

OR13 commented 2 years ago

https://www.iana.org/assignments/jose/jose.xhtml#web-signature-encryption-algorithms

I would recommend 2 MUST implement algorithms from that list, leave the rest of the list as MAY.

I would add language to the effect of DIDComm implementations SHOULD NOT rely on unregistered algorithms.

I am planning on using off the shelf libraries to implement DIDComm, and they tend to only support registered algorithms.

mirceanis commented 2 years ago

@dhh1128 we use ECDH-ES+XC20PKW and ECDH-1PU+XC20PKW, but we plan to use more off-the-shelf libraries to add support for more.

35359595 commented 2 years ago

Tagging the following people, each of whom is a significant contributor to an implementation of a library that implements DIDComm v2: @simonas-notcat, @mirceanis, @brianorwhatever, @vimmerru, @ashcherbakov, @Moopli, @35359595, @ademcaglin, @andrewwhitehead, @andkononykhin, @asmarty, @spivachuk

It's been proposed that the DIDComm v2 spec should NOT require support for AES256-GCM and XP20P. I have the following crisp question for each of you:

Does your implementation already support these two algorithms? If not, do you want to add it, or would you vote against the requirement because you don't want to accept a new requirement?

(For the guys from DSR: I mainly want to know if we already have support; I assume you don't want to add new features unless you're paid to do so... :-)

didcomm-rs implements AES256-GCM, XC20P and AES-256-CBC (encryption ATM). I dont mind any changes at all while algorithm spec is finalized and there is solid native rust implementation of it :)

ashcherbakov commented 2 years ago

@dhh1128 All our implementations (didcomm-jvm, didcomm-python, didcomm-rust, didcomm-ts, didcomm-swift) already support all three algorithms:

TelegramSam commented 2 years ago

Discussed WG 20220207. Options for resolution are:

verify NIST approved algorithms (Steve will pursue)

  1. make the two anoncrypt options MAY
  2. make only the NIST approved anoncrypt required, other as a MAY.
  3. Leave as is, all 3 a MUST.
mccown commented 2 years ago

tl;dr: either keep things as they are or just require the NIST algorithms

In the DIDComm WG today, the question of which algorithms are necessary arose and I volunteered to research it a bit. Here’s some of what I found:

1) For NIST requirements (USG, contractors, vendors, etc.), NIST SP 800-131A Rev. 2 (https://nvlpubs.nist.gov/nistpubs/SpecialPublications/NIST.SP.800-131Ar2.pdf) specifies that: “…AES-256 is acceptable for encryption and decryption using the approved modes in the SP 800-38 series of publications." SP 800-38 (https://csrc.nist.gov/projects/block-cipher-techniques/bcm/current-modes) lists several modes (incl. GCM and CBC). 


2) In Decentralized Identity, the “Public Key Authenticated Encryption for JOSE: ECDH-1PU” (https://datatracker.ietf.org/doc/html/draft-madden-jose-ecdh-1pu-04) is normally referenced and uses both A256GCM and A256CBC-HS256 in its key agreement processes.

3) XC20P (https://tools.ietf.org/id/draft-amringer-jose-chacha-02.html): This document defines how to use the AEAD algorithms in JOSE. However, it seems to be in “Informational” status. Is there a better reference?

For projects with NIST requirements, it seems like A256GCM (Anoncrypt; JOSE recommended) and A256CBC-HS512 (Authcrypt/Anoncrypt; JOSE required) are required and that XC20P (not JOSE listed) is still a frequently requested option for non-NIST efforts.

With that in mind, I would propose that we either just keep the 2 NIST-based algorithms as mandatory or, for interoperability’s sake, that we maybe leave things as they are.

vimmerru commented 2 years ago

verify NIST approved algorithms (Steve will pursue)

  1. make the two anoncrypt options MAY
  2. make only the NIST approved anoncrypt required, other as a MAY.
  3. Leave as is, all 3 a MUST.

It would be good to have at least one non-NIST option for all use cases. Now authcrypt forces A256CBC-HS512 that is show stopper for a lot of use cases outside US.

TelegramSam commented 2 years ago

Discussed WG 20220214. Troy to make PRs adjusting language.

troyronda commented 2 years ago

Updated in https://github.com/decentralized-identity/didcomm-messaging/pull/336