742162 - Allow CertificateRequest to have an empty certificate_authorities list

Status: RESOLVED FIXED

(NSS :: Libraries, defect, P1)

Description

[Wan-Teh Chang]

Attachment: Patch by Eric Rescorla

The attached patch makes ssl3_SendCertificateRequest allow nnames to
be 0.  This results in a CertificateRequest message with an empty
certificate_authorities list.  This is explicitly permitted by RFC 5246.

In DTLS-SRTP, the certificates are self-signed, so there is no sensible
value to put in the certificate_authorities list.

EKR said he would be more than happy to have an extra option required to
disable this check if it makes people nervous.  Bob, Nelson, do you
think this check is valuable for a regular SSL server?

Note to myself: if we remove this check, we should annotate sslerr.h
to note that SSL_ERROR_NO_TRUSTED_SSL_CLIENT_CA is not reported in
NSS 3.13.5 or later.

Comment 1

[Wan-Teh Chang]

I forgot to express my own opinion.  I think it is fine to simply
remove this check rather than adding an option to disable this check.
But I'd appreciate Bob and Nelson's opinions.

I found that RFC 4346 (TLS 1.1) also explicitly allows the
certificate_authorities list to be empty.

Comment 2

[Eric Rescorla (:ekr)]

To follow-up: it's important to me to be able to do this. I don't care what the mechanism is or how many options the user has to set.

Comment 3

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

FWIW, I agree with Wan-Teh that an option is not needed.

Comment 4

[Wan-Teh Chang]

Attachment: Patch by Eric Rescorla, v2

Brian: please review and check this in.  Feel free to edit
the comments.  Thanks.

Comment 5

[Robert Relyea]

Comment on attachment 612762 [details] [diff] [review]
Patch by Eric Rescorla, v2

r+ rrelyea

With some tentative thoughts.

My main worry is SSL servers. I believe the check was added because users would enable Client Auth on their SSL servers without adding any trusted client auth CAs. Then clients would fail to connect to the servers.

Most NSS servers are pretty aggressive on failing on startup if there is a problem.

So that being said, why did I r+ this? Because I don't think this failure would trigger the startup fail. At this point we are already too late.

What this means is that we won't get the message in the server log the the connection failed because there are no trusted client auth certs (the failure would simply be that the client was not trusted). I think this is probably OK, certainly simpler than trying to detect if we are running dtls and that it's a case where an empty calist is allowed (or reasonable).

bob

Comment 6

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

Comment on attachment 612762 [details] [diff] [review]
Patch by Eric Rescorla, v2

Review of attachment 612762 [details] [diff] [review]:
-----------------------------------------------------------------

r+. I suggest removing the comment in ssl3con.c, so that the change is purely a removal of code in ssl3con.c + the comment in sslerr.h.

Comment 7

[Wan-Teh Chang]

Attachment: Patch by Eric Rescorla, v3

Patch checked in on the NSS trunk (NSS 3.14) with
the change suggested by bsmith.

Checking in ssl3con.c;
/cvsroot/mozilla/security/nss/lib/ssl/ssl3con.c,v  <--  ssl3con.c
new revision: 1.180; previous revision: 1.179
done
Checking in sslerr.h;
/cvsroot/mozilla/security/nss/lib/ssl/sslerr.h,v  <--  sslerr.h
new revision: 1.24; previous revision: 1.23
done