Check that integer representation of message is smaller than modulus

[KostasTsiounis]

In RSA cipher operations, the message is converted into an integer that needs to be smaller than the public key's modulus.

A check is added to fail early with the proper message, instead of failing during the actual operation with a not so helpful message.

An additional test to verify that behaviour is introduced as well.

Signed-off-by: Kostas Tsiounis kostas.tsiounis@ibm.com

[jasonkatonica]

I wonder if this could impact performance creating a new BigInteger on each doFinal.

[KostasTsiounis]

That is a possibility, but I'm not sure what we can do. I would think we need this check regardless. Do you think we might wanna skip checking here?

[jasonkatonica]

I guess i cant come up with a better idea here. Maybe we can just run the RSACipherBenchmark.java just in case its significant.

[jasonkatonica]

This would be just so we know what the impact is not that I can think of doing something better here as doing our own bit math here for comparison is unlikely to be faster then just doing the m.compareTo(n) just below this.

[JinhangZhang]

If I understand correctly, based on the following new test in the BaseTestRSA, when a 2048-bit rsa public key is generated, the size of this.msgBuffer will be 256. The this.msgBuffer.array() returns the full backing array (e.g., 256 bytes for a 2048-bit key), regardless of msgLength. If a user encrypts a short 5-byte message, it sits at the beginning of the buffer, followed by 251 trailing zeros (0x00).

From my understanding, for a 2048-bit RSA key, the modulus n has a length of 256 bytes. To guarantee that it is genuinely a 2048-bit number (rather than 2047 bits), the most significant bit of its first byte should be set to 1. That's being said, the first byte of any valid RSA modulus will inherently fall between 0x80 and 0xFF.

In the BigInteger.CompareTo method, it has an internal method called compareMagnitude, as you can see, it actually compares the integer bytes by bytes.

So, I was thinking a scenario, let assume, If a user inputs a valid short message whose first byte happens to be 0xFF (e.g., [0xFF, 0x00, 0x00, 0x00, 0x00]), maybe at this moment, the RSA public modulus starts with ([0x80, 0xFF, 0xFF, 0xFF, 0x80). When m.compareTo(n) is called, the underlying JDK implementation (Integer.compareUnsigned) compares the int[] mag arrays from left to right. It may evaluate m>n, and incorrectly throw the BadPaddingException for a perfectly valid short plaintext.

[JinhangZhang]

Maybe we can add a test to verify this?

[KostasTsiounis]

Yes, you are right. I thought creating a BigInteger would cover that but it only strips leading zeroes, not ones coming later on.

I added a test to verify it (it originally failed). The issue was fixed by moving the check further down after the input has been copied to the end of the backing array, making the zeroes leading, and eventually being removed during the BigInteger instantiation.