366803 - Improve SSL tracing, make it work in browsers

Status: RESOLVED FIXED

(NSS :: Libraries, enhancement, P2)

Description

[Nelson Bolyard (seldom reads bugmail)]

libSSL has a trace/debug feature that writes to stdout.  
mozilla browsers close stdout (at least on Windows), causing all that
trace output to be lost.  So, the first requested enhancement is to 
be able to name a file to which trace/debug output is written by libSSL.

The code for making SSL records out of application data and sending 
them out was largely rewritten for NSS 3.11, and consequently the 
tracing calls in that code are now inadequate to completely follow the 
activities in that processing.  The code needs to be changed to trace
All the calls and all the return values to that function, not just the
ones that sucesfully write all the data.  

Patch forthcoming.

Comment 1

[Nelson Bolyard (seldom reads bugmail)]

Attachment: patch v1

With this patch I was able to produce the log file I attached to bug 356470.

Comment 2

[Alexei Volkov]

Comment on attachment 251269 [details] [diff] [review]
patch v1


> #ifdef DEBUG
>+	ev = getenv("SSLDEBUGFILE");
>+	if (ev && ev[0]) {
>+	    ssl_trace_iob = fopen(ev, "w");
>+	    SSL_TRACE(("SSL: debugging sent to %s", ev));
>+	}

SSL_TRACE call is useless. User will only see it after he opens a debug file. I would make fopen and SSL_TRACE switch places.

>-    va_end(args);
>-    puts(buf);

By removing puts(bug) we limit ourself to
print only to an open ssl_trace_iob. I'd like still to be able to print trace into standard output.

     va_start(args, format);
     PR_vsnprintf(buf, sizeof(buf), format, args);
     va_end(args);

>+
>+    if (ssl_trace_iob) {
>+
>+	fputs(buf,  ssl_trace_iob);
>+	fputs("\n", ssl_trace_iob);
     } else {
        puts(bug);
     }
> }
> #endif

Comment 3

[Nelson Bolyard (seldom reads bugmail)]

Attachment: patch v2

This patch eliminates the superfluous print at the start of the file,
and changes the trace function to output to stderr if SSLDEBUGFILE is
not given in the environment or names a file that cannot be created.

Comment 4

[Neil Williams]

Comment on attachment 251333 [details] [diff] [review]
patch v2

looks good.

Comment 5

[Alexei Volkov]

Comment on attachment 251333 [details] [diff] [review]
patch v2

r=alexei.volkov

Comment 6

[Nelson Bolyard (seldom reads bugmail)]

Committed on trunk.
Checking in sslimpl.h;  new revision: 1.54; previous revision: 1.53
Checking in sslsecur.c; new revision: 1.37; previous revision: 1.36
Checking in sslsock.c;  new revision: 1.51; previous revision: 1.50
Checking in ssltrace.c; new revision: 1.4; previous revision: 1.3

Checkin comment:
Bug 366803 - Improve SSL tracing, make it work in browsers, to help with
debugging bug 356470.  r=neil.williams,alexei.volkov

Comment 7

[Nelson Bolyard (seldom reads bugmail)]

Committed on branch

Checking in sslimpl.h;  new revision: 1.42.2.9; previous revision: 1.42.2.8
Checking in sslsecur.c; new revision: 1.34.2.3; previous revision: 1.34.2.2
Checking in sslsock.c;  new revision: 1.44.2.7; previous revision: 1.44.2.6
Checking in ssltrace.c; new revision: 1.3.28.1; previous revision: 1.3