Cannot import private keys that start with zero bytes

[rhelmer]

First seen on Travis, I can reproduce locally by running ptest -v in a loop:

build/ptest/encrypt_test.c:306: mu_check((rv = (PrivateKey_decrypt(pvtkey_imp, decrypt, &plainLen, sizeof(decrypt), output, outputLen))) == SECSuccess) failed, resuming test case
build/ptest/encrypt_test.c:313: mu_check(rv == SECSuccess) failed, resuming test ca

Tests done:
	10 test suite(s) passed, 1 failed, 0 skipped.
	73 test case(s) passed, 1 failed.
	1441815 check(s) passed, 2 failed.

I've reproduced it on master (a898b55) after ~100 tries.

[rhelmer]

One variable I want to isolate is the version of NSS, as we went from 3.39 to 3.43 in the meantime between CI runs.

[henrycg]

I can reproduce this on Linux too, so I suspect it might be a bug in how I wrote the test. I can take a look now...

[henrycg]

I just compiled with NSS 3.39 and I don't see the test failing, which makes me now think that it might be a recent NSS bug. Does that sound right to you? If it'd be helpful, I can look through NSS changelog to see what might have happened.

[henrycg]

Oops, after running it for ~1000 iterations, I do see the test fail, even with NSS 3.39. So maybe it's a bug on our side after all...

[henrycg]

It looks like the failure always happens when the first byte of the private key is 0x00.

[henrycg]

@rhelmer After some more debugging: It looks like PrivateKey_import misbehaves (without throwing an error) when it's fed a private key bytestring whose leading byte is 0x00. Later on, when you try to use the imported private key, the decryption fails.

The implementation of PrivateKey_import is pretty hacky, but at the time I wrote it, I didn't see a cleaner way of getting a private key bytestring into NSS. Is there an NSS expert you could ask to see if there is a cleaner way to implement this?

[rhelmer]

@rhelmer After some more debugging: It looks like PrivateKey_import misbehaves (without throwing an error) when it's fed a private key bytestring whose leading byte is 0x00. Later on, when you try to use the imported private key, the decryption fails.

Thanks for tracking that down!

The implementation of PrivateKey_import is pretty hacky, but at the time I wrote it, I didn't see a cleaner way of getting a private key bytestring into NSS. Is there an NSS expert you could ask to see if there is a cleaner way to implement this?

@franziskuskiefer Franziskus, do you have any thoughts?

[henrycg]

@rhelmer @franziskuskiefer Is there a chance that there is a bug in NSS's PK11_ImportDERPrivateKeyInfoAndReturnKey function?

In particular, I created a test that

• constructs a curve25519 keypair that has a private key that begins with a leading 0x00 byte,
• builds a PKCS#8 encoding of this key, and
• tries to import this key into NSS using PK11_ImportDERPrivateKeyInfoAndReturnKey.

The import operation succeeds, but when I later try to use this private key, valgrind reports memory errors, which I suspect are OOB reads:

==10673== Conditional jump or move depends on uninitialised value(s)
==10673==    at 0x73581D6: ??? (in /usr/lib/x86_64-linux-gnu/nss/libfreeblpriv3.so)
==10673==    by 0x732452D: ??? (in /usr/lib/x86_64-linux-gnu/nss/libfreeblpriv3.so)
==10673==    by 0x6A3393F: ??? (in /usr/lib/x86_64-linux-gnu/nss/libsoftokn3.so)
==10673==    by 0x4E913D9: PK11_PubDeriveWithKDF (in /usr/lib/x86_64-linux-gnu/libnss3.so)
==10673==    by 0x407802: derive_dh_secret (encrypt.c:116)
==10673==    by 0x407802: PrivateKey_decrypt (encrypt.c:555)
==10673==    by 0x405AEE: test_import_hex_zeros (encrypt_test.c:496)
==10673==    by 0x4020DF: mu_run_suites (in build/ptest/ptest)
==10673==    by 0x402893: main (mutest.c:75)

In addition, when I later try to use this private key to decrypt valid ciphertexts, I get decryption failures. I get the same behavior even when I strip off the leading zeros before writing the private key into the PKCS#8 blob.

[henrycg]

This should be fixed by using the latest version of NSS.