language correction and speed-up

[myheroyuki]

Addresses #205. I also moved an assert check up since the blinding will usually add length to the signed message which caused the appending zeroes test to fail with a different exception. If this doesn't seem right let me know!

[JeremyTubongbanua]

This actually helps! +1

[sybrenstuvel]

I also moved ...

Please do only one thing per PR. Combining changes muddles the discussion and makes things harder to accept (because everything has to be good in order to accept the PR).

In this case, moving the size check forward will make the timing of the 'happy flow' and 'unhappy flow' more distinct. Wouldn't this make it easier for an attacker to figure out the code path taken, as it'll be possible to distinguish between the different failure modes?

[myheroyuki]

Please only do one thing per PR.

Sorry, I considered them related because only making the correction/speed-up causes a test to fail. I did not see a way to decouple the two problems.

Wouldn't this make it easier for an attacker to figure out the code path taken

You're absolutely right about that.
However, the attack shouldn't be able do anything with that information that they couldn't already do by themselves;
if this gave the attacker an advantage, they could just run a vulnerable implementation of verify on their own machine.
The concern about figuring out code paths is more relevant when a private key is involved, for example during decryption or signing.

To be sure I wasn't missing anything, I traced the commit history for the source and found "Fix BB'06 attack in verify()..." in the commit message.
(This attack should not be confused with an "adaptive chosen-ciphertext attack" which might be abbreviated "BB'98".)
I found the commit author's article on the attack (here) and their source about it (here).

The source claims the attack is only good against keys with exponent of 3. This is not good practice, but we can't control what users do and should definitely try to protect them even when they make mistakes. The source also says the attack depends on "a failure to check a certain condition while verifying the RSA signature." Specifically, "The error that Bleichenbacher exploits is if the implementation does not check that the hash+ASN.1 data is right-justified within the PKCS-1 padding." This concern is unrelated to my changes.

Nonetheless, the commit author says their article is about a "straightforward variant" of BB'06. I read through the article and determined the concerns there were unrelated to my changes.

Finally, I found the relevant commit (here).
It looks like the presence of exception raising at the end of the function was simply an incidental artifact of greater changes to the whole function.
It's hard to say more; it seems like python-rsa used to be on Bitbucket and the issue/PR was lost in the migration to GitHub.

[sybrenstuvel]

Thank you for your elaborate explanation, that makes it very simple to now hit the 'merge' button :)

[sybrenstuvel]

Hmm the automated build failed, could you take a look at that @myheroyuki ?
https://github.com/sybrenstuvel/python-rsa/actions/runs/4798410202

[sybrenstuvel]

Never mind, must have been a hickup @ Github. Triggering the actions to run again did complete them all succesfully.