panva / openid-client

OAuth 2 / OpenID Connect Client API for JavaScript Runtimes
MIT License
1.83k stars 392 forks source link

Error: ERR_OSSL_BN_NEGATIVE_NUMBER in Auth Code callback #176

Closed bcallaghan-fri closed 5 years ago

bcallaghan-fri commented 5 years ago

Describe the bug

When working through the Authentication Code flow, the following error occurs during the callback step. I'm attempting to get a token from Azure Active Directory v2.0. This result occurs with both the common and organizations tenants. The error appears to occur while getting the signing key for the returned ID Token.

Error: error:0300006d:bignum routines:OPENSSL_internal:NEGATIVE_NUMBER
    at createPublicKey (internal/crypto/keys.js:315:10)
    at asKey (C:\Users\...\node_modules\@panva\jose\lib\jwk\import.js:73:19)
    at C:\Users\...\@panva\jose\lib\jwks\keystore.js:162:39
    at Array.map (<anonymous>)
    at Object.asKeyStore (C:\Users\...\node_modules\@panva\jose\lib\jwks\keystore.js:162:26)
    at Issuer.keystore (C:\Users\...\node_modules\openid-client\lib\issuer.js:87:38)
    at processTicksAndRejections (internal/process/task_queues.js:86:5)
    at async Issuer.key (C:\Users\...\node_modules\openid-client\lib\issuer.js:119:22)
    at async Client.validateIdToken (C:\Users\...\node_modules\openid-client\lib\client.js:831:13)
    at async Client.callback (C:\Users\...\node_modules\openid-client\lib\client.js:472:7)

To Reproduce Issuer and Client configuration: (inline or gist) - Don't forget to redact your secrets.

const issuer = await Issuer.discover('https://login.microsoftonline.com/organizations/v2.0');
this.client = new issuer.Client({
    client_id: '<my-client-id>',
    redirect_uris: ['https://login.microsoftonline.com/common/oauth2/nativeclient']
    token_endpoint_auth_method: 'none'
});

Expected behaviour The ID Token and Access Token are returned, or a validation error is thrown.

Environment:

panva commented 5 years ago

@bcallaghan-fri This is strange, i thought windows might be an issue (since it works locally for me) so I've triggered a CI build including a windows one doing the same code path that doesn't work for you - resolving the AAD issuer and then fetching the keystore.

As you can see the regular windows build passed so the only thing left to check is electron's bundled node version - I suggest you open the issue over at https://github.com/electron/node

I can only guarantee node runtime compatibility, electron bundles BoringSSL instead of OpenSSL which is what node's crypto is working on. If BoringSSL is not API compatible with OpenSSL then there are operations bound to fail.

Although BoringSSL is an open source project, it is not intended for general use, as OpenSSL is. We don’t recommend that third parties depend upon it. Doing so is likely to be frustrating because there are no guarantees of API or ABI stability.

bcallaghan-fri commented 5 years ago

Thank you for your fast response. I was able to replicate the issue in a minimal project based on the test case you linked. The following code works with Node 12.0.0, but does not work with Electron 6.0.0-beta.13.

const { Issuer } = require('openid-client');

async function run() {
    const aad = await Issuer.discover('https://login.microsoftonline.com/organizations/v2.0');
    const keystore = await aad.keystore();
    console.log(keystore);
}

run()
.catch(console.error)
.then(() => process.exit());

I'll file a new bug report with Electron.

bcallaghan-fri commented 5 years ago

A new bug report has been filed with Electron Node.

panva commented 5 years ago

@bcallaghan-fri i'll be releasing @panva/jose 1.5.1 shortly with a fix to the JWK import (you should run npm upgrade --depth 99999 to upgrade your deps), but please note that electron still fails to import the last jwks_uri x5c as a certificate. Even if that got fixed, i just ran the jose test suite under electron and there are plenty of failures. Mostly probably coming from missing cipher support in boringssl compared to openssl.

So bottom line, while the lingering bug in the jose library is going to be fixed, no good news for you as electron still doesn't behave as node when it comes to importing keys and i cannot guarantee it as a supported runtime. I'll keep digging tho

panva commented 5 years ago

I've pinned https://github.com/panva/jose/issues/34 in @panva/jose summarizing what doesn't work due to BoringSSL use.

panva commented 5 years ago

@bcallaghan-fri you should be unblocked now with @panva/jose 1.5.2 (do npm upgrade --depth 99999), i've just tried to split the cert by 64 and join with new lines and it worked. All other limitations to the jose capabilities under electron from the issue above still apply.

bcallaghan-fri commented 5 years ago

@panva Those two patches worked for me. I can now do the full Auth Code flow when running in Electron 6. Thanks for your help!