electron/node

crypto.createPublicKey Error: ERR_OSSL_BN_NEGATIVE_NUMBER

bcallaghan-fri opened this issue · 13 comments

I am attempting to use the package openid-client within an Electron app to authenticate the current user. When this library attempts to download the key store from Azure Active Directory, the following error is thrown:

{ 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\...\node_modules\@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 run (C:\Users\...\test-aad-keyset\index.js:5:19)
  opensslErrorStack:
   [ 'error:04000064:RSA routines:OPENSSL_internal:BAD_ENCODING' ],
  library: 'bignum routines',
  function: 'OPENSSL_internal',
  reason: 'NEGATIVE_NUMBER',
  code: 'ERR_OSSL_BN_NEGATIVE_NUMBER' }

The maintainer of openid-client suggested that this is a bug in Electron's implementation of the crypto package. With his help, I have published a minimal test project that produces repeatable results. This project works under Node 12.0.0, but fails under Electron 6.0.0-beta.13. When successful, this program will print the JWK store to the console.

This error still exists in Electron 6.0.0-beta.15

@bcallaghan-fri it works perfectly outside of electron?

@nornagon anything up with the BoringSSL patches?

panva commented

@bcallaghan-fri it works perfectly outside of electron?

Yes, on all platforms.

Here's the public key that openid-client is passing to crypto.createPublicKey:

-----BEGIN RSA PUBLIC KEY-----
MIIBCQKCAQChFFAb6JvfTHT15a0OkvTBZqED6GCNzSVahBTiXAEWNdXY988iaIC/
DE7E0bfgyCfDcr2G4Fguyu85ABsx0GYTyXDPwMUoxaoeqgffFKoMaEhgWhtAOJVG
SR5ylt6sfmGolq5IPs+LAMFJD9EIMiNvRI6ZJOllH3taziTXP3XnxVF/gFRsaPNW
3i1QDP/LZwjkmxdB9yAA8He+g/x66fEbBn+Izx6/DeFQ7Dv6e6I/K2zsdKS9XoYz
Ctd87KZj/DTuZjHD/Yucoid+xYG1xEgMb42uyPbfsuC0GErtOTO5WuC6r//5uQPD
AXepNFcYBf0qvBjiG5HENGyUwiDfQeqDAgMBAAE=
-----END RSA PUBLIC KEY-----

This parses (openssl asn1parse) as

    0:d=0  hl=4 l= 265 cons: SEQUENCE
    4:d=1  hl=4 l= 256 prim: INTEGER           :-5EEBAFE4176420B38B0A1A52F16D0B3E995EFC179F7232DAA57BEB1DA3FEE9CA2A270830DD977F40F3B13B2E481F37D83C8D42791FA7D13510C6FFE4CE2F99EC368F303F3AD73A55E155F820EB55F397B79FA5E4BFC76AB9B6E18D692153819E576951B7C13074FF3EB6F02EF7CDDC90BB7166DB169AE084A531DB28C08A183AAE807FAB93970CA921D2AFF3003498F71B64E8BE08DFFF0F88417C0385160EE4F9807730E140F21EAF13C405845DC0D493138B5B42A179CCF5288313599C03CB1199CE3C0274635DD8813A7E4A3BB7F39072513709204D1F4BE7B512C6CC46A51F4550000646FC3CFE8856CBA8E7FA02D543E71DE46E3BCB936B3DDF20BE157D
  264:d=1  hl=2 l=   3 prim: INTEGER           :010001

Seems like bssl doesn't expect negative numbers in this situation. See this line, committed here. OpenSSL says it ignores the sign, rather than throwing an error if the number is negative. Further, it states:

all BIGNUMs used are non negative and anything that looks negative is normally due to an encoding error

I'm not quite sure whether this is a configuration error when producing the jwk key, or when encoding it as PEM, but I think BoringSSL might be in the right here.

@ryzokuken It works on pure Node when using 12.0.0 and 12.4.0, but does not work on Electron 6. I get the same results when running on Windows and macOS. I haven't tested other versions.

@bcallaghan-fri that helps narrow it down to bssl, but @nornagon already pointed out what the issue seems to be. I agree that this seems to be more like something wrong with the key itself vs boringssl

@nornagon The JWK key is coming from Azure AD, so I doubt that it is being produced incorrectly. The issue is probably with the PEM encoding.

@panva After some research, I found that the ASN.1 format only supports signed integers, while JWKs contain unsigned integers. The asn1.js library used by @panva/jose will pad the integer with a zero byte to ensure that it is treated as a positive value. However, it does not perform this padding if the input is already a Buffer, as is the case with JWK values from @panva/jose. Thus, some values are incorrectly treated as negative numbers. Which library should I move this issue to?

panva commented

I’ll dig into it.

panva commented

Ok, i can fix the way JWKs get encoded as PEM/DER in @panva/jose pretty easy, but there's still one thing remaining for you guys.

The last key in the discovery has an x5c member which i can import in node like so

const { createPublicKey } = require('crypto')

const x5c = 'MIIC8TCCAdmgAwIBAgIQfEWlTVc1uINEc9RBi6qHMjANBgkqhkiG9w0BAQsFADAjMSEwHwYDVQQDExhsb2dpbi5taWNyb3NvZnRvbmxpbmUudXMwHhcNMTgxMDE0MDAwMDAwWhcNMjAxMDE0MDAwMDAwWjAjMSEwHwYDVQQDExhsb2dpbi5taWNyb3NvZnRvbmxpbmUudXMwggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIBAQDEdJxkw+jwWJ+gNyuCdxZDuYYm2IqGuyGjT64U9eD452dEpi51MPv4GrUwONypZF/ch8NWMUmLFBLrUzvCb3AAsOP76Uu4kn2MNQBZMDfFa9AtuIEz6CpTSyPiZzaVabqc9+qXJh5a1BxILSxiQuVrI2BfiegoNpeK+FU6ntvAZervsxHN4vj72qtFgqO7Md9kvuQ0EKyo7Xzk/Q0jYm2bD4SypiysJoex81EZGtO9QdSreFZrzn2Qr/m413tN5jZkTApUPx7MKZJ9Hn1nPLFO24+mQJIdL061S9LeapNiK3vepy+muOXdHyGmNctvyh+1+laveEVF2nGvC6hAQ7hBAgMBAAGjITAfMB0GA1UdDgQWBBQ5TKadw06O0cvXrQbXW0Nb3M3h/DANBgkqhkiG9w0BAQsFAAOCAQEAI48JaFtwOFcYS/3pfS5+7cINrafXAKTL+/+he4q+RMx4TCu/L1dl9zS5W1BeJNO2GUznfI+b5KndrxdlB6qJIDf6TRHh6EqfA18oJP5NOiKhU4pgkF2UMUw4kjxaZ5fQrSoD9omjfHAFNjradnHA7GOAoF4iotvXDWDBWx9K4XNZHWvD11Td66zTg5IaEQDIZ+f8WS6nn/98nAVMDtR9zW7Te5h9kGJGfe6WiHVaGRPpBvqC4iypGHjbRwANwofZvmp5wP08hY1CsnKY5tfP+E2k/iAQgKKa6QoxXToYvP7rsSkglak8N5g/+FJGnq4wP6cOzgZpjdPMwaVt5432GA==';

createPublicKey({
  key: `-----BEGIN CERTIFICATE-----\n${x5c}\n-----END CERTIFICATE-----`,
  format: 'pem'
})

but it fails in electron with no usable error message

Error: Failed to read asymmetric key
    at createPublicKey (internal/crypto/keys.js:321:10)

Note the other two keys work so maybe azure is to blame here, but then again, it works fine in node.

@panva Yes, probably another inconsistency between BoringSSL and OpenSSL. I will try to look into it if @nornagon cannot.

panva commented

@ryzokuken i've summarized my findings after running the full jose test suite in electron@beta, see https://github.com/panva/jose#electron-support to find out whats not supported.

The changes made by @panva solved the issue I was having. Thank you all for your help.

Thanks @panva and @bcallaghan-fri, glad you got it sorted out!