Closed
Bug 937976
Opened 11 years ago
Closed 11 years ago
libssl stores current time in gmt_unix_time field of ClientHello and ServerHello; should use random value
Categories
(NSS :: Libraries, defect)
NSS
Libraries
Tracking
(Not tracked)
RESOLVED
FIXED
3.15.4
People
(Reporter: briansmith, Assigned: briansmith)
References
()
Details
(Keywords: privacy)
Attachments
(2 files)
1.06 KB,
patch
|
agl
:
review+
|
Details | Diff | Splinter Review |
576 bytes,
patch
|
briansmith
:
review+
|
Details | Diff | Splinter Review |
See http://www.ietf.org/mail-archive/web/tls/current/msg09861.html and https://github.com/openssl/openssl/pull/23 and https://github.com/openssl/openssl/pull/29
Attachment #831265 -
Flags: review?(agl)
Updated•11 years ago
|
Attachment #831265 -
Flags: review?(agl) → review+
Assignee | ||
Comment 1•11 years ago
|
||
http://hg.mozilla.org/projects/nss/rev/99b8adde480e
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Comment 2•11 years ago
|
||
Brian, Adam: are you guys sure about not filling gmt_unix_time with the current time? I know some people are interested in using the gmt_unix_time in ServerHello as a time source or hint, e.g., to detect a false certificate expired error due to incorrect system clock on the client computer.
Attachment #8342565 -
Flags: review?(brian)
Comment 3•11 years ago
|
||
I think this is a good change for clients and should land in NSS. We do have some cases where the time from a google.com ServerHello is used, so I wouldn't make a similar change there immediately, but maybe we'll switch to using a Date header or something in the future.
Assignee | ||
Comment 4•11 years ago
|
||
Comment on attachment 8342565 [details] [diff] [review] Remove a stale comment in ssl3_GetNewRandom Review of attachment 8342565 [details] [diff] [review]: ----------------------------------------------------------------- I purposely left that comment in so that it was more clear that we're intentionally not doing anything special with the gmt_unix_time field. But, I don't have strong feelings either way about whether the comment stays or goes.
Attachment #8342565 -
Flags: review?(brian) → review+
Comment 5•11 years ago
|
||
Comment on attachment 831265 [details] [diff] [review] stop-using-time-for-gmt_unix_time-field.patch Review of attachment 831265 [details] [diff] [review]: ----------------------------------------------------------------- ::: lib/ssl/ssl3con.c @@ -888,2 @@ > /* first 4 bytes are reserverd for time */ > - rv = PK11_GenerateRandom(&random->rand[4], SSL3_RANDOM_LENGTH - 4); Brian: this comment only makes sense when the code uses the magic array index 4. If you want to keep the comment, you need to update it to explain why the first 4 bytes are not the gmt_unix_time as specified in the TLS protocol.
You need to log in
before you can comment on or make changes to this bug.
Description
•