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

Error decrypting JWE created from jose in v4.3.4 vs node-jose in v4.0.7 #692

Open
zionsg opened this issue Sep 14, 2024 · 2 comments
Open

Comments

@zionsg
Copy link

zionsg commented Sep 14, 2024

Describe the bug

I am currently working on using MockPass with MyInfo Gov Client in my Demo App.

When calling the /person endpoint for MyInfo Personal in MockPass, the JWE from the response cannot be decrypted by MyInfoGovClient, namely _decryptJWE() in https://github.com/opengovsg/myinfo-gov-client/blob/v4.1.2/src/MyInfoGovClient.class.ts as it assumes the payload is wrapped in quotes and attempts JSON.parse().

Traced it to the payload not being wrapped in quotes, due to the switch from node-jose to jose package in MockPass.

// v4.0.7 using node-jose - https://github.com/opengovsg/mockpass/blob/v4.0.7/lib/express/myinfo/controllers.js
Result: { 
  key: JWKBaseKeyObject {
    keystore: JWKStore {},
    length: 2048,
    kty: 'RSA',
    kid: 'bza0dXf6Fjla1FQrVKmATuZb9-4M90LxDuf3ujLYbqg',
    use: '',
    alg: ''
  },
  header: {
    enc: 'A128CBC-HS256',  
    alg: 'RSA-OAEP',  
    kid: 'bza0dXf6Fjla1FQrVKmATuZb9-4M90LxDuf3ujLYbqg'
  },
  protected: [ 'enc', 'alg', 'kid' ],
  plaintext: <Buffer 22 65 79 4a 68 62 47 63 69 4f 69 4a 53 55 7a 49 31 4e 69 49 73 49 6e 52 35 63 43 49 36 49 6b 70 58 56 43 4a 39 2e 65 79 4a 75 59 57 31 6c 49 6a 70 37 ... 823 more bytes>,
  payload: <Buffer 22 65 79 4a 68 62 47 63 69 4f 69 4a 53 55 7a 49 31 4e 69 49 73 49 6e 52 35 63 43 49 36 49 6b 70 58 56 43 4a 39 2e 65 79 4a 75 59 57 31 6c 49 6a 70 37 ... 823 more bytes>
} 

Payload: "eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCJ9.eyJuYW1lIjp7Imxhc3R1cGRhdGVkIjoiMjAyMC0wNC0xNiIsInNvdXJjZSI6IjEiLCJjbGFzc2lmaWNhdGlvbiI6IkMiLCJ2YWx1ZSI6IkxJTSBZT05HIFhJQU5HIn0sImVtYWlsIjp7Imxhc3R1cGRhdGVkIjoiMjAyMC0wNC0xNiIsInNvdXJjZSI6IjIiLCJjbGFzc2lmaWNhdGlvbiI6IkMiLCJ2YWx1ZSI6Im15aW5mb3Rlc3RpbmdAZ21haWwuY29tIn0sIm1vYmlsZW5vIjp7Imxhc3R1cGRhdGVkIjoiMjAyMC0wNC0xNiIsInNvdXJjZSI6IjIiLCJjbGFzc2lmaWNhdGlvbiI6IkMiLCJhcmVhY29kZSI6eyJ2YWx1ZSI6IjY1In0sInByZWZpeCI6eyJ2YWx1ZSI6IisifSwibmJyIjp7InZhbHVlIjoiOTczOTkyNDUifX0sImlhdCI6MTcyNjMwMDQwMn0.miKImZU9xOEgAN2FWXtdYKZQtG_J4yjTCxnadP0-s5pNsUmNpJiffFSFHibcvsRnQTG_c2xZx_C8XHov_AOedLHp5MbClYq_VTidGfLdkemkpfhLw6eliuyHyeiD0cpLI8XUInGf5JAfzXdQdWUUV5sEH1U-EzXGJ_lNxYUI0jC6MGpW44H52z6S5RyFwAto_tIIc_Ka4-slr0FJIJhfOPmy6UZU3dz0Ozm2784a7skJ2VYHJ3l7kBbTQxtR-NMjGGcMNaWAOOfkDU9czIwn4h8aqGoVLEsHtjYn8P3-uu1wcQtALwcgkUSpXno1VzWoHo-fk0sq1RyDIx3I32O89A"

// v4.3.4 using jose - https://github.com/opengovsg/mockpass/blob/v4.3.4/lib/express/myinfo/controllers.js
Result:  {
  key: JWKBaseKeyObject {
    keystore: JWKStore {},
    length: 2048,
    kty: 'RSA',
    kid: 'bza0dXf6Fjla1FQrVKmATuZb9-4M90LxDuf3ujLYbqg',
    use: '',
    alg: ''
  },
  header: { 
    alg: 'RSA-OAEP', 
    enc: 'A256GCM' 
  },
  protected: [ 'alg', 'enc' ],
  plaintext: <Buffer 65 79 4a 68 62 47 63 69 4f 69 4a 53 55 7a 49 31 4e 69 4a 39 2e 65 79 4a 75 59 57 31 6c 49 6a 70 37 49 6d 78 68 63 33 52 31 63 47 52 68 64 47 56 6b 49 ... 782 more bytes>,
  payload: <Buffer 65 79 4a 68 62 47 63 69 4f 69 4a 53 55 7a 49 31 4e 69 4a 39 2e 65 79 4a 75 59 57 31 6c 49 6a 70 37 49 6d 78 68 63 33 52 31 63 47 52 68 64 47 56 6b 49 ... 782 more bytes>
} 

Payload: eyJhbGciOiJSUzI1NiJ9.eyJuYW1lIjp7Imxhc3R1cGRhdGVkIjoiMjAyMC0wNC0xNiIsInNvdXJjZSI6IjEiLCJjbGFzc2lmaWNhdGlvbiI6IkMiLCJ2YWx1ZSI6IkxJTSBZT05HIFhJQU5HIn0sImVtYWlsIjp7Imxhc3R1cGRhdGVkIjoiMjAyMC0wNC0xNiIsInNvdXJjZSI6IjIiLCJjbGFzc2lmaWNhdGlvbiI6IkMiLCJ2YWx1ZSI6Im15aW5mb3Rlc3RpbmdAZ21haWwuY29tIn0sIm1vYmlsZW5vIjp7Imxhc3R1cGRhdGVkIjoiMjAyMC0wNC0xNiIsInNvdXJjZSI6IjIiLCJjbGFzc2lmaWNhdGlvbiI6IkMiLCJhcmVhY29kZSI6eyJ2YWx1ZSI6IjY1In0sInByZWZpeCI6eyJ2YWx1ZSI6IisifSwibmJyIjp7InZhbHVlIjoiOTczOTkyNDUifX19.VkYRwZsMi8_z3Aa_dIKXYCbXcNaUXIANVzKls8QSnay0fkTdfGel4_WccMYmNIImMnhpV-gVH532ykX5OevxpIH6PH-6AOyxDdC5-pFTzKhBhfF-fBf_Ziet1rqjb511JeCzk_SIDIqmNCUVi9CPUgr77CKus8qnnC5JPS1pxwhXlpjZRXVSxze593XX-M-KjTbzB21Hzg-wZd3DZUPjeuBert80n1_vKp9je5JadCxqHA5Hbu3SF2xARjnMBkaEutkao_dn41p43Te74O1JsfvHNzFIGJ2-H0KplY6JN0kb7H5mNjitlwhuUW5qE-IXO7B7-1LQgX5THiOXM0JvcQ

To Reproduce
Steps to reproduce the behavior:

  1. Use the following function to decrypt the JWE (adapted from _decryptJWE() in https://github.com/opengovsg/myinfo-gov-client/blob/v4.1.2/src/MyInfoGovClient.class.ts):

    // Put this function in lib/express/myinfo/controllers.js of MockPass
    async function _decryptJWE(jwe) {
        const jose = require('node-jose');
        const jsonwebtoken = require('jsonwebtoken');
    
        let clientPrivateKey = fs.readFileSync(
            path.resolve(__dirname, '../../../static/certs/key.pem'),
        );
        let myInfoPublicKey = fs.readFileSync(
            path.resolve(__dirname, '../../../static/certs/spcp.crt'),
        );
    
        let jwt;
        let decoded;
        try {
            const keystore = await jose.JWK.createKeyStore().add(
                clientPrivateKey,
                'pem',
            );
            const { payload } = await jose.JWE.createDecrypt(keystore).decrypt(jwe);
    
            // The JSON.parse here is important, as the payload is wrapped in quotes
            console.log('Payload', payload.toString());
            jwt = JSON.parse(payload.toString());
        } catch (err) {
            throw err;
        }
    
        try {
            decoded = jsonwebtoken.verify(jwt, myInfoPublicKey, {
                algorithms: ['RS256'],
            });
    
            console.log('Decoded', decoded);
        } catch (err) {
            throw err;
        }
    
        if (typeof decoded !== 'object') {
            throw new Error('WrongDataShapeError');
        }
    
        return decoded;
    }
    
  2. Use the above function to decrypt the JWE returned by encryptPersona() in v4.3.4 of MockPass, i.e. https://github.com/opengovsg/mockpass/blob/v4.3.4/lib/express/myinfo/controllers.js and an error will occur.

  3. Use the above function to decrypt the JWE returned by encryptPersona() in v4.0.7 of MockPass, i.e. https://github.com/opengovsg/mockpass/blob/v4.0.7/lib/express/myinfo/controllers.js and the original persona will be returned.

Expected behavior
While this is definitely breaking backwards compatibility, I'm not sure whether the payload is supposed to be wrapped in quotes in the 1st place.

@cflee
Copy link
Contributor

cflee commented Sep 15, 2024

Thanks for the detailed note.

In RFC 7519 Appendix A.2, the example of a nested JWT takes just the JWS Compact Serialization form (without enclosing in quotes) as the plaintext/message for the JWE. I think that in general, JWS-in-JWE does not need to wrap the JWS in quotes / turn it into a JSON string prior to running the JWE.

The Myinfo v3 spec (v3.2.6) is silent about this detail but the code sample does have an extra JSON parse step as myinfo-gov-client is doing, so that implies that it's required as a Myinfo-specific JWS-in-JWE implementation detail.

Looks like a bug, from a regression in https://github.com/opengovsg/mockpass/pull/563/files#diff-3014c5111aef85faf3eeab80a6ef706e213621b4fea8b2c9c087f2b7c5681b47L42

@zionsg
Copy link
Author

zionsg commented Sep 16, 2024

Thanks for the reference to the RFC and MyInfo specs.

It seems that the wrapping in quotes was a result of the JSON.stringify(signedPersona) inside encryptPersona of MockPass v4.0.7 which used node-jose, e.g. JSON.stringify('eyJhbGc') => '"eyJhbGc"'.

Tried removing the JSON.stringify() in MockPass v4.0.7 and it yielded the same error as in MockPass v4.3.4 which uses jose. For now, have overridden _decryptJWE() of MyInfoGovClient in my Demo App to cater for both scenarios.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants