202348 - libssl should check cert & key pointers returned by client auth callback function

Status: RESOLVED FIXED

(NSS :: Libraries, defect, P2)

Description

[Julien Pierre]

A client application can use SSL_GetClientAuthDataHook to set a callback
function to be called when a server requests client authentication. The function
is expected to return a key, a cert, and a status.

If the callback function returns a NULL key and/or cert, but a SECSuccess for
status, libssl does not notice the pointers are NULL and continues the
handshake, eventually crashing in a NULL pointer dereference. There should be a
less severe way to handle this case. I suggest that if either the key or the
cert is NULL, we treat it as if the function had returned SECFailure, except we
free any key/cert actually returned (eg. if the callback sets the key but no
cert, or the cert but no key).

Unfortunately I don't think there is a good way to return an error to the
application. The only way would be to set an NSPR error and make the immediate
PR_Write or PR_Read call that triggered the handshake fail with that error (eg.
SSL_ERROR_BAD_CALLBACK_FUNCTION). That's a secondary issue though. The primary
goal is to prevent the crash from occurring.

Comment 1

[Julien Pierre]

Attachment: Check for NULL pointers returned by client auth application callback

Do this in both ssl2 & ssl3 .

Comment 2

[Julien Pierre]

I ran all.sh and it passed with attachment 120781 [details] [diff] [review] .
We would need an additional test for the crashing condition that this test fixes.

The test would just need to make an SSL connection, return a NULL cert and/or
key in the callback, and SECSuccess .

Comment 3

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 120781 [details] [diff] [review]
Check for NULL pointers returned by client auth application callback

r=nelsonb

Comment 4

[Julien Pierre]

Checked in to the tip :

cvs commit: Examining .
Checking in ssl3con.c;
/cvsroot/mozilla/security/nss/lib/ssl/ssl3con.c,v  <--  ssl3con.c
new revision: 1.54; previous revision: 1.53
done
Checking in sslcon.c;
/cvsroot/mozilla/security/nss/lib/ssl/sslcon.c,v  <--  sslcon.c
new revision: 1.21; previous revision: 1.20
done

and to NSS_3_8_BRANCH :

Checking in ssl3con.c;
/cvsroot/mozilla/security/nss/lib/ssl/ssl3con.c,v  <--  ssl3con.c
new revision: 1.53.2.1; previous revision: 1.53
done
Checking in sslcon.c;
/cvsroot/mozilla/security/nss/lib/ssl/sslcon.c,v  <--  sslcon.c
new revision: 1.20.2.1; previous revision: 1.20
done