Last Comment Bug 430743 - Update ssltap to understand the TLS session ticket extension
: Update ssltap to understand the TLS session ticket extension
Status: RESOLVED FIXED
:
Product: NSS
Classification: Components
Component: Tools (show other bugs)
: unspecified
: All All
: -- enhancement (vote)
: 3.12.1
Assigned To: Wan-Teh Chang
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2008-04-24 18:03 PDT by Wan-Teh Chang
Modified: 2008-05-07 08:44 PDT (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Proposed stopgap patch (2.87 KB, patch)
2008-04-24 18:03 PDT, Wan-Teh Chang
nelson: review+
Details | Diff | Splinter Review
Alternative patch (2.87 KB, patch)
2008-04-24 18:56 PDT, Wan-Teh Chang
nelson: review+
Details | Diff | Splinter Review
Parse the NewSessionTicket handshake message (4.37 KB, patch)
2008-04-29 20:59 PDT, Wan-Teh Chang
no flags Details | Diff | Splinter Review
Parse the NewSessionTicket handshake message, v2 (4.37 KB, patch)
2008-05-06 17:53 PDT, Wan-Teh Chang
nelson: review+
Details | Diff | Splinter Review

Description Wan-Teh Chang 2008-04-24 18:03:37 PDT
Created attachment 317642 [details] [diff] [review]
Proposed stopgap patch

We should update ssltap to understand the TLS session ticket extension (RFC 5077, formerly RFC 4507).

This patch doesn't parse the NewSessionTicket message yet.
Comment 1 Wan-Teh Chang 2008-04-24 18:56:59 PDT
Created attachment 317650 [details] [diff] [review]
Alternative patch

In looking at RFC 5077 closely, I found that it named the new handshake
message type for NewSessionTicket inconsistently:

      struct {
          HandshakeType msg_type;
          uint24 length;
          select (HandshakeType) {
              case hello_request:       HelloRequest;
              case client_hello:        ClientHello;
              case server_hello:        ServerHello;
              case certificate:         Certificate;
              case server_key_exchange: ServerKeyExchange;
              case certificate_request: CertificateRequest;
              case server_hello_done:   ServerHelloDone;
              case certificate_verify:  CertificateVerify;
              case client_key_exchange: ClientKeyExchange;
              case finished:            Finished;
              case session_ticket:      NewSessionTicket; /* NEW */
          } body;
      } Handshake;

So in this alternative patch, I use the exact name "session_ticket" out of the
RFC rather than the more logical name "new_session_ticket".

Nelson, which do you prefer?
Comment 2 Nelson Bolyard (seldom reads bugmail) 2008-04-24 23:14:12 PDT
Comment on attachment 317642 [details] [diff] [review]
Proposed stopgap patch

Both patches are a good start.  I prefer the string new_session_ticket, 
because it's visually distinct from the label for the extension name, and calls attention to the difference between the extension and the message.  But I'm not insistent about it.
Comment 3 Nelson Bolyard (seldom reads bugmail) 2008-04-24 23:15:01 PDT
Comment on attachment 317650 [details] [diff] [review]
Alternative patch

I can live with either patch, although I prefer the other one.
Comment 4 Wan-Teh Chang 2008-04-25 09:59:29 PDT
Comment on attachment 317642 [details] [diff] [review]
Proposed stopgap patch

I checked in this patch on the NSS trunk (NSS 3.12.1).

Checking in ssltap.c;
/cvsroot/mozilla/security/nss/cmd/ssltap/ssltap.c,v  <--  ssltap.c
new revision: 1.11; previous revision: 1.10
done
Comment 5 Wan-Teh Chang 2008-04-29 20:59:30 PDT
Created attachment 318540 [details] [diff] [review]
Parse the NewSessionTicket handshake message

1. Add the GET_32 macro for reading uint32.
2. Use {...} (with three dots) for consistency.
3. Parse the NewSessionTicket handshake message.  I
   imitated the code for parsing ServerHello.

An example is show below:

SSLRecord { [Tue Apr 29 20:44:21 2008]
   0: 16 03 01 00  ca                                     | .....
   type    = 22 (handshake)
   version = { 3,1 }
   length  = 202 (0xca)
   handshake {
   0: 04 00 00 c6                                         | ....
      type = 4 (new_session_ticket)
      length = 198 (0x0000c6)
         NewSessionTicket {
            ticket_lifetime_hint = Wed, 30-Apr-08 03:44:21 GMT
            ticket = {
                length = 192
                contents = {...}
   0: c4 e0 92 5b  79 9b 43 90  11 f7 21 c9  b5 87 ce bd  | ...[y.C...!.....
  10: 16 78 57 79  d6 a9 33 a1  af 12 0a 32  8b 39 4c 8e  | .xWy..3....2.9L.
  20: 8a ed de 7f  63 b3 ac 32  82 b3 fb c5  d5 fd d7 81  | ...^?c..2........
  30: 12 f3 d9 25  c5 33 21 e3  8d af e4 8b  c4 1f 8c 85  | ...%.3!.........
  40: 12 ca 9d 8e  d0 75 de 5c  39 8a be 8a  a9 e4 f3 4a  | .....u.\9......J
  50: 19 78 61 82  cb ce e6 b3  f1 96 0a 99  37 da d5 02  | .xa.........7...
  60: 09 4e 6c 2f  31 e9 ac 59  3f 03 59 27  f0 60 6a 29  | .Nl/1..Y?.Y'.`j)
  70: e1 be 69 e8  15 90 fd 46  31 2b 3b b0  4d 1c fc 40  | ..i....F1+;.M..@
  80: 3e 94 1f b4  78 9f 96 fa  8f 15 2d b5  73 3c c9 0d  | >...x.....-.s<..
  90: 5e db ec 7f  8b c9 d9 4b  81 f8 cd 19  41 4b 47 fc  | ^..^?...K....AKG.
  a0: 57 60 85 27  36 d7 54 61  01 35 35 2a  dd da 93 eb  | W`.'6.Ta.55*....
  b0: a2 a9 e4 4e  a2 3a 9d 00  ab 8c 1e 99  b7 8c e2 0f  | ...N.:..........
            }
         }
   }
}
Comment 6 Wan-Teh Chang 2008-05-06 17:53:55 PDT
Created attachment 319697 [details] [diff] [review]
Parse the NewSessionTicket handshake message, v2

The only change is to use a four-digit year when printing
ticket_lifetime_hint.
Comment 7 Nelson Bolyard (seldom reads bugmail) 2008-05-06 18:47:28 PDT
Comment on attachment 319697 [details] [diff] [review]
Parse the NewSessionTicket handshake message, v2

r=nelson
Comment 8 Wan-Teh Chang 2008-05-07 08:44:44 PDT
I checked in the patch "Parse the NewSessionTicket handshake
message, v2" (attachment 319697 [details] [diff] [review]) on the NSS trunk (NSS 3.12.1).

Checking in ssltap.c;
/cvsroot/mozilla/security/nss/cmd/ssltap/ssltap.c,v  <--  ssltap.c
new revision: 1.12; previous revision: 1.11
done

Note You need to log in before you can comment on or make changes to this bug.