540535 - crash in libssl when cache is disabled and attempting sending an alert to a client that has provided revoked cert

Status: RESOLVED FIXED

(NSS :: Libraries, defect, P2)

Description

[Alexei Volkov]

Attachment: fix the problem only in SSL3_SendAlert.

The problem is reproducible using selfserv with -N(disable cache) argument.

An attempt to establish a connection that requires client auth will result in server crash provided that a client responded with invalid certificate.

selfserv -D -p 10443 -d ../server -n <host cert> -B -s           -w nss   -i ../tests_pid.42202 -r -r -N

strsclnt -q -p 10443 -d ../client -B -s -w nss -c 1000 -C c -T <host>

Stack:
(gdb) where
#0  0x00000000 in ?? ()
#1  0x0005f41a in SSL3_SendAlert (ss=0x831800, level=alert_fatal, desc=certificate_revoked) at ssl3con.c:2528
#2  0x0006a979 in ssl3_HandleCertificate (ss=0x831800, b=0x7da26f "\020", length=0) at ssl3con.c:7896
#3  0x0006be02 in ssl3_HandleHandshakeMessage (ss=0x831800, b=0x7da004 "", length=619) at ssl3con.c:8424
#4  0x0006c2b2 in ssl3_HandleHandshake (ss=0x831800, origBuf=0x831a4c) at ssl3con.c:8548
#5  0x0006ce62 in ssl3_HandleRecord (ss=0x831800, cText=0xb0100000, databuf=0x831a4c) at ssl3con.c:8863
#6  0x0006df98 in ssl3_GatherCompleteHandshake (ss=0x831800, flags=0) at ssl3gthr.c:206
#7  0x00070c0a in ssl_GatherRecord1stHandshake (ss=0x831800) at sslcon.c:1258
#8  0x0007bdd9 in ssl_Do1stHandshake (ss=0x831800) at sslsecur.c:151
#9  0x0007dcc6 in ssl_SecureRecv (ss=0x831800, buf=0xb010045c "GET /abc HTTP/1.0\r\n\r\n", len=10239, flags=0) at sslsecur.c:1141
#10 0x0007dda4 in ssl_SecureRead (ss=0x831800, buf=0xb010045c "GET /abc HTTP/1.0\r\n\r\n", len=10239) at sslsecur.c:1160
#11 0x000858b3 in ssl_Read (fd=0x429ba0, buf=0xb010045c, len=10239) at sslsock.c:1620
#12 0x003a2691 in PR_Read (fd=0x429ba0, buf=0xb010045c, amount=10239) at ../../../../pr/src/io/priometh.c:141
#13 0x00003336 in handle_connection (tcp_sock=0x429ba0, model_sock=0x402fb0, requestCert=2) at selfserv.c:1131
#14 0x000025f4 in jobLoop (a=0x0, b=0x0, c=2) at selfserv.c:609
#15 0x000024ae in thread_wrapper (arg=0x42828c) at selfserv.c:578
#16 0x003c75a7 in _pt_root (arg=0x428510) at ../../../../pr/src/pthreads/ptthread.c:228
#17 0x9228b155 in _pthread_start ()
#18 0x9228b012 in thread_start ()

The problem is that the code assumes that pointers of cache and uncache functions are always initialized.

Comment 1

[Wayne Mery (:wsmwk)]

 Alexei, are you intending to have this reviewed?
see https://developer.mozilla.org/en/Code_Review_FAQ
and http://www.mozilla.org/about/owners.html

Comment 2

[Wan-Teh Chang]

NSS user 'passfree' reported in the mozilla-dev-tech-crypto
newsgroup that ssl3_HandleAlert may crash in a similar way
when an SSL server receives a fatal alert from a client due
to a server certificate error:
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/security/nss/lib/ssl/ssl3con.c&rev=1.149&mark=2663-2664#2663

We should add a similar null pointer check for ss->sec.uncache.

A possible alternative fix is to initialize the global
variables ssl_sid_cache to ServerSessionIDCache,
ssl_sid_uncache to ServerSessionIDUncache, etc.
But I guess we can't use ServerSessionIDCache and
ServerSessionIDUncache until the server session ID
cache has been initialized.  So perhaps the null pointer
checks are the right fix.

Comment 3

[Nelson Bolyard (seldom reads bugmail)]

passfree's problem is due to running an SSL server socket in a process where 
the SSL server cache is uninitialized.  LOTS of bad things will happen in 
that case. The uncache value being initialized with NULL is just the tip of 
the iceberg.  

There already is code to initialize the global variable ssl_sid_cache. 
That code gets called when the sid cache is initialized.  But in Firefox, 
it is never initialized.  

The moral of this story is: Don't use SSL server sockets without initializing
NSS for use in a server.  

BTW, Mozilla plans to remove (ifdef out) SSL server socket support completely from their builds of libSSL, so use of server-side SSL sockets in Mozilla is 
inadvisable, IMO.

Comment 4

[passfree]

What are the consequences of using dummy cache and uncache functions within NSS? my tests show so far no problems what so ever, but of course I do not fully understand NSS.

Ripping off a valid function from firefox for no apparent reason is a bad move, imho. I agree that the there are limited use cases for SSL server sockets at the moment. However, I believe that my module can quickly change that. In particular, it will make projects such as POW, a lot simpler to develop. SSL server and client sockets can also be used as part of other protocols different than HTTP and to enable a plethora of other xulrunner applications.

Comment 5

[Wan-Teh Chang]

I found the previous work on this problem: bug 237724.
We should treat this bug as a continuation of that work.

In bug 237724, we added a null pointer check for ss->sec.cache
in ssl3_HandleFinished, but more important, we added code to
fail with the SSL_ERROR_SERVER_CACHE_NOT_CONFIGURED error code
if the server session ID cache is not configured.

This bug shows that the detection of unconfigured server
session ID cache in ssl3_HandleClientHello is incomplete.
It is only done under some conditions:

http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/security/nss/lib/ssl/ssl3con.c&rev=1.149&mark=6122-6124,6130,6134#6117

We should change ssl3_HandleClientHello to always check
if the server session ID cache is configured.

Then, we can consider either removing the null pointer check
for ss->sec.cache (from ssl3_HandleFinished), or adding null
pointer checks for ss->sec.uncache as Alexei and I proposed.

Comment 6

[Wan-Teh Chang]

Attachment: Patch for ssl3_HandleAlert, by Eric Rescorla

This patch by EKR fixes the problem described in comment 2, originally
reported by passfree.

Comment 7

[Wan-Teh Chang]

Attachment: Patch for SSL3_SendAlert and ssl3_HandleAlert

Don't call ss->sec.uncache() if the SSL_NO_CACHE option is enabled.

I decided to test !ss->opt.noCache instead of ss->sec.uncache,
in the hope that this will catch servers that forget to configure
the SSL session ID cache.  They will still crash unless they enable
the SSL_NO_CACHE option to indicate they really don't want the SSL
session ID cache.

Patch checked in on the NSS trunk (NSS 3.13.5).

Checking in ssl3con.c;
/cvsroot/mozilla/security/nss/lib/ssl/ssl3con.c,v  <--  ssl3con.c
new revision: 1.177; previous revision: 1.176
done