Closed Bug 1272001 Opened 8 years ago Closed 8 years ago

Fix missing server key share detection

Categories

(NSS :: Libraries, defect)

3.18
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: ekr, Assigned: ekr)

Details

Attachments

(1 file)

tls13_HandleServerKeyShare fumbles the list emptiness test and then semi-coincidentally ends up reporting a malformed server key share rather than a missing key share. Attached patch fixes that.
Comment on attachment 8751303 [details] [diff] [review]
0001-Bug-1272001-Fix-missing-server-key-share-detection-f.patch

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

Otherwise, LGTM

::: external_tests/ssl_gtest/ssl_extension_unittest.cc
@@ +212,4 @@
>                             SSL_LIBRARY_VERSION_TLS_1_3) {}
>  };
>  
> +class TlsExtensionTest13Stream

You need to guard this with #ifdef NSS_ENABLE_TLS_1_3

::: lib/ssl/SSLerrs.h
@@ -455,4 @@
>  ER3(SSL_ERROR_RX_UNEXPECTED_ENCRYPTED_EXTENSIONS, (SSL_ERROR_BASE + 142),
>      "SSL received an unexpected Encrypted Extensions handshake message.")
>  
> -ER3(SSL_ERROR_MISSING_EXTENSION_ALERT, (SSL_ERROR_BASE + 143),

Rename not safe.
Attachment #8751303 - Flags: review?(martin.thomson)
Work out why the TOO_LONG thing is happening too, because it's just odd that the alert is handled this way.
I know why it's happening. It's https://dxr.mozilla.org/mozilla-central/source/security/nss/lib/ssl/tls13con.c#2665

Basically, it's just a bad choice of error value.


Landed as:
https://hg.mozilla.org/projects/nss/rev/7e53ffa95952
looks like this can be closed.
Assignee: nobody → ekr
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → 3.25
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: