Adding RFC2818 compliance to axTLS, CyaSSL and other SSL-backend related fixes, e.g. making PolarSSL usable again. #48

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
2 participants
Contributor

okoeroo commented Nov 8, 2012

axTLS:
This will make the axTLS backend perform the RFC2818 checks, honoring the VERIFYHOST setting similar to the OpenSSL backend.

Generic for OpenSSL and axTLS:
Also move the hostcheck and cert_hostcheck functions from the lib/ssluse.c files to make them genericly available for both the OpenSSL, axTLS and other SSL backends in the near future. Currently these are now in the lib/rawstr.c but will be moved later in a separate file.

CyaSSL:
CyaSSL has the RFC2818 checks also enabled now by default. There is a limitation that the verifyhost can not be enabled exclusively on the Subject CN field comparison. This SSL backend will thus behave like the NSS and the GnuTLS (meaning: RFC2818 ok, or bust). In other words: setting verifyhost to 0 or 1 will disable the Subject Alt Names checks too.

Schannel:
Updated the schannel information messages: Split the IP address usage message from the verifyhost setting and changed the message about disabling SNI (Server Name Indication, used in HTTP virtual hosting) into a message stating that the Subject Alternative Names checks are being disabled when verifyhost is set to 0 or 1. As a side effect of switching off the RFC2818 related servername checks with SCH_CRED_NO_SERVERNAME_CHECK (http://msdn.microsoft.com/en-us/library/aa923430.aspx) the SNI feature is being disabled. This effect is not documented in MSDN, but Wireshark output clearly shows the effect (details on the libcurl maillist).

PolarSSL:
Fix the prototype change in PolarSSL of ssl_set_session() and the move of the peer_cert from the ssl_context to the ssl_session. Found this change in the PolarSSL SVN between r1316 and r1317 where the POLARSSL_VERSION_NUMBER was at 0x01010100. But to accommodate the Ubuntu PolarSSL version 1.1.4 the check is to discriminate between lower then PolarSSL version 1.2.0 and 1.2.0 and higher. Note: The PolarSSL SVN trunk jumped from version 1.1.1 to 1.2.0.

curl tool:
Sets a CURLOPT_SSL_VERIFYHOST value of 0L.

Generic:
All the SSL backends are fixed and checked to work with the ssl.verifyhost as a boolean, which is an internal API change.

- Adding RFC2818 compliance checks to axTLS. Completely new are the S…
…ubjectAltName checks. The peer CN field fallback is implemented matching the OpenSSL behaviour. VERIFYHOST 0 == 1.

- Moved the hostcheck and cert_hostcheck static functions from lib/ssluse.c to rawstr.c to make them usable by the axTLS and other SSL backends. Also prefixed the functions with Curl_
- Adding RFC2818 checks for CyaSSL. This implementation will work similar to GnuTLS and NSS, meaning that you can't distinghuish between a SubjectAltName failure and CN failure
- Split the IP address usage message from the verifyhost setting and changing the wrong message about disabling SNI (Server Name Indication, used in HTTP virtual hosting) where SAN (Subject Alternative Name) is meant according to http://msdn.microsoft.com/en-us/library/aa923430.aspx about the flag SCH_CRED_NO_SERVERNAME_CHECK
- Moved the hostmatch and cert_hostcheck function out of the rawstr.c and into new files hostcheck.c and hostcheck.h. The OpenSSL and axTLS SSL backend code is now adjusted to use the hostcheck.h header for the prototypes. The Makefile.inc is updated with these two new files and everything builds and tests nicely.
- Remote unused code and added an explicit errSSLHostNameMismatch error switch-case that changes curl's generic failure on an SSL issue (CURLE_SSL_CONNECT_ERROR) to the explicit error code CURLE_PEER_FAILED_VERIFICATION which is used in all the other SSL backends for the same situation.
- Fix the prototype change of ssl_set_session and the move of the peer_cert from the ssl_context to the ssl_session. Found in the PolarSSL SVN between r1316 and r1317 where the POLARSSL_VERSION_NUMBER was at 0x01010100.
- PolarSSL works again with Ubuntu 12.10 and Debian 6 wheezy. PolarSSL is at version 1.1.4 on the Debians at this time. The code is prepared to build against PolarSSL 1.2.0 and olders (API) versions.
- For the PolarSSL version 1.2.0 and up now getting the peer certificate via the function ssl_get_peer_cert() with thanks the PolarSSL twitter account for feedback.
- Compensating in the axTLS and CyaSSL code for the internal change of the set.ssl.verifyhost from an int to a boolean.
- Added an explicit errSSLHostNameMismatch error switch-case that changes curl's generic failure on an SSL issue (CURLE_SSL_CONNECT_ERROR) to the explicit error code CURLE_PEER_FAILED_VERIFICATION which is used in all the other SSL backends for the same situation.
Contributor

okoeroo commented Nov 8, 2012

Various commits have been rebased on a known recent state and integrated into this one commit.

Owner

bagder commented Nov 8, 2012

Thanks! I've now pushed your patch. You'll find that I edited it a bit because...

Code style:

  • you used numerous too long lines
  • you didn't follow our if() for() etc code style
  • you added extra newlines and changed only whitespace on several places

Code:

  • hostcheck.c was built unconditional while only two backends need it
  • missed a static for a local function

git:

  • the common style of a commit message is a short first line two newlines and then the rest wrapped at column 72
  • When sending patches, you really should be prepared to edit and rebase your patch set after comments and feedback. That doesn't mean you have to squash them all into a single commit, as that isn't preferable either.

@bagder bagder closed this Nov 8, 2012

Contributor

okoeroo commented Nov 9, 2012

Thank you for this feedback. I'll use this in future patches.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment