ssl: add server cert's "sha256//" hash to verbose

[gnawhleinad]

Add a "pinnedpubkey" section to the "Server Certificate" verbose

Bug: #410
Reported-by: W. Mark Kubacki

[gnawhleinad]

I was hoping to get an initial review on this change, and don't expect a merge at it's current state.

With the proposed fix:

$ ./curl --verbose --head --pinnedpubkey "sha256//daHR2E1iBqPT1X9qK/UISUZlkqW6MGJ7eBI+HDNtARs=" https://community.scaleway.com/
*   Trying 212.47.225.64...
* Connected to community.scaleway.com (212.47.225.64) port 443 (#0)
* Cipher selection: ALL:!EXPORT:!EXPORT40:!EXPORT56:!aNULL:!LOW:!RC4:@STRENGTH
* TLSv1.0 (OUT), TLS handshake, Client hello (1):
* TLSv1.0 (IN), TLS handshake, Server hello (2):
* TLSv1.0 (IN), TLS handshake, Certificate (11):
* TLSv1.0 (IN), TLS handshake, Server key exchange (12):
* TLSv1.0 (IN), TLS handshake, Server finished (14):
* TLSv1.0 (OUT), TLS handshake, Client key exchange (16):
* TLSv1.0 (OUT), TLS change cipher, Client hello (1):
* TLSv1.0 (OUT), TLS handshake, Finished (20):
* TLSv1.0 (IN), TLS change cipher, Client hello (1):
* TLSv1.0 (IN), TLS handshake, Finished (20):
* SSL connection using TLSv1.0 / DHE-RSA-AES256-SHA
* Server certificate:
*    subject: OU=GT30628797; OU=See www.rapidssl.com/resources/cps (c)15; OU=Domain Control Validated - RapidSSL(R); CN=*.scaleway.com
*    start date: 2015-02-05 06:59:04 GMT
*    expire date: 2017-02-07 02:23:01 GMT
*    subjectAltName: community.scaleway.com matched
*    issuer: C=US; O=GeoTrust Inc.; CN=RapidSSL SHA256 CA - G3
*    SSL certificate verify ok.
*    pinnedpubkey: sha256//wZfuNCfeEh0w6+eUazYO4VORGcsxqSACDPdqvnWBN/A=
* SSL: public key does not match pinned public key!
* Closing connection 0
* TLSv1.0 (OUT), TLS alert, Client hello (1):
curl: (90) SSL: public key does not match pinned public key!

[bagder]

Looks clean and appropriate to me!

[gnawhleinad]

The CI failure doesn't seem related to this change:

1801: data FAILED:
--- log/check-expected  2015-09-12 22:34:23.000000000 +0000
+++ log/check-generated 2015-09-12 22:34:23.000000000 +0000
@@ -1 +1,2 @@
 HTTP/1.1 101 Switching![LF]
+[LF]

Is there anything else I should add to this change?

[jay]

FYI the reason for the sha256 hash mismatch in that output is because the public key you specified is from RapidSSL's generic certificate and the certificate curl received is a different certificate that's server-specific for scaleway.

Likely what happened is someone made the hash you are passing in by retrieving the server certificate without specifying the server name. For OpenSSL's tool you would have to pass -servername to request the certificate for a server name. I've now added that to the PUBLIC KEY EXTRACTION example.

The correct hash appears to be the one your curl received,
sha256//wZfuNCfeEh0w6+eUazYO4VORGcsxqSACDPdqvnWBN/A=

I like the idea of showing it when verbose. Why did you make a copy of sha256sumdigest? That looks unnecessary.

[gnawhleinad]

Why did you make a copy of sha256sumdigest? That looks unnecessary.

You're right! It was unnecessary and I removed it. Thanks for the feedback!

[jay]

Great though now I notice something else. You did not free the memory allocated by the base64 encoding function. I fixed that and a few nits re style changes, and eliminated storing the return value from the base64 encoding function, instead testing it directly in an if.

Branch is here:
https://github.com/bagder/curl/compare/master...jay:pr_430_review?expand=1

This could still be improved. Now that we are encoding the sha256 binary data to base64 we can just compare that with what is supplied by the user rather than what we are currently doing where we take each of the users base64 encoded sha256// and convert them to binary and compare that. There's no point in doing that if we have the base64 encoded version.

Also regardless of opinions on that I think we should fail if Curl_base64_encode of the server's public key fails.

@bagder I know you're assigned, not trying to step on your toes here.

[gnawhleinad]

Now that we are encoding the sha256 binary data to base64 we can just compare that

I noticed that as well but thought it would be better to do that as a separate change. Nevertheless, it seems worthwhile to pursue in this commit, so I've updated the PR. I also included your style changes.

[bagder]

@jay I assure you I feel nothing but gratitude when you step in and help out!

[mark-kubacki]

Thanks, @jay!

[jay]

Daniel Hwang did most of the work so thanks to him :) One more minor thing, I've changed the designator we're using to show the server's pubkey hash from pinnedpubkey to public key hash because that hash is not necessarily one of the pinned hashes.