Closed
Bug 951199
Opened 12 years ago
Closed 11 years ago
Interface for Socket Level Access to TLS Version Used
Categories
(Core :: Security: PSM, defect)
Tracking
()
RESOLVED
FIXED
mozilla29
People
(Reporter: mcmanus, Assigned: mcmanus)
References
Details
Attachments
(1 file)
4.32 KB,
patch
|
keeler
:
review+
briansmith
:
review+
|
Details | Diff | Splinter Review |
This patch allows necko to determine the TLS version used for a particular connection. In particular, HTTP/2 carries a requirement for connections >= TLS 1.1 and this lets sensible application level error checking occur.
Assignee | ||
Comment 1•12 years ago
|
||
Attachment #8348796 -
Flags: review?(dkeeler)
Assignee | ||
Updated•12 years ago
|
Attachment #8348796 -
Flags: review?(brian)
Assignee | ||
Comment 2•12 years ago
|
||
the linux platforms are all reporting (on try):
8:08:19 INFO - TEST-INFO | skipping /builds/slave/test/build/tests/xpcshell/tests/netwerk/test/unit/test_spdy.js | run-if: hasNode
18:08:19 INFO - TEST-INFO | skipping /builds/slave/test/build/tests/xpcshell/tests/netwerk/test/unit/test_http2.js | run-if: hasNode
![]() |
||
Comment 4•12 years ago
|
||
Comment on attachment 8348796 [details] [diff] [review]
0003-idl-for-getting-the-ssl-handshake-version.patch
Review of attachment 8348796 [details] [diff] [review]:
-----------------------------------------------------------------
LGTM - just a few questions.
::: netwerk/socket/nsISSLSocketControl.idl
@@ +70,5 @@
> */
> readonly attribute uint32_t providerFlags;
> +
> + /* These values are defined by TLS. */
> + const short SSL_VERSION_2 = 0x0002;
SSLv2 is completely disabled, right? We probably don't need this.
::: security/manager/ssl/src/nsNSSCallbacks.cpp
@@ +1237,4 @@
> // 0=ssl3, 1=tls1, 2=tls1.1, 3=tls1.2
> unsigned int versionEnum = channelInfo.protocolVersion & 0xFF;
> Telemetry::Accumulate(Telemetry::SSL_HANDSHAKE_VERSION, versionEnum);
> + infoObject->SetSSLVersionUsed(channelInfo.protocolVersion);
Do we also need to do this in CanFalseStartCallback?
Attachment #8348796 -
Flags: review?(dkeeler) → review+
Comment 5•12 years ago
|
||
Comment on attachment 8348796 [details] [diff] [review]
0003-idl-for-getting-the-ssl-handshake-version.patch
Review of attachment 8348796 [details] [diff] [review]:
-----------------------------------------------------------------
::: netwerk/socket/nsISSLSocketControl.idl
@@ +70,5 @@
> */
> readonly attribute uint32_t providerFlags;
> +
> + /* These values are defined by TLS. */
> + const short SSL_VERSION_2 = 0x0002;
I agree.
::: security/manager/ssl/src/nsNSSCallbacks.cpp
@@ +1237,4 @@
> // 0=ssl3, 1=tls1, 2=tls1.1, 3=tls1.2
> unsigned int versionEnum = channelInfo.protocolVersion & 0xFF;
> Telemetry::Accumulate(Telemetry::SSL_HANDSHAKE_VERSION, versionEnum);
> + infoObject->SetSSLVersionUsed(channelInfo.protocolVersion);
This is redundant with the call in PreliminaryHandshakeDone.
Attachment #8348796 -
Flags: review?(brian) → review+
Assignee | ||
Comment 6•11 years ago
|
||
based on review comments I removed the V2 const and the redundant code.
Assignee | ||
Comment 7•11 years ago
|
||
Comment 8•11 years ago
|
||
Assignee: nobody → mcmanus
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
You need to log in
before you can comment on or make changes to this bug.
Description
•