690980 - selfserv -n option is broken with TLS SNI

Status: RESOLVED FIXED

(NSS :: Tools, defect, P3)

Description

[Julien Pierre]

When using selfserv -n <nickname> where nickname is NOT a DNSname, and without setting the -a option , if a client sends the SNI the server always fails and returns an "unrecognized name" alert in response to clients that send the SNI extension .

In practice, this translates to Firefox not being able to connect to selfserv.
This is particularly problematic when using a hardware token, where the nickname can never match a DNSname since it will be of the form "token:nickname".

IE eventually falls down to SSL 3 and does not send an SNI extension, and is then able to connect successfully.

The fix is simple enough - selfserv should not call SSL_SNISocketConfigHook if the -a option has not been set.

I would also suggest that the -a option be changed to be a pair of strings, such as{DNSname,nickname} rather than assuming that they are both the same. As currently implemented, it is impossible to use certs in hardware tokens with SNI and selfserv.

Comment 1

[Julien Pierre]

Attachment: Don't setup SNI unless some hostnames were specified on the command-line

Comment 2

[Julien Pierre]

Ed, I'm a previous NSS contributor, but no longer have my SSH key for NSS checkin priviledge, so the assignee should be somebody else.

Comment 3

[Ed Morley [:emorley]]

I can change if you'd rather, but the assignee is typically the patch author, since checkin-needed in the keywords field can be used to ask someone to commit. (The main thing I wanted to avoid was having an empty assignee, since it makes it more time consuming for contributors to find bugs that have not yet been started).

Comment 4

[Robert Relyea]

Comment on attachment 597660 [details] [diff] [review]
Don't setup SNI unless some hostnames were specified on the command-line

r+ 

Initially I say a local variable turned to a global and thought 'bleh', but I see it's really associated with another global array. I would not be upset if the name changed from virtServerNameIndex to virtServerNameCount between now and checkin:).

bob

Comment 5

[Kai Engert [:KaiE:]]

Checking in selfserv.c;
/cvsroot/mozilla/security/nss/cmd/selfserv/selfserv.c,v  <--  selfserv.c
new revision: 1.99; previous revision: 1.98
done