Closed Bug 525056 Opened 12 years ago Closed 12 years ago

Timing attack against ssl3ext.c:ssl3_ServerHandleSessionTicketXtn()

Categories

(NSS :: Libraries, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
3.12.5

People

(Reporter: dveditz, Assigned: agl)

Details

Attachments

(1 file)

security@mozilla has received the following report from Trevor Perrin:

Hi,

I think there may be a potential minor security issue in the Netscape
Security Services (NSS) library.

The issue is in the ssl3ext.c:ssl3_ServerHandleSessionTicketXtn() function.

This function uses PORT_Memcmp() to check the session ticket's HMAC,
which is #define'd to memcmp() in secport.h.

On some systems memcmp() will compare the bytes of its inputs
sequentially, returning as soon as a difference is detected.  Thus, on
these systems, memcmp() leaks timing information regarding how many
bytes match at the start of its inputs.

(For example, on an x86 system there may be a several clock cycle
difference between a memcmp() that checks the first X bytes and one that
checks the first X+1 bytes).

Consider an attacker who has received a valid session ticket and is
trying to modify the ciphertext and construct a valid MAC for the
modified ticket.

This attacker could try the following:
 - Vary the first byte of the ticket's MAC from 0...255, for each value
performing a ClientHello with this modified ticket and measuring the
response time.  When a value is discovered which takes longer to process
than other values, this value is the first byte of the correct MAC.
 - Holding the above first byte constant, vary the second byte and
repeat the process.
 - Repeat for the third byte, and so on, until the entire correct MAC is
discovered.

Anyways, the fix is to use a constant-time verify function which always
iterates over the entire length of the input arrays, accumulating
differences by OR'ing the XOR of the values to compare, i.e.:

  /* nonEqual will be set to nonzero if the inputs are not equal */
  nonEqual = 0;
  for (i = 0; i != length; i++)
  {
    nonEqual |= (a[i] ^ b[i]);
  }


I'd appreciate you letting me know if you can confirm this issue.
Looks like someone saw bug 522580 and the patch that was committed for it,
found one more memcmp call, and filed a bug on it.
Indeed, although he's correct that I missed one. I can create a patch if that's the procedure.
(In reply to comment #2)
> Indeed, although he's correct that I missed one. I can create a patch if that's
> the procedure.

Most definitely feel free to attach a patch and request review from an NSS owner/peer.
I hadn't seen 522580, the NSS_SecureMemcmp looks good though.

I only reported the memcmp() in ticket-verification because that's the only case I'm aware of where someone could repeatedly submit queries against a single MAC.
Attached patch patchSplinter Review
Attachment #408964 - Flags: review?
Comment on attachment 408964 [details] [diff] [review]
patch

tagging wtc, but MisterSSL is welcome to it.
Attachment #408964 - Flags: review? → review?(wtc)
Assignee: nobody → agl
This bug doesn't need to be security-sensitive.
Group: core-security
Status: NEW → ASSIGNED
Comment on attachment 408964 [details] [diff] [review]
patch

r=wtc.  I checked in the patch on the NSS trunk (NSS 3.12.5).

Checking in ssl3ext.c;
/cvsroot/mozilla/security/nss/lib/ssl/ssl3ext.c,v  <--  ssl3ext.c
new revision: 1.4; previous revision: 1.3
done
Attachment #408964 - Flags: review?(wtc) → review+
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → 3.12.5
You need to log in before you can comment on or make changes to this bug.