I enabled SSLpinning testing for mbedTLS but half of the pinning tests
(2035, 2038, 2042) fail because the code doesn't return
CURLE_SSL_PINNEDPUBKEYNOTMATCH on fail. The reason for this appears to
be because the comparison is done in an mbedTLS callback and we can't
return that there. A workaround would be we modify the callback's user
pointer to use a struct that includes a CURLcode result where we record
the result of whatever we're doing in the callback.

Here are the SSLpinning test numbers:
runtests.pl 2034 2035 2037 2038 2041 2042

Also I notice if peer verification is disabled the SSLpinning result
isn't checked. This was not caught by the tests so I'll add a test for that.

I propose a different solution for both of those issues. Rather than
call Curl_pin_peer_pubkey during certificate verification from a verify
callback we call it after the certificate verification (in
mbedtls_connect_step2). I think that would be an easier way to deal with
it. See code at [1].

Two things I'm still unclear on:

- There is a comment in the code that says a peer cert is not available
after an SSL session resume, specifically "If the session was resumed,
there will be no peer cert". It appears that was copied from PolarSSL. I
looked in the mbedTLS documentation to confirm but there's nothing
documented in mbedtls_ssl_get_peer_cert. However mbedtls_ssl_get_session
[2] has a notice that says "Currently, peer certificate is lost in the
operation." Yet I tested it and the peer certificate seems to be
available on resume:

./curl -v --pinnedpubkey
sha256//C4G4mPCYzTEVZBFSwJ5u+IxQYaKOxhQwBz7YeD/ELxk= https://google.com
https://google.com

If the peer certificate is truly not available on resume, or not
supposed to be, we should have a way to identify that separately from
the case where no peer cert is provided (since in both cases the return
is NULL).

- mbedtls_pk_write_pubkey_der expects a mbedtls_pk_context that is
non-const [3], but mbedtls_ssl_get_peer_cert returns a const
certificate. I don't see a way to make a non-const copy of the public
key but there is probably a way to do this, or copy the cert or some
parent container.

Unfortunately I don't think I'll have much more time to look into these
issues, I have some other curl things I need to close out before the
release. Let me know what way you want to go and I'll help where I can.

[1]:
https://github.com/bagder/curl/compare/master...jay:fix_mbedtls_pinned_key?expand=1
[2]: https://tls.mbed.org/api/ssl_8h.html#ad617a178adfaf259db89c4fe092ad6e1
[3]: https://tls.mbed.org/api/pk_8h.html#af5b7c3953ad6fe8be09d35508ee569f6

-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-library
Etiquette: http://curl.haxx.se/mail/etiquette.html
Received on 2016-01-11