Closed
Bug 1271878
Opened 9 years ago
Closed 9 years ago
ServerHello parsing for extensions is broken in tls_filter.cc
Categories
(NSS :: Test, defect)
NSS
Test
Tracking
(Not tracked)
RESOLVED
FIXED
3.25
People
(Reporter: mt, Assigned: mt)
References
Details
Attachments
(1 file)
6.42 KB,
patch
|
ekr
:
review+
|
Details | Diff | Splinter Review |
The format of the TLS 1.3 ServerHello has changed and so the parsing is wrong.
Assignee | ||
Comment 1•9 years ago
|
||
This is one of the "gratuitous changes" you asked me to split out from the DHE patch. I was depending on it though as it turns out.
Attachment #8751100 -
Flags: review?(ekr)
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → martin.thomson
Comment 2•9 years ago
|
||
As I didn't review this file yet, I wouldn't say that I asked you to split it out.
However, it seems like good practice in any case.
Assignee | ||
Comment 3•9 years ago
|
||
:) I did what I could to break out what I could. There were only a few things.
Comment 4•9 years ago
|
||
Comment on attachment 8751100 [details] [diff] [review]
bug1271878-1.patch
Review of attachment 8751100 [details] [diff] [review]:
-----------------------------------------------------------------
::: external_tests/ssl_gtest/libssl_internals.c
@@ +1,1 @@
> +/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
What happened here?
@@ +37,5 @@
>
> + if (authAlgorithm != ssl_auth_rsa_decrypt &&
> + authAlgorithm != ssl_auth_rsa_sign) {
> + return 0;
> + }
And here.
::: external_tests/ssl_gtest/test_io.cc
@@ +26,5 @@
> << __FUNCTION__ << std::endl; \
> PR_ASSERT(PR_FALSE); \
> PR_SetError(PR_NOT_IMPLEMENTED_ERROR, 0)
>
> +#define LOG(a) std::cerr << name_ << ": " << a << std::endl
What happened here
Attachment #8751100 -
Flags: review?(ekr)
Comment 5•9 years ago
|
||
Comment on attachment 8751100 [details] [diff] [review]
bug1271878-1.patch
Review of attachment 8751100 [details] [diff] [review]:
-----------------------------------------------------------------
r+ except for the changes to libssl_internals, which belong in a different patch.
Attachment #8751100 -
Flags: review+
Assignee | ||
Comment 6•9 years ago
|
||
Comment on attachment 8751100 [details] [diff] [review]
bug1271878-1.patch
Review of attachment 8751100 [details] [diff] [review]:
-----------------------------------------------------------------
::: external_tests/ssl_gtest/libssl_internals.c
@@ +1,1 @@
> +/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
This file has 4 character indents, but a modeline that has 2. That's annoying.
@@ +37,5 @@
>
> + if (authAlgorithm != ssl_auth_rsa_decrypt &&
> + authAlgorithm != ssl_auth_rsa_sign) {
> + return 0;
> + }
Too often if you have a test fail, this causes the test to crash.
Comment 7•9 years ago
|
||
Landed without the libssl_internals.c changes:
https://hg.mozilla.org/projects/nss/rev/aa86e41ad475
Assignee | ||
Updated•9 years ago
|
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → 3.25
You need to log in
before you can comment on or make changes to this bug.
Description
•