Closed Bug 1591315 Opened 2 years ago Closed 2 years ago

Should NSC_DecryptFinal return value be treated as secret?

Categories

(NSS :: Libraries, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: deian, Assigned: deian)

Details

(Keywords: csectype-other, sec-audit)

Attachments

(1 file)

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.

Assignee: nobody → deian
Flags: needinfo?(kjacobs.bugzilla)

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!

Flags: needinfo?(kjacobs.bugzilla)

Update NSC_Decrypt length in constant time

Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → 3.48
Group: crypto-core-security → core-security-release
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.