1591315 - Should NSC_DecryptFinal return value be treated as secret?

Status: RESOLVED FIXED

(NSS :: Libraries, defect)

Description

[Deian Stefan]

In NSC_DecryptFinal we treat the return value of the padding check as secret. This was brought up here.

Craig's verification tool gave NSC_DecryptFinal a thumbs up, but flagged NSC_Decrypt. Specifically: https://searchfox.org/nss/source/lib/softoken/pkcs11c.c#1761-1762
where we branch on the return value of NSC_DeryptFinal.

We can be conservative and turn this into a constant-time branch too, but I'm wondering if this is too conservative? What this leaks really is whether or not the padding is invalid and not the length of the pad. I think that for most AEAD constructions that this is fine, but happy to be wrong.

Happy to patch if we want to be conservative.

Comment 1

[Kevin Jacobs [:kjacobs]]

The utility of this seems to be pretty low, but I'd prefer to be complete since this is generic functionality and we're already doing this for the NSC_Decrypt branch. I'm happy to review such a patch.

Thank you for following up with the tool results!

Comment 2

[Deian Stefan]

Attachment: Bug 1591315 - Update NSC_Decrypt length in constant time

Update NSC_Decrypt length in constant time

Comment 3

[J.C. Jones [:jcj] (he/they)]

https://hg.mozilla.org/projects/nss/rev/7f578a829b29a2d63383ff7cf36ba778235ab77e