Fix JWE encryption not setting protected header correctly

[hawk259]

jose_jwe_enc using elliptic curve keys does not add the algo or epk values to the protected header value it generates.

These values will be missing when generating a compact serialized JWE string and will fail decrypting because the protected value is missing them.

The patch below tweaks things to include them in the protected value.

lib/jwe.c change to match lib/jws.c putting it protected instead of header.

lib/openssl/ecdhes.c changes to put things in both protected and header.

I do not have test cases for other encryption cases, but I think "protected" might need to be added to the add_entity call in other files in lib/openssl.

[sarroutbi]

Hello @hawk259 : this change seems to break unit tests ... is it because they need to be changed? I am not sure change can be merged if unit tests are broken ...

[hawk259]

I have been trying to figure out how to fix the tests, but not making much progress.

Here is a test that you can try to add to see how things break:

1. setup keys and call jose_jwe_enc on a string
2. generate a JWT token string using <protected>.<encrypted_key>.<iv>.<ciphertext>.<tag> keys from jwe object
3. Using that string, split on ., create a json object with the above keys, setup keys and call jose_jwe_dec passing that object.

The jose_jwe_dec will fail because it doesn't have enough info in the protected value to decrypt the strings. The problem is the things needed are saved in the header value, but not copied to protected value.

[hawk259]

We ran into this issue again trying to test different RSA keys and I wrote a bash script using the command line tool, here is an overview:

Create a jwk:
jose jwk gen -i '{"kty":"RSA","bits":2048}' -o key.jwk

Create a jwe:
jose jwe enc -r '{"header": {"alg": "RSA-OAEP", "enc": "A128GCM"}}' -I data.txt -k key.jwk -o data.jwe

Decrypt jwe [WORKS]:
jose jwe dec -i data.jwe -k key.jwk -O data.out

Compact jwe:
jose jwe fmt -i data.jwe -o data.jwe-c -c

Decrypt compact jwe [FAILS]:
jose jwe dec -i data.jwe-c -k key.jwk -O data.out-c

This fails because the compact protected header is missing key fields needed to decrypt. The jwe has

  "header": {
    "alg": "RSA-OAEP",
    "enc": "A128GCM"
  },

in it, but the compact version protected header only has

{"enc":"A128GCM"}

At the bottom of the script it generates a new compact file with the correct protected header:

{"alg": "RSA-OAEP", "enc": "A128GCM"}

Decrypt compact jwe v2 [FAILS, but data matches]:
jose jwe dec -i data.jwe-c2 -k key.jwk -O data.out-c2

The file that is written out matches the original data, but for some reason the command failed.

I also added EC commands to show the issue as well. The EC jwe file contains

  "header": {
    "alg": "ECDH-ES+A128KW",
    "enc": "A128CBC-HS256",
    "epk": {
      "crv": "P-256",
      "kty": "EC",
      "x": "<snip>",
      "y": "<snip>"
    }

but the compacted jwe protected field only contains

{"enc":"A128CBC-HS256"}

which is not enough information to decrypt the compact jwe.

[sarroutbi]

@hawk259 : can you please rebase to check what is the current status with Unit tests?

[hawk259]

Update fork to latest code. This patch is not the real fix because it only works for EC via shared library API calls, doesn't fix the command line and breaks the unit tests.

The script I added shows the issue, which is simple... using jose cli to create a jwk, enc data, fmt to compact/serialize format can not be decrypted because the protected header is missing important data.

[sarroutbi]

Update fork to latest code. This patch is not the real fix because it only works for EC via shared library API calls, doesn't fix the command line and breaks the unit tests.

The script I added shows the issue, which is simple... using jose cli to create a jwk, enc data, fmt to compact/serialize format can not be decrypted because the protected header is missing important data.

I see. I was working last weekend on verifying why the tests were failing, but could not get to a final patch yet. I will continue investigating

[hawk259]

Digging into this more I have fixed the jose cli program with a simple change, forcing this if block to always execute will make the jose cli work correctly.

I am still digging into the share lib API, but I can tell you all the unit tests for jwe are broken. They all take a json object (jwe) as input, call jose_jwe_enc which updates the jwe and takes the same json object to pass into jose_jwe_dec. This works because all the field values that are needed are there (just in the wrong spots).

After the jose_jwe_enc call, add json_object_del(jwe, "header"); and then call jose_jwe_dec and it will break every unit test for jwe.

Deleting the header value from the json object (jwe) is like turning it in the compact format. After calling jose_jwe_enc the json object looks like this (for RSA):

{
    "protected": "<snip, includes enc key>",
    "header": {
        "alg": "<snip>"
    },
    "encrypted_key": "<snip>",
    "iv": "<snip>",
    "tag": "<snip>",
    "ciphertext": "<snip>"
}

The compact format is <protected>.<encrypted_key>.<iv>.<ciphertext>.<tag>, all the fields except header. Delete the header field and jose_jwe_dec will fail because alg (and epk for EC) are not in the protected field where they should be.

Doing jose_jwe_enc, json_object_del(jwe, "header");, jose_jwe_dec is the same as calling the jose cli enc, fmt with -c and dec. It was failing too because protected header wasn't getting the values it needed until we forced all the time.

This making sense?

[hawk259]

I have updated the patch and I think I have everything working.

cmd/jwe/enc.c this makes the compact code the default, so the right values get added to the protected header all the time. This only fixes the jose cli.

lib/jwe.c calling jose_jwe_hdr and passing jwe for both values will take the current protected field and the header field and combine them, then reset it as the protected field, making sure all the right values are set in the protected value. This only fixes the lib api calls.

tests/jose-jwe-enc this updates the tests to look in the protected value instead of the header value for things.

[sarroutbi]

Please, if possible, fix indentation of cmd/jwe/enc.c

If not possible, let me know and I will update the patch to fix it.

Thanks for your PR, great job !!!

[hawk259]

I think I fixed the indention. Today I will be replacing our patch with this in our internal build and running through our code testing to make sure nothing is broken. I don't believe there are any issues, but will report back either way.

[sarroutbi]

@sergio-correia : Could you PTAL?

[hawk259]

Patch looks good for our internal build and all the unit tests work, will be updating our build on Monday.

[sergio-correia]

Thanks, @hawk259!