Incorrect padding removal condition

[ksivask]

with cbc, the padding char <= block_size, the code in r_jwe_remove_padding incorrectly checks for < instead of <=

  if (pad && pad < (unsigned char)block_size) {

References:
[1] Original Code: https://github.com/babelouest/rhonabwy/blob/master/src/jwe.c#L2258
[2] GnuTLS: https://github.com/gnutls/gnutls/blob/master/lib/crypto-api.c#L519

Sample JWKS:

root@a3ca92d2b7fe:/rhonabwy/build# cat /dummy.jwks
{"keys": [
	{"kty": "oct", "k": "12345zORNt_UA-yuabcdeNPH6Gab2xePB8tE5h12345"}
]}

Sample Data:

a) <32: {'sub':'sivak','uid':8,'vni':4}

EVP_EncryptFinal_ex plaintext 31 enc_raw 32
Encrypted Payload
51 98 03 EE DB 1E E2 E2  37 09 BD D4 CB BE C6 B2
CA CA 24 4C 25 34 A2 4E  73 DA B8 55 88 E6 D9 A3

jwe: 142 eyJhbGciOiAiZGlyIiwgImVuYyI6ICJBMTI4Q0JDLUhTMjU2In0..PkxHOsl5OrNDdkD5jborCQ.UZgD7tse4uI3Cb3Uy77GssrKJEwlNKJOc9q4VYjm2aM.VIn678i1oGLgjuPlbRGCRg

Decrypted Payload
7B 22 73 75 62 22 3A 22  73 69 76 61 6B 22 2C 22
75 69 64 22 3A 38 2C 22  76 6E 69 22 3A 34 7D

plain: 31 31:{"sub":"sivak","uid":8,"vni":4}

b) ==32:{'sub':'sivak','uid':8,'vni':14}

EVP_EncryptFinal_ex plaintext 32 enc_raw 48
Encrypted Payload
B7 C5 99 E9 9E F8 11 50  22 B6 A1 11 25 49 94 1B
F7 96 9B CB 9D B0 FA 0D  56 E9 BC 3F 74 81 CA C8
2E 3C 51 1C DB 6E 7C D5  41 D1 6B B4 86 BB 93 5E

jwe: 163 eyJhbGciOiAiZGlyIiwgImVuYyI6ICJBMTI4Q0JDLUhTMjU2In0..xkholdGHy-Kssu98WUyk5Q.t8WZ6Z74EVAitqERJUmUG_eWm8udsPoNVum8P3SBysguPFEc22581UHRa7SGu5Ne.EEOjbucoMmknNFODVGa_AQ

Decrypted Payload
7B 22 73 75 62 22 3A 22  73 69 76 61 6B 22 2C 22
75 69 64 22 3A 38 2C 22  76 6E 69 22 3A 31 34 7D

plain: 32 32:{"sub":"sivak","uid":8,"vni":14}

c) >32:{'sub':'sivak','uid':8,'vni':148}

Encrypted Payload
89 0F 4F 20 BE 0E DA F9  43 DA 7E 30 E6 08 EB C0
3D DD 2D AB F2 7C 83 ED  46 4B B7 97 80 62 8C DB
43 3D 5A 81 D6 97 88 0A  A7 AA 41 A4 E1 9A D7 F0

jwe: 163 eyJhbGciOiAiZGlyIiwgImVuYyI6ICJBMTI4Q0JDLUhTMjU2In0..e9m40nF7Axi-0MiYtxK45Q.iQ9PIL4O2vlD2n4w5gjrwD3dLavyfIPtRku3l4BijNtDPVqB1peICqeqQaThmtfw.DHvomt2nmLVx00Usp8pWOQ

Decrypted Payload
7B 22 73 75 62 22 3A 22  73 69 76 61 6B 22 2C 22
75 69 64 22 3A 38 2C 22  76 6E 69 22 3A 31 34 38
7D

plain: 33 33:{"sub":"sivak","uid":8,"vni":148}

Pre-Fix (using apt install rnbyc)

(a)
root@40a9640e6589:/# rnbyc -K /dummy.jwks -d -H -t "eyJhbGciOiAiZGlyIiwgImVuYyI6ICJBMTI4Q0JDLUhTMjU2In0..PkxHOsl5OrNDdkD5jborCQ.UZgD7tse4uI3Cb3Uy77GssrKJEwlNKJOc9q4VYjm2aM.VIn678i1oGLgjuPlbRGCRg"
2024-05-04T06:36:11Z - rnbyc INFO: Starting rnbyc debug mode
Token payload decrypted
{
  "alg": "dir",
  "enc": "A128CBC-HS256"
}
{
  "sub": "sivak",
  "uid": 8,
  "vni": 4
}

(b)
root@40a9640e6589:/# rnbyc -K /dummy.jwks -d -H -t "eyJhbGciOiAiZGlyIiwgImVuYyI6ICJBMTI4Q0JDLUhTMjU2In0..xkholdGHy-Kssu98WUyk5Q.t8WZ6Z74EVAitqERJUmUG_eWm8udsPoNVum8P3SBysguPFEc22581UHRa7SGu5Ne.EEOjbucoMmknNFODVGa_AQ"
2024-05-04T06:36:20Z - rnbyc INFO: Starting rnbyc debug mode
Unable to decrypt payload 3       <----
{
  "alg": "dir",
  "enc": "A128CBC-HS256"
}
{}

(c)
root@40a9640e6589:/# rnbyc -K /dummy.jwks -d -H -t "eyJhbGciOiAiZGlyIiwgImVuYyI6ICJBMTI4Q0JDLUhTMjU2In0..e9m40nF7Axi-0MiYtxK45Q.iQ9PIL4O2vlD2n4w5gjrwD3dLavyfIPtRku3l4BijNtDPVqB1peICqeqQaThmtfw.DHvomt2nmLVx00Usp8pWOQ"
2024-05-04T06:36:03Z - rnbyc INFO: Starting rnbyc debug mode
Token payload decrypted
{
  "alg": "dir",
  "enc": "A128CBC-HS256"
}
{
  "sub": "sivak",
  "uid": 8,
  "vni": 148
}

Post-Fix

(a)
root@a3ca92d2b7fe:/rhonabwy/build# ./rnbyc -K /dummy.jwks -d -H -t "eyJhbGciOiAiZGlyIiwgImVuYyI6ICJBMTI4Q0JDLUhTMjU2In0..PkxHOsl5OrNDdkD5jborCQ.UZgD7tse4uI3Cb3Uy77GssrKJEwlNKJOc9q4VYjm2aM.VIn678i1oGLgjuPlbRGCRg"
2024-05-04T06:33:05Z - rnbyc INFO: Starting rnbyc debug mode
Token payload decrypted
{
  "alg": "dir",
  "enc": "A128CBC-HS256"
}
{
  "sub": "sivak",
  "uid": 8,
  "vni": 4
}

(b)
root@a3ca92d2b7fe:/rhonabwy/build# ./rnbyc -K /dummy.jwks -d -H -t "eyJhbGciOiAiZGlyIiwgImVuYyI6ICJBMTI4Q0JDLUhTMjU2In0..xkholdGHy-Kssu98WUyk5Q.t8WZ6Z74EVAitqERJUmUG_eWm8udsPoNVum8P3SBysguPFEc22581UHRa7SGu5Ne.EEOjbucoMmknNFODVGa_AQ"
2024-05-04T06:33:31Z - rnbyc INFO: Starting rnbyc debug mode
Token payload decrypted
{
  "alg": "dir",
  "enc": "A128CBC-HS256"
}
{
  "sub": "sivak",
  "uid": 8,
  "vni": 14
}

(c)
root@a3ca92d2b7fe:/rhonabwy/build# ./rnbyc -K /dummy.jwks -d -H -t "eyJhbGciOiAiZGlyIiwgImVuYyI6ICJBMTI4Q0JDLUhTMjU2In0..e9m40nF7Axi-0MiYtxK45Q.iQ9PIL4O2vlD2n4w5gjrwD3dLavyfIPtRku3l4BijNtDPVqB1peICqeqQaThmtfw.DHvomt2nmLVx00Usp8pWOQ"
2024-05-04T06:33:44Z - rnbyc INFO: Starting rnbyc debug mode
Token payload decrypted
{
  "alg": "dir",
  "enc": "A128CBC-HS256"
}
{
  "sub": "sivak",
  "uid": 8,
  "vni": 148
}

[ksivask]

PR: #37

[babelouest]

Hello @ksivask ,

Thanks a lot for pointing out the issue. I realize that the CBC padding implementation is incorrect...

[ksivask]

@babelouest, I have opened the PR #37 for the fix.

[babelouest]

Yes, but the patch fixed the case when a unpadding with 16 iterations of 0x10 was used. There was an error in the padding algorithm too.

I've added some improvements in the fix-cbc-padding branch to cover padding when payload size is a 16-bytes set of blocks, and raise an error when the unpadding is wrong.

This should make it PKCS#7 padding compatible now.