proxyco / swift-jose

Swift implementation of the JSON Object Signing and Encryption (JOSE) family of specifications
MIT License
4 stars 2 forks source link

Issue with ECDH-1PU-A256KW key agreement algorithm #1

Open jay-jay-lama opened 1 year ago

jay-jay-lama commented 1 year ago

Hello I tried version from main branch And got an error during encryption process

-CryptoKitError
  - underlyingCoreCryptoError : 1 element
    - error : -7

Code

let privateKey = P256.KeyAgreement.PrivateKey()
let publicKey = privateKey.publicKey
let header: JOSEHeader =  .init(algorithm: .ecdh1PUA256KW, encryptionAlgorithm: .a256CBCHS512)

try JWE.encrypt(
             plaintext: "Test".data(using: .utf8)!,
             to: publicKey.jwkRepresentation,
             from: privateKey.jwkRepresentation,
             protectedHeader: header)

We got this issue here encryptedKey = try AES.KeyWrap.wrap(.init(data: contentEncryptionKey), using: .init(data: keyEncryptionKey)) When we are trying to create encryptedKey for receiver.

jay-jay-lama commented 1 year ago

I think that the problem is with the contentEncryptionKey

zssz commented 1 year ago

The key management algorithm, ecdh1PUA256KW, is incompatible with the content encryption algorithm a256CBCHS512. This is due to the fact that a256CBCHS512 generates a 512-bit content encryption key, which ecdh1PUA256KW cannot accommodate because CryptoKit.AES.KeyWrap only supports keys of up to 256 bits.

To resolve this, you have two options:

Option 1: Switch to using a128CBCHS256:

let header: JOSEHeader = .init(algorithm: .ecdh1PUA256KW, encryptionAlgorithm: .a128CBCHS256)

Option 2: Opt out of AES key wrapping:

let header: JOSEHeader = .init(algorithm: .ecdh1PU, encryptionAlgorithm: .a256CBCHS512)
jay-jay-lama commented 1 year ago

Thank you a lot for your answer Option 1 is working.

I'm sorry for one more question. I'm new to cryptography, so if anything you just can tell that this question is bullshit ))))

I see that during the using a256CBCHS512 we generate a key with length 512 witch is divided into 2 parts : MAC_KEY and ENC_KEY the length of which is 32 and 32 bytes. (According to documentation)

MAC_KEY - is used to calculate Authentication Tag ENC_KEY - is used to encrypt plainText

Shouldn't ENC_KEY be used to wrap the key?

And also i looked into the implementation of this a256CBCHS512 algorithm in the library jose swift

Here you can see that even with using A256CBCHS512 they get key for key wrapping with length 256

And here i can see that they are using ENC_KEY to implement key wrapping. Function

func encrypt(_ plaintext: Data, additionalAuthenticatedData: Data) throws -> ContentEncryptionContext 

Default CryptoKit.AES.KeyWrap and custom wrap implementation in this library returns the equal result

Implementation of key generating

func retrieveKeys(from inputKey: Data) throws -> (hmacKey: Data, encryptionKey: Data) {
           switch self {
           case .A256CBCHS512:
               guard checkKeyLength(for: inputKey) else {
                   throw JWEError.keyLengthNotSatisfied
               }

               return (inputKey.subdata(in: 0..<32), inputKey.subdata(in: 32..<64))

           case .A128CBCHS256:
               guard checkKeyLength(for: inputKey) else {
                   throw JWEError.keyLengthNotSatisfied
               }
               return (inputKey.subdata(in: 0..<16), inputKey.subdata(in: 16..<32))
           }
       } 

I just want to understand witch approach is right... Thank you

zssz commented 1 year ago

The confusion is understandable, cryptography can be a complex field! To start with, it's important to note that the key used for AES Key Wrapping and the encryption key used for the content encryption process are not the same.

When using AES Key Wrap, you're wrapping (encrypting) the content encryption key (CEK). The wrapped CEK is then transported securely to the recipient, who unwraps it (decrypts it) to recover the original CEK. This CEK is what's used for encrypting and decrypting the content.

Now, coming to your question about the a256CBCHS512 algorithm, it does indeed generate a 512-bit key which is divided into two 256-bit halves, one for MAC (Message Authentication Code) and one for encryption. However, the key used for encryption here is specifically for encrypting the plaintext data, not for wrapping the CEK.

The function you referred to in the jose swift library is used to generate the MAC_KEY and ENC_KEY from the provided input key (the CEK). This is done after the CEK has been unwrapped.

The key for AES Key Wrap (the key encryption key, or KEK) comes from the key agreement process (ecdh1PU or ecdh1PUA256KW in your example). This is a separate key that's generated by each party independently, using their own private key and the other party's public key.

The observation you made about the jose swift library using ENC_KEY for key wrapping appears to be a misunderstanding. The key wrapping process should be using the KEK derived from the key agreement process, not the ENC_KEY.

In summary, the approach where a 512-bit key is split into two 256-bit halves, one for MAC and one for encryption, is correct. The important thing to remember is that the keys for AES Key Wrap and content encryption are derived from different processes and serve different purposes.

I hope this helps to clarify things!

jay-jay-lama commented 1 year ago

Hello Thank you a lot for this answer and for your time. You helped me to understand more in this topic 🙏

jay-jay-lama commented 1 year ago

Hello I just wanted to put your attention to that fact that ecdh1PUA256KW and a256CBCHS512 should work.

We have the problem with this function CryptoKit.AES.KeyWrap right? It works with 2 values: key to wrap - which is our content key with 512 length (this length can be any) key encryption key - which is our agreed key... and it's length shouldn't be more than 256

So the problem is with the key encryption key because its length now depends on content alg key length. But according to documentation

keydatalen  This is set to the number of bits in the desired output
      key.  For "ECDH-1PU", this is the length of the key used by the
      "enc" algorithm.  For "ECDH-1PU+A128KW", "ECDH-1PU+A192KW", and
      "ECDH-1PU+A256KW", this is 128, 192, and 256, respectively.

It should be equal to 256 because of A256KW... we should take the "enc" key length only for "ECDH-1PU" So the problem in deriveKey function line 71 let keyDataLen = encryptionAlgorithm.keySizeInBits

beatt83 commented 9 months ago

Hi there. :) First let me say I really like a few components on this framework. They are really well designed.

I had the same problem and actually was checking a few things, I noticed that there is actually a bug in this key management algorithm, in the deriveKey. Since this algorithm requires you to encrypt the data BEFORE generating the shared key, because you need the authenticationTag when deriving the shared key.

This is documented in the ECDH-1PU draft rfc https://datatracker.ietf.org/doc/html/draft-madden-jose-ecdh-1pu-04#section-2.3

jay-jay-lama commented 9 months ago

@beatt83 Hey Thank you a lot Wow i've really missed it in the documentation I hope that developers of this awesome library will fix these tiny things Thank you @zssz one more time. Your implementation is really helped me so much