Last Comment Bug 201139 - SSLTap should display plain text for NULL cipher suites
: SSLTap should display plain text for NULL cipher suites
Status: RESOLVED FIXED
:
Product: NSS
Classification: Components
Component: Tools (show other bugs)
: 3.8
: All All
: P3 enhancement (vote)
: 3.12
Assigned To: Alexei Volkov
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2003-04-07 20:59 PDT by Nelson Bolyard (seldom reads bugmail)
Modified: 2006-09-20 15:39 PDT (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
View application data in hex and ASCII with NULL ciphers (2.89 KB, patch)
2004-05-24 11:47 PDT, Saul Edwards
julien.pierre: review-
Details | Diff | Splinter Review
Sample output with the first patch (12.34 KB, text/plain)
2004-05-24 11:49 PDT, Saul Edwards
no flags Details
new output when NULL cipher is used (18.59 KB, text/plain)
2006-09-19 17:15 PDT, Alexei Volkov
no flags Details
patch (10.62 KB, patch)
2006-09-20 10:43 PDT, Alexei Volkov
no flags Details | Diff | Splinter Review
comments and fix for ASCII string alignment (12.04 KB, patch)
2006-09-20 14:54 PDT, Alexei Volkov
nelson: review+
Details | Diff | Splinter Review

Description Nelson Bolyard (seldom reads bugmail) 2003-04-07 20:59:30 PDT
When SSLTAP receives an SSL/TLS "Finished" message from either end, it stops
attempting to decode/display the record contents after that and instead 
displays the message "(encrypted)".  

It should not do this for the "NULL" cipher suites, since they do not encrypt
the data.
Comment 1 Nelson Bolyard (seldom reads bugmail) 2003-05-14 19:23:55 PDT
taking bug
Comment 2 Saul Edwards 2004-05-24 11:47:14 PDT
Created attachment 149221 [details] [diff] [review]
View application data in hex and ASCII with NULL ciphers

Compiled and tested on Solaris.  Does not work for SSL v2.
Comment 3 Saul Edwards 2004-05-24 11:49:10 PDT
Created attachment 149222 [details]
Sample output with the first patch
Comment 4 Julien Pierre 2004-05-24 15:34:31 PDT
Comment on attachment 149221 [details] [diff] [review]
View application data in hex and ASCII with NULL ciphers

The patch looks good, but I think you should include sslproto.h and use the
macros for the cipher suites, rather than typing the values again, which is
error prone. I know that's not the existing usage elsewhere in ssltap, but I
think it makes more sense, and should at least be done for the new code, if not
also for the existing.

In order to get the names of all the cipher suites, you may need to #ifdef
NSS_ENABLE_ECC . You will also need to add NSS_ENABLE_ECC support to the
manifest.mn for ssltap.
Comment 5 Nelson Bolyard (seldom reads bugmail) 2006-02-27 11:04:05 PST
reassigning to Alexei.
Comment 6 Alexei Volkov 2006-09-19 17:15:48 PDT
Created attachment 239276 [details]
new output when NULL cipher is used
Comment 7 Alexei Volkov 2006-09-20 10:43:33 PDT
Created attachment 239367 [details] [diff] [review]
patch

Use sslproto.h macro definition for cipher suite instead on hex numbers.
Removes obsolite ECC cipher definitions.
Prints MAC separetely from content. Calculates MAC size based on remaining
number of bytes in "finished" record. Check attached output.
Comment 8 Nelson Bolyard (seldom reads bugmail) 2006-09-20 14:30:50 PDT
Comment on attachment 239367 [details] [diff] [review]
patch

I've asked Alexei to add some comments and also to fix print_hex to properly align the ASCII text output.
Comment 9 Alexei Volkov 2006-09-20 14:54:20 PDT
Created attachment 239424 [details] [diff] [review]
comments and fix for ASCII string alignment
Comment 10 Nelson Bolyard (seldom reads bugmail) 2006-09-20 15:24:18 PDT
Comment on attachment 239424 [details] [diff] [review]
comments and fix for ASCII string alignment

r=nelson, with one small suggested change to a comment.

>+      if (!isNULLmac(currentcipher) && !hMACsize) {
>+          /* To calculate the size of MAC, we will sum the size
>+           * of verify_data(sslh.length) and 4(type + length
>+           * of finished handshake message) and subtract the
>+           * result from the total number of bytes in handshake
>+           * message(alloclen) */

This should say "from the total number of bytes in the handshake record",
the crucial difference being that the MAC comes at the end of a record,
and is not part of the message.  (records contain messages and MACs).
Comment 11 Alexei Volkov 2006-09-20 15:39:02 PDT
Integrated into 3.12 with corrected comment.

/cvsroot/mozilla/security/nss/cmd/ssltap/ssltap.c,v  <--  ssltap.c
new revision: 1.10; previous revision: 1.9

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