Conscrypt RSA NativeCrypto_EVP_PKEY_new_RSA does not appear to clear BoringSSL's error queue when needed

[vcsjones]

When importing an RSA key pair that goes through NativeCrypto_EVP_PKEY_new_RSA, and it fails - it appears to leave the error queue on BoringSSL's side unhandled, which can manifest as a nonsensical error in other places where an error occurs.

For example, let's say you try importing an RSA public key that has a modulus that is too large - 32K. BoringSSL rejects this, but the error stays in the queue.

To demonstrate, consider this use of ChaCha20Poly1305 which we expect to fail to decrypt because of a bad authentication tag. But before that, we try importing a giant RSA modulus. Instead of throwing a AEADBadTagException as we expect ChaCha20+Poly1305 to, it throws an InvalidKeyException with MODULUS_TOO_LARGE:

@Test
public void oversizedRsaFailureDoesNotAffectChaChaAeadBadTag() throws Exception {
    Provider conscryptProvider = TestUtils.getConscryptProvider();

    rejectOversizedRsaKey(conscryptProvider);

    Cipher decrypt = Cipher.getInstance("ChaCha20/Poly1305/NoPadding", conscryptProvider);
    Key key = new SecretKeySpec(new byte[32], "ChaCha20");
    decrypt.init(Cipher.DECRYPT_MODE, key, new IvParameterSpec(new byte[12]));
    decrypt.updateAAD(new byte[] { 1 });

    try {
        decrypt.doFinal(new byte[16]);
        fail("Bad AEAD tag should be rejected - we shouldn't get here");
    } catch (AEADBadTagException expected) {
        // Expected.
    }
}

private static void rejectOversizedRsaKey(Provider provider) throws Exception {
    byte[] modulus = new byte[4096]; // Giant modulus, it doesn't matter if it is valid or not for our purposes here
    modulus[0] = (byte) 0x80;
    modulus[modulus.length - 1] = 1;

    KeyFactory keyFactory = KeyFactory.getInstance("RSA", provider);
    RSAPublicKeySpec keySpec = new RSAPublicKeySpec(
            new BigInteger(1, modulus), BigInteger.valueOf(65537));

    try {
        keyFactory.generatePublic(keySpec);
        fail("Oversized RSA modulus should be rejected - we shouldn't get here");
    } catch (InvalidKeySpecException expected) {
        // Expected.
    }
}

I think this line:

conscrypt/common/src/jni/main/cpp/conscrypt/native_crypto.cc

Line 917 in 5203ae3

conscrypt::jniutil::throwRuntimeException(env, "Creating RSA key failed");

should use throwExceptionFromBoringSSLError, not throwRuntimeException. This "fixes" the problem, but I am very new to this project so it's entirely likely I may not be understanding intentions here.