Bug 516056

Thanks for the report! There is no doubt the message could be more verbose. Nothing intended. Fedora curl was only switched from OpenSSL to NSS two years ago. I am currently working on the NSS support of curl. Feel free to report another ideas how to improve it.

Additionally discovered by Tomas that CURLOPT_SSL_VERIFYHOST is ignored by the current curl/NSS code.

patch prepared for review:
http://curl.haxx.se/mail/lib-2009-08/0106.html

amended and applied upstream:
http://cool.haxx.se/cvs.cgi/curl/lib/nss.c.diff?r1=1.51&r2=1.52

Thank Daniel Stenberg for review!

I am going to build updates for Fedora along with the new upstream release 7.19.6.

curl-7.19.6-1.fc10 has been submitted as an update for Fedora 10.
http://admin.fedoraproject.org/updates/curl-7.19.6-1.fc10

curl-7.19.6-1.fc11 has been submitted as an update for Fedora 11.
http://admin.fedoraproject.org/updates/curl-7.19.6-1.fc11

curl-7.19.6-1.fc10 has been pushed to the Fedora 10 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update curl'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F10/FEDORA-2009-8517

curl-7.19.6-1.fc11 has been pushed to the Fedora 11 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update curl'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F11/FEDORA-2009-8629

Error message from updated curl, noted for posterity:

curl: (51) common name 'CN=mail.google.com,O=Google Inc,L=Mountain View,ST=California,C=US' does not match 'blah'

Slightly different from openssl version (showing whole subject, not only CN, bit different wording).  Not sure if error message consistency between various SSL libraries support code is a big deal (there seem to be some differences between openssl and gnutls wrapping too).  Without that concern, this message should be good enough, imo.

Created attachment 358551 [details]
follow-up related to comment #9

Good point. Please look at the attached patch. I am going to apply it upstream. Unfortunately curl.haxx.se seems to be down right now.

(In reply to comment #10)
> Good point. Please look at the attached patch. I am going to apply it upstream.
> Unfortunately curl.haxx.se seems to be down right now.

The patch seems to do whole subject -> CN change for error message.  So we don't care about error message consistency?  These are messages currently printed:

ssluse:

  if(data->set.ssl.verifyhost > 1) {
    failf(data, "SSL: certificate subject name '%s' does not match "
          "target host name '%s'", peer_CN, conn->host.dispname);
    res = CURLE_PEER_FAILED_VERIFICATION;
  }
  else
    infof(data, "\t common name: %s (does not match '%s')\n",
          peer_CN, conn->host.dispname);


gtls:

  if(data->set.ssl.verifyhost > 1) {
    failf(data, "SSL: certificate subject name (%s) does not match "
          "target host name '%s'", certbuf, conn->host.dispname);
    gnutls_x509_crt_deinit(x509_cert);
    return CURLE_PEER_FAILED_VERIFICATION;
  }
  else
    infof(data, "\t common name: %s (does not match '%s')\n",
          certbuf, conn->host.dispname);


(the two above are almost identical ;)


nss:

  if(conn->data->set.ssl.verifyhost) {
    failf(conn->data, "common name '%s' does not match '%s'",
          subject_cn, conn->host.dispname);
    success = SECFailure;
  } else {
    infof(conn->data, "warning: common name '%s' does not match '%s'\n",
          subject_cn, conn->host.dispname);
  }


Yeah, just nitpicks... ;)

(In reply to comment #11)
Sorry, I've overlooked the note about error message wording. If I understand, you want to get the same error message as with the OpenSSL curl, right?

I am only not sure with the info message starting with "\t". NSS variant displays server's certificate only after successful handshake (in contrast to OpenSSL). Do you want to change this behavior as well? Or will be only the change of the error message sufficient for you?

(In reply to comment #12)
> (In reply to comment #11)
> Sorry, I've overlooked the note about error message wording. If I understand,
> you want to get the same error message as with the OpenSSL curl, right?

Not sure if I *want*, I just noticed the difference.  And as you are changing that code already, wouldn't it make sense to have the same error message for the same error condition, regardless of the crypto library in use?

Of course, I have no problem with "wontfix" as an answer if the differences are desired or hard to avoid (e.g. because of briefly mentioned \t difference issue).  From user pov, using the same (as similar as practical) messages for same problems seems like the "right thing".  That's all.

Created attachment 359042 [details]
follow-up related to comments #9 and #11

It makes sense to me. Are you fine with the attached patch?

(In reply to comment #14)
> Created an attachment (id=359042) [details]
> follow-up related to comments #9 and #11
> 
> It makes sense to me. Are you fine with the attached patch?  

Looks good from a quick look.

You can always ask upstream how much they care about wording consistency when submitting this patch...

Thank you!

Speaking for upstream:

We don't particularly need or aim for wording consistency in the error messages. But I'm also not against making them more similar as I'm sure it'll benefit users in the end.

The current standpoint is mostly due to pragmatic reasons (== nobody has tried to unify the SSL lib error messages).

We do however make an effort to make the APIs and ABIs the same, unregarding of which actual lib is used, so the actual return codes are considered very important to remain the same all over.

curl-7.19.6-1.fc10 has been pushed to the Fedora 10 stable repository.  If problems still persist, please make note of it in this bug report.

curl-7.19.6-1.fc11 has been pushed to the Fedora 11 stable repository.  If problems still persist, please make note of it in this bug report.