On 24-10-12 22:45, Daniel Stenberg wrote:
> Hi friends,
>
> The Most Dangerous Code in the World: Validating SSL Certificates in
> Non-Browser Software" is a report from 6 authors I noticed today:
>
> http://www.cs.utexas.edu/~shmat/shmat_ccs12.pdf
>
> Among many things it has the following charming remark about libcurl's API:
> "This interface is almost perversely bad."
>
> From what I understand, the single reason behind that statement is that we
> have the CURLOPT_SSL_VERIFY HOST option which takes a three-value option and
> not just a boolean. The authors found several source codes that treated it
> as a boolean and set it to TRUE (== 1) and thus it doesn't check the
> certificate properly.
>
> So instead of posting a patch to us, instead of mailing us a suggestion,
> instead of posting a bug report they write this document.
>

Hi,

The "The Most Dangerous Code in the World" paper -
http://www.cs.utexas.edu/~shmat/shmat_ccs12.pdf - only speaks of the OpenSSL
backend of cUrl. As Daniel rightfully mentions in one of the posts here,
there's more then just OpenSSL.

I pulled a fresh cUrl and looked at the cUrl code enabling axTLS, CyaSSL,
GnuTLS, NSS, OpenSSL, PolarSSL and QsoSSL. I could only find the API docs
for QsoSSL, but I've downloaded and read the code in these SSL
implementation up to and including where it started to verify the
certificate chain as a TLS client and where/how either the SSL library or
cUrl is implementing the RFC2818 checks for a secure HTTP over TLS check
before we set an CURL_OK;.

My hunt started with looking for the VERIFYHOST value and if the value was
actually being used in the same way across all the SSL implementation cUrl
support, so I've also included details on the usage of VerifyHost setting in
my tiny report here. I also give some code hints. This should be worked in,
including a bug in how cUrl interacts with PolarSSL, this is a security risk
as it's hardwired in cUrl at the moment. I'm also betting a stack of
stroopwaffels down the line somewhere to spice up my day, which I'm willing
to ship internationally of course.

With respect to the ongoing discussion on a patch, I wonder if making the
value of 1 is the best solution. For the OpenSSL backend the value only
effects checking of host certificate that doesn't have any SubjectAltNames,
but do have a Subject DN field. If there isn't a CN field in the absence,
it's an error:

1289 else if(!peer_CN) {

In the OpenSSL backend the following seems true: The VERIFYHOST == 2 means:
Must match the CN field, in absence of SubjectAltNames. VERIFYHOST == 1
means, when the cert_hostcheck() fails (no match) cast an informational
message that the CN didn't match the connection string. VERIFYHOST = 0 means
what VERIFYHOST == 1 means. Code snippet:

1288 else if(!cert_hostcheck((const char *)peer_CN, conn->host.name)) {
1289 if(data->set.ssl.verifyhost > 1) {
1290 failf(data, "SSL: certificate subject name '%s' does not match "
1291 "target host name '%s'", peer_CN, conn->host.dispname);
1292 res = CURLE_PEER_FAILED_VERIFICATION;
1293 }
1294 else
1295 infof(data, "\t common name: %s (does not match '%s')\n",
1296 peer_CN, conn->host.dispname);
1297 }
1298 else {
1299 infof(data, "\t common name: %s (matched)\n", peer_CN);
1300 }

Side note: My explanation is a representation of the code, not the
documentation.

    Oscar

ps: I'm sorry if my Thunderbird messed up the formatting.

De vele SSL's in cUrl
---------------------

lib/axtls.c
VerifyHost is not used.
axTLS is not implementing hostname certificate verification, the calling
application/library needs to implement this. Curl is not doing this for the
axTLS backend.
Hint: use "const char * ssl_get_cert_subject_alt_dnsname(const SSL *ssl, int
dnsindex)"

lib/cyassl.c
VerifyHost is not used.
I didn't check for CyaSSL on it's hostname certificate verification
capabilities. In effect Curl is not implementinghostname verification.
Hint: use "char* CyaSSL_X509_get_next_altname(CYASSL_X509*);"

lib/gtls.c
VerifyHost is used
Is RFC2818 compliance is implemented in GnuTLS

 650 /* This function will check if the given certificate's subject
matches the
 651 given hostname. This is a basic implementation of the matching
described
 652 in RFC2818 (HTTPS), which takes into account wildcards, and the
subject
 653 alternative name PKIX extension. Returns non zero on success, and
zero on
 654 failure. */
 655 rc = gnutls_x509_crt_check_hostname(x509_cert, conn->host.name);
 656
 657 if(!rc) {
 658 if(data->set.ssl.verifyhost > 1) {
 659 failf(data, "SSL: certificate subject name (%s) does not match "
 660 "target host name '%s'", certbuf, conn->host.dispname);
 661 gnutls_x509_crt_deinit(x509_cert);
 662 return CURLE_PEER_FAILED_VERIFICATION;
 663 }
 664 else
 665 infof(data, "\t common name: %s (does not match '%s')\n",
 666 certbuf, conn->host.dispname);
 667 }
 668 else
 669 infof(data, "\t common name: %s (matched)\n", certbuf);

lib/nss.c
VerifyHost is used
Is RFC2818 compliance is implemented in Mozilla's libNSS in
CERT_VerifyCertName()

 610 static SECStatus BadCertHandler(void *arg, PRFileDesc *sock)
 [...]
 614 PRErrorCode err = PR_GetError();
 [...]
 625 switch(err) {
 [...]
 633 case SSL_ERROR_BAD_CERT_DOMAIN:
 634 if(conn->data->set.ssl.verifyhost) {
 635 failf(conn->data, "SSL: certificate subject name '%s' does not
match "
 636 "target host name '%s'", subject_cn, conn->host.dispname);
 637 }
 638 else {
 639 result = SECSuccess;
 640 infof(conn->data, "warning: SSL: certificate subject name '%s'
does not "
 641 "match target host name '%s'\n", subject_cn,
conn->host.dispname);
 642 }
 643 break;

1153 CURLcode Curl_nss_connect(struct connectdata *conn, int sockindex)
[...]
1302 if(!data->set.ssl.verifypeer && data->set.ssl.verifyhost)
1303 infof(data, "warning: ignoring value of ssl.verifyhost\n");
1304 else if(data->set.ssl.verifyhost == 1)
1305 infof(data, "warning: ignoring unsupported value (1) of
ssl.verifyhost\n");

lib/polarssl.c
VerifyHost is not used.
The PolarSSL authmode is set to SSL_VERIFY_OPTIONAL. This MUST be
SSL_VERIFY_REQUIRED unless you meant to state CURLOPT_SSL_VERIFYPEER = 0...

199 ssl_set_authmode(&conn->ssl[sockindex].ssl, SSL_VERIFY_OPTIONAL);

  ssl_set_authmode(&conn->ssl[sockindex].ssl, SSL_VERIFY_OPTIONAL);
Polar performs a liberal RFC2818 check. It doesn't look at the type of
SubjectAltNames it compares. So any value you put in the
ssl_set_ca_chain(..., conn->host.name) is being matched with any AtlName.
RFC2818 makes a difference between the dNSAltname and others, but I don't
think this is an issue at all. It probably aids to implement the 3rd
checking clause in RFC2818, quote: "If the client has external information
as to the expected identity of the server, the hostname check MAY be omitted.".

lib/qssl.c
VerifyHost is used
The setting is used to install a callback which returns the magical value of
1. Given the API INFO on IBM's publib I'm not to sure what this really does
internally. I completely boils down to the SSL_Handshake() function if it
internally performs the RFC2818 checks, besides the certificate chain
validation. Looking at the SSLHandle struct's contents there is no place to
actually set the connection string. So... unless you magically reverse the
IP and reverse lookup from the socket internally in the SSL library (pretty
bad and unlikely), I'll be betting a stack of stroopwaffels on the first to
prove me wrong this SSL library is actually capable of complying with RFC2818.

API Info:
http://publib.boulder.ibm.com/infocenter/iseries/v5r3/topic/apis/sslhands.htm

156 static int Curl_qsossl_trap_cert(SSLHandle * h)
157
158 {
159 return 1; /* Accept certificate. */
160 }
[...]
163 static CURLcode Curl_qsossl_handshake(struct connectdata * conn, int
sockindex)
[...]
174 if(!data->set.ssl.verifyhost)
175 h->exitPgm = Curl_qsossl_trap_cert;
[...]
211 rc = SSL_Handshake(h, SSL_HANDSHAKE_AS_CLIENT);

lib/ssluse.c
1123 static CURLcode verifyhost(struct connectdata *conn,
1124 X509 *server_cert)
1125 {
1126 int matched = -1; /* -1 is no alternative match yet, 1 means match and 0
1127 means mismatch */

[... Only reached after non-existance of SubjectAltNames in hostcert and a
last/most significant CN exists ...]

1280 if(res)
1281 /* error already detected, pass through */
1282 ;
1283 else if(!peer_CN) {
1284 failf(data,
1285 "SSL: unable to obtain common name from peer certificate");
1286 res = CURLE_PEER_FAILED_VERIFICATION;
1287 }
1288 else if(!cert_hostcheck((const char *)peer_CN, conn->host.name)) {
1289 if(data->set.ssl.verifyhost > 1) {
1290 failf(data, "SSL: certificate subject name '%s' does not match "
1291 "target host name '%s'", peer_CN, conn->host.dispname);
1292 res = CURLE_PEER_FAILED_VERIFICATION;
1293 }
1294 else
1295 infof(data, "\t common name: %s (does not match '%s')\n",
1296 peer_CN, conn->host.dispname);
1297 }
1298 else {
1299 infof(data, "\t common name: %s (matched)\n", peer_CN);
1300 }

[... note: this means VERIFYHOST == 0 is equal to VERIFYHOST == 1 ...]

-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-library
Etiquette: http://curl.haxx.se/mail/etiquette.html
Received on 2012-10-28