Closed Bug 1016836 Opened 5 years ago Closed 5 years ago

RSA_PrivateKeyCheck not robust against 'hostile' inputs, crash

Categories

(NSS :: Libraries, defect, P1)

3.16.1
defect

Tracking

(Not tracked)

RESOLVED FIXED
3.16.2

People

(Reporter: ryan.sleevi, Assigned: ryan.sleevi)

Details

Attachments

(3 files)

The freebl function RSA_PrivateKeyCheck, used to check parameter consistency, is not robust against hostile input, in that it always calls SECITEM_TO_MPINT on the supplied parameters.

The related function, RSA_PopulatePrivateKey, is more consistent, by only calling SECITEM_TO_MPINT when (param).data is non-NULL, and ensuring the operation fails if the necessary parameters are not present.
Attached patch RobustnessSplinter Review
Bob,

This matches our discussion on the call, which was to change RSA_PrivateKeyCheck to require all of the parameters be supplied when importing a key to softoken, rather than recomputing the CRT factors.

For robustness sake, if any of the parameters are missing, it will early return - to avoid SECITEM_TO_MPINT blowing up.

The softoken side is in Bug 1016811.
Attachment #8432001 - Flags: review?(rrelyea)
Attachment #8432001 - Flags: review?(rrelyea) → review+
Fixed in https://hg.mozilla.org/projects/nss/rev/7dc83fd04549
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
All calls to swap_in_key_value were removed in the previous patch.
Attachment #8436365 - Flags: review?(ryan.sleevi)
Comment on attachment 8436365 [details] [diff] [review]
Remove the unused function swap_in_key_value.

Review of attachment 8436365 [details] [diff] [review]:
-----------------------------------------------------------------

r+
Attachment #8436365 - Flags: review?(ryan.sleevi) → review+
Comment on attachment 8436365 [details] [diff] [review]
Remove the unused function swap_in_key_value.

Patch checked in: https://hg.mozilla.org/projects/nss/rev/b066f111ac16
Attachment #8436365 - Flags: checked-in+
These comments were more useful when RSA_PrivateKeyCheck
would fix up the input RSAPrivateKey. Now that RSA_PrivateKeyCheck
only performs checks, it is not necessary to mention which
errors can be recovered from and which cannot.
Attachment #8440180 - Flags: review?(ryan.sleevi)
Attachment #8440180 - Flags: review?(ryan.sleevi) → review+
Comment on attachment 8440180 [details] [diff] [review]
Remove stale comments from RSA_PrivateKeyCheck

Patch checked in: https://hg.mozilla.org/projects/nss/rev/b07697c94038
Attachment #8440180 - Flags: checked-in+
You need to log in before you can comment on or make changes to this bug.