From 357ff4d1dc1209b67183d14f544757abe2c8d60e Mon Sep 17 00:00:00 2001 From: Patrick Monnerat Date: Mon, 13 Oct 2014 18:34:51 +0200 Subject: [PATCH] Factorize pinned public key code into generic file handling and backend specific --- lib/strerror.c | 2 +- lib/vtls/gtls.c | 80 +++++++++++++--------------------------------- lib/vtls/openssl.c | 68 +++++++-------------------------------- lib/vtls/vtls.c | 56 ++++++++++++++++++++++++++++++++ lib/vtls/vtls.h | 3 ++ 5 files changed, 95 insertions(+), 114 deletions(-) diff --git a/lib/strerror.c b/lib/strerror.c index 1a1360607..6e75579ff 100644 --- a/lib/strerror.c +++ b/lib/strerror.c @@ -299,7 +299,7 @@ curl_easy_strerror(CURLcode error) return "The max connection limit is reached"; case CURLE_SSL_PINNEDPUBKEYNOTMATCH: - return "SSL public key does not matched pinned public key"; + return "SSL public key does not match pinned public key"; /* error codes not used by current libcurl */ case CURLE_OBSOLETE20: diff --git a/lib/vtls/gtls.c b/lib/vtls/gtls.c index 8364f4458..6ffe29798 100644 --- a/lib/vtls/gtls.c +++ b/lib/vtls/gtls.c @@ -678,22 +678,22 @@ gtls_connect_step1(struct connectdata *conn, return CURLE_OK; } -static int pkp_pin_peer_pubkey(gnutls_x509_crt_t cert, char *pinnedpubkey) +static CURLcode pkp_pin_peer_pubkey(gnutls_x509_crt_t cert, + const char *pinnedpubkey) { /* Scratch */ - FILE* fp = NULL; size_t len1 = 0, len2 = 0; - unsigned char *buff1 = NULL, *buff2 = NULL; - long size = 0; + unsigned char *buff1 = NULL; gnutls_pubkey_t key = NULL; /* Result is returned to caller */ - int ret = 0, result = FALSE; + int ret = 0; + CURLcode result = CURLE_SSL_PINNEDPUBKEYNOTMATCH; /* if a path wasn't specified, don't pin */ - if(NULL == pinnedpubkey) return TRUE; - if(NULL == cert) return FALSE; + if(NULL == pinnedpubkey) return CURLE_OK; + if(NULL == cert) return result; do { /* Begin Gyrations to get the public key */ @@ -719,57 +719,14 @@ static int pkp_pin_peer_pubkey(gnutls_x509_crt_t cert, char *pinnedpubkey) /* End Gyrations */ - fp = fopen(pinnedpubkey, "r"); - - if(NULL == fp) - break; /* failed */ - - /* Seek to eof to determine the file's size */ - ret = fseek(fp, 0, SEEK_END); - if(0 != ret) - break; /* failed */ - - /* Fetch the file's size */ - size = ftell(fp); - - /* - * if the size of our certificate doesn't match the size of - * the file, they can't be the same, don't bother reading it - */ - if(size > 0 && len2 != (size_t)size) - break; /* failed */ - - /* Rewind to beginning to perform the read */ - ret = fseek(fp, 0, SEEK_SET); - if(0 != ret) - break; /* failed */ - - /* http://www.openssl.org/docs/crypto/buffer.html */ - buff2 = malloc(len2); - if(NULL == buff2) - break; /* failed */ - - /* Returns number of elements read, which should be 1 */ - ret = (int)fread(buff2, (size_t)len2, 1, fp); - if(1 != ret) - break; /* failed */ - /* The one good exit point */ - result = (0 == memcmp(buff1, buff2, (size_t)len2)); - + result = Curl_pin_peer_pubkey(pinnedpubkey, buff1, len1); } while(0); - if(NULL != fp) - fclose(fp); - if(NULL != key) gnutls_pubkey_deinit(key); - if(NULL != buff2) - free(buff2); - - if(NULL != buff1) - free(buff1); + Curl_safefree(buff1); return result; } @@ -876,10 +833,12 @@ gtls_connect_step3(struct connectdata *conn, issuerp = load_file(data->set.ssl.issuercert); gnutls_x509_crt_import(x509_issuer, &issuerp, GNUTLS_X509_FMT_PEM); rc = gnutls_x509_crt_check_issuer(x509_cert,x509_issuer); + gnutls_x509_crt_deinit(x509_issuer); unload_file(issuerp); if(rc <= 0) { failf(data, "server certificate issuer check failed (IssuerCert: %s)", data->set.ssl.issuercert?data->set.ssl.issuercert:"none"); + gnutls_x509_crt_deinit(x509_cert); return CURLE_SSL_ISSUER_ERROR; } infof(data,"\t server certificate issuer check OK (Issuer Cert: %s)\n", @@ -965,6 +924,7 @@ gtls_connect_step3(struct connectdata *conn, if(certclock == (time_t)-1) { if(data->set.ssl.verifypeer) { failf(data, "server cert expiration date verify failed"); + gnutls_x509_crt_deinit(x509_cert); return CURLE_SSL_CONNECT_ERROR; } else @@ -974,6 +934,7 @@ gtls_connect_step3(struct connectdata *conn, if(certclock < time(NULL)) { if(data->set.ssl.verifypeer) { failf(data, "server certificate expiration date has passed."); + gnutls_x509_crt_deinit(x509_cert); return CURLE_PEER_FAILED_VERIFICATION; } else @@ -988,6 +949,7 @@ gtls_connect_step3(struct connectdata *conn, if(certclock == (time_t)-1) { if(data->set.ssl.verifypeer) { failf(data, "server cert activation date verify failed"); + gnutls_x509_crt_deinit(x509_cert); return CURLE_SSL_CONNECT_ERROR; } else @@ -997,6 +959,7 @@ gtls_connect_step3(struct connectdata *conn, if(certclock > time(NULL)) { if(data->set.ssl.verifypeer) { failf(data, "server certificate not activated yet."); + gnutls_x509_crt_deinit(x509_cert); return CURLE_PEER_FAILED_VERIFICATION; } else @@ -1006,11 +969,14 @@ gtls_connect_step3(struct connectdata *conn, infof(data, "\t server certificate activation date OK\n"); } - if(data->set.str[STRING_SSL_PINNEDPUBLICKEY] != NULL && - TRUE != pkp_pin_peer_pubkey(x509_cert, - data->set.str[STRING_SSL_PINNEDPUBLICKEY])) { - failf(data, "SSL: public key does not matched pinned public key!"); - return CURLE_SSL_PINNEDPUBKEYNOTMATCH; + ptr = data->set.str[STRING_SSL_PINNEDPUBLICKEY]; + if(ptr) { + result = pkp_pin_peer_pubkey(x509_cert, ptr); + if(result != CURLE_OK) { + failf(data, "SSL: public key does not match pinned public key!"); + gnutls_x509_crt_deinit(x509_cert); + return result; + } } /* Show: diff --git a/lib/vtls/openssl.c b/lib/vtls/openssl.c index aacd2778f..6bf7275a9 100644 --- a/lib/vtls/openssl.c +++ b/lib/vtls/openssl.c @@ -2366,20 +2366,18 @@ static CURLcode get_cert_chain(struct connectdata *conn, * Heavily modified from: * https://www.owasp.org/index.php/Certificate_and_Public_Key_Pinning#OpenSSL */ -static int pkp_pin_peer_pubkey(X509* cert, char *pinnedpubkey) +static CURLcode pkp_pin_peer_pubkey(X509* cert, const char *pinnedpubkey) { /* Scratch */ - FILE* fp = NULL; int len1 = 0, len2 = 0; - unsigned char *buff1 = NULL, *buff2 = NULL, *temp = NULL; - long size = 0; + unsigned char *buff1 = NULL, *temp = NULL; /* Result is returned to caller */ - int ret = 0, result = FALSE; + CURLcode result = CURLE_SSL_PINNEDPUBKEYNOTMATCH; /* if a path wasn't specified, don't pin */ - if(NULL == pinnedpubkey) return TRUE; - if(NULL == cert) return FALSE; + if(NULL == pinnedpubkey) return CURLE_OK; + if(NULL == cert) return result; do { /* Begin Gyrations to get the subjectPublicKeyInfo */ @@ -2409,54 +2407,11 @@ static int pkp_pin_peer_pubkey(X509* cert, char *pinnedpubkey) /* End Gyrations */ - /* See the warning above!!! */ - fp = fopen(pinnedpubkey, "r"); - - if(NULL == fp) - break; /* failed */ - - /* Seek to eof to determine the file's size */ - ret = fseek(fp, 0, SEEK_END); - if(0 != ret) - break; /* failed */ - - /* Fetch the file's size */ - size = ftell(fp); - - /* - * if the size of our certificate doesn't match the size of - * the file, they can't be the same, don't bother reading it - */ - if(len2 != size) - break; /* failed */ - - /* Rewind to beginning to perform the read */ - ret = fseek(fp, 0, SEEK_SET); - if(0 != ret) - break; /* failed */ - - /* http://www.openssl.org/docs/crypto/buffer.html */ - buff2 = OPENSSL_malloc(len2); - if(NULL == buff2) - break; /* failed */ - - /* Returns number of elements read, which should be 1 */ - ret = (int)fread(buff2, (size_t)len2, 1, fp); - if(1 != ret) - break; /* failed */ - /* The one good exit point */ - result = (0 == memcmp(buff1, buff2, (size_t)len2)); - + result = Curl_pin_peer_pubkey(pinnedpubkey, buff1, len1); } while(0); - if(NULL != fp) - fclose(fp); - /* http://www.openssl.org/docs/crypto/buffer.html */ - if(NULL != buff2) - OPENSSL_free(buff2); - if(NULL != buff1) OPENSSL_free(buff1); @@ -2483,6 +2438,7 @@ static CURLcode servercert(struct connectdata *conn, X509 *issuer; FILE *fp; char *buffer = data->state.buffer; + const char *ptr; if(data->set.ssl.certinfo) /* we've been asked to gather certificate info! */ @@ -2586,11 +2542,11 @@ static CURLcode servercert(struct connectdata *conn, infof(data, "\t SSL certificate verify ok.\n"); } - if(data->set.str[STRING_SSL_PINNEDPUBLICKEY] != NULL && - TRUE != pkp_pin_peer_pubkey(connssl->server_cert, - data->set.str[STRING_SSL_PINNEDPUBLICKEY])) { - failf(data, "SSL: public key does not matched pinned public key!"); - return CURLE_SSL_PINNEDPUBKEYNOTMATCH; + ptr = data->set.str[STRING_SSL_PINNEDPUBLICKEY]; + if(retcode == CURLE_OK && ptr) { + retcode = pkp_pin_peer_pubkey(connssl->server_cert, ptr); + if(retcode != CURLE_OK) + failf(data, "SSL: public key does not match pinned public key!"); } X509_free(connssl->server_cert); diff --git a/lib/vtls/vtls.c b/lib/vtls/vtls.c index 7d9894449..fd025b1da 100644 --- a/lib/vtls/vtls.c +++ b/lib/vtls/vtls.c @@ -682,6 +682,62 @@ int Curl_ssl_random(struct SessionHandle *data, return curlssl_random(data, entropy, length); } +/* + * Generic pinned public key check. + */ + +CURLcode Curl_pin_peer_pubkey(const char *pinnedpubkey, + const unsigned char *pubkey, size_t pubkeylen) +{ + FILE *fp = NULL; + unsigned char *buf = NULL; + long size = 0; + CURLcode result = CURLE_SSL_PINNEDPUBKEYNOTMATCH; + + /* if a path wasn't specified, don't pin */ + if(!pinnedpubkey) + return CURLE_OK; + if(!pubkey || !pubkeylen) + return result; + fp = fopen(pinnedpubkey, "rb"); + if(!fp) + return result; + + do { + /* Determine the file's size */ + if(fseek(fp, 0, SEEK_END)) + break; + size = ftell(fp); + if(fseek(fp, 0, SEEK_SET)) + break; + + /* + * if the size of our certificate doesn't match the size of + * the file, they can't be the same, don't bother reading it + */ + if((long) pubkeylen != size) + break; + + /* Allocate buffer for the pinned key. */ + buf = malloc(pubkeylen); + if(!buf) + break; + + /* Returns number of elements read, which should be 1 */ + if((int) fread(buf, pubkeylen, 1, fp) != 1) + break; + + /* The one good exit point */ + if(!memcmp(pubkey, buf, pubkeylen)) + result = CURLE_OK; + } while(0); + + Curl_safefree(buf); + fclose(fp); + + return result; +} + void Curl_ssl_md5sum(unsigned char *tmp, /* input */ size_t tmplen, unsigned char *md5sum, /* output */ diff --git a/lib/vtls/vtls.h b/lib/vtls/vtls.h index 4c29d98a2..99965156d 100644 --- a/lib/vtls/vtls.h +++ b/lib/vtls/vtls.h @@ -108,6 +108,9 @@ void Curl_ssl_md5sum(unsigned char *tmp, /* input */ size_t tmplen, unsigned char *md5sum, /* output */ size_t md5len); +/* Check pinned public key. */ +CURLcode Curl_pin_peer_pubkey(const char *pinnedpubkey, + const unsigned char *pubkey, size_t pubkeylen); #define SSL_SHUTDOWN_TIMEOUT 10000 /* ms */