opengovsg / mockpass

A mock SingPass/CorpPass/MyInfo server for dev purposes
https://blog.data.gov.sg/mockpass-a-mock-singpass-corppass-server-for-testing-in-development-a583193c898c
MIT License
86 stars 82 forks source link

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

Open zionsg opened 1 month ago

zionsg commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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.