On 08/26/2014 05:39 PM, Daniel Stenberg wrote: > 1 - build with ./configure --enable-debug and gcc will help you point > out a > lot of C90 non-compliant mistakes, like mixing code and variable > declarations and using // comments. Personally I also use > --enable-werror > to make really sure I don't miss a warning...
Following this and advice in GIT-INFO I am now using this configure line: ./configure --disable-shared --enable-debug --enable-maintainer-mode --enable-debug --enable-werror > 3 - the "Arbitrary size" in pkp_pin_peer_pubkey() is not explained much > but > is set to 2048. How about making it a define, putting it somewhere > at the > top and explaining some reasoning why 2048 might be suitable? I realized this wasn't even actually needed, and simplified the code by comparing the length of the pinned public key file to the length of the public key returned by the server before even reading the file in. I have also fixed the indenting issue, created documentation, and created tests, though I'm not sure the tests are actually correct. As before, the patch is attached and also pushed up to my fork on github: https://github.com/moparisthebest/curl Thanks for the input, and let me know anything else that needs done, moparisthebest
From 3af97630b8c11e1ed56b28efb86549960ff66f04 Mon Sep 17 00:00:00 2001 From: moparisthebest <andr...@moparisthebest.org> Date: Tue, 26 Aug 2014 22:23:06 -0400 Subject: [PATCH] Implement public key pinning Option --pinnedpubkey takes a path to a public key in DER format and only connect if it matches (currently only implemented with OpenSSL). Extract a public RSA key from a website like so: openssl s_client -connect google.com:443 2>&1 < /dev/null | \ sed -n '/-----BEGIN/,/-----END/p' | openssl x509 -noout -pubkey \ | openssl rsa -pubin -outform DER > google.com.der --- docs/curl.1 | 15 ++++ docs/libcurl/opts/CURLOPT_PINNEDPUBLICKEY.3 | 51 +++++++++++++ include/curl/curl.h | 6 ++ lib/strerror.c | 3 + lib/url.c | 8 ++ lib/urldata.h | 1 + lib/vtls/openssl.c | 109 ++++++++++++++++++++++++++++ src/tool_cfgable.c | 1 + src/tool_cfgable.h | 1 + src/tool_getparam.c | 6 ++ src/tool_help.c | 1 + src/tool_operate.c | 3 + tests/data/Makefile.am | 2 +- tests/data/test2034 | 57 +++++++++++++++ tests/data/test2035 | 43 +++++++++++ 15 files changed, 306 insertions(+), 1 deletion(-) create mode 100644 docs/libcurl/opts/CURLOPT_PINNEDPUBLICKEY.3 create mode 100644 tests/data/test2034 create mode 100644 tests/data/test2035 diff --git a/docs/curl.1 b/docs/curl.1 index 6bb2d1c..cb345bb 100644 --- a/docs/curl.1 +++ b/docs/curl.1 @@ -522,6 +522,19 @@ OpenSSL-powered curl to make SSL-connections much more efficiently than using If this option is set, the default capath value will be ignored, and if it is used several times, the last one will be used. +.IP "--pinnedpubkey <pinned public key>" +(SSL) Tells curl to use the specified public key file to verify the peer. The +file must contain a single public key in DER format. + +When negotiating a TLS or SSL connection, the server sends a certificate +indicating its identity. A public key is extracted from this certificate +and if it does not exactly match the public key provided to this option, +curl will abort the connection before sending or receiving any data. + +This is currently only implemented in the OpenSSL backend, with more backends +expected to follow shortly. + +If this option is used several times, the last one will be used. .IP "-f, --fail" (HTTP) Fail silently (no output at all) on server errors. This is mostly done to better enable scripts etc to better deal with failed attempts. In @@ -2162,6 +2175,8 @@ unable to parse FTP file list FTP chunk callback reported error .IP 89 No connection available, the session will be queued +.IP 90 +SSL public key does not matched pinned public key .IP XX More error codes will appear here in future releases. The existing ones are meant to never change. diff --git a/docs/libcurl/opts/CURLOPT_PINNEDPUBLICKEY.3 b/docs/libcurl/opts/CURLOPT_PINNEDPUBLICKEY.3 new file mode 100644 index 0000000..a478065 --- /dev/null +++ b/docs/libcurl/opts/CURLOPT_PINNEDPUBLICKEY.3 @@ -0,0 +1,51 @@ +.\" ************************************************************************** +.\" * _ _ ____ _ +.\" * Project ___| | | | _ \| | +.\" * / __| | | | |_) | | +.\" * | (__| |_| | _ <| |___ +.\" * \___|\___/|_| \_\_____| +.\" * +.\" * Copyright (C) 1998 - 2014, Daniel Stenberg, <dan...@haxx.se>, et al. +.\" * +.\" * This software is licensed as described in the file COPYING, which +.\" * you should have received as part of this distribution. The terms +.\" * are also available at http://curl.haxx.se/docs/copyright.html. +.\" * +.\" * You may opt to use, copy, modify, merge, publish, distribute and/or sell +.\" * copies of the Software, and permit persons to whom the Software is +.\" * furnished to do so, under the terms of the COPYING file. +.\" * +.\" * This software is distributed on an "AS IS" basis, WITHOUT WARRANTY OF ANY +.\" * KIND, either express or implied. +.\" * +.\" ************************************************************************** +.\" +.TH CURLOPT_PINNEDPUBLICKEY 3 "27 Aug 2014" "libcurl 7.38.0" "curl_easy_setopt options" +.SH NAME +CURLOPT_PINNEDPUBLICKEY \- set pinned public key +.SH SYNOPSIS +#include <curl/curl.h> + +CURLcode curl_easy_setopt(CURL *handle, CURLOPT_PINNEDPUBLICKEY, char *pinnedpubkey); +.SH DESCRIPTION +Pass a pointer to a zero terminated string as parameter. The string should be +the file name of your pinned public key. The format expected is "DER". + +When negotiating a TLS or SSL connection, the server sends a certificate +indicating its identity. A public key is extracted from this certificate +and if it does not exactly match the public key provided to this option, +curl will abort the connection before sending or receiving any data. + +This is currently only implemented in the OpenSSL backend, with more backends +expected to follow shortly. +.SH DEFAULT +NULL +.SH PROTOCOLS +All TLS based protocols: HTTPS, FTPS, IMAPS, POP3, SMTPS etc. +.SH EXAMPLE +TODO +.SH AVAILABILITY +If built TLS enabled. +.SH RETURN VALUE +Returns CURLE_OK if TLS enabled, CURLE_UNKNOWN_OPTION if not, or +CURLE_OUT_OF_MEMORY if there was insufficient heap space. diff --git a/include/curl/curl.h b/include/curl/curl.h index d40b2db..ccd9c3b 100644 --- a/include/curl/curl.h +++ b/include/curl/curl.h @@ -521,6 +521,8 @@ typedef enum { CURLE_CHUNK_FAILED, /* 88 - chunk callback reported error */ CURLE_NO_CONNECTION_AVAILABLE, /* 89 - No connection available, the session will be queued */ + CURLE_SSL_PINNEDPUBKEYNOTMATCH, /* 90 - specified pinned public key did not + match */ CURL_LAST /* never use! */ } CURLcode; @@ -1611,6 +1613,10 @@ typedef enum { /* Pass in a bitmask of "header options" */ CINIT(HEADEROPT, LONG, 229), + /* The public key in DER form used to validate the peer public key + this option is used only if SSL_VERIFYPEER is true */ + CINIT(PINNEDPUBLICKEY, OBJECTPOINT, 230), + CURLOPT_LASTENTRY /* the last unused */ } CURLoption; diff --git a/lib/strerror.c b/lib/strerror.c index 66033f2..1a13606 100644 --- a/lib/strerror.c +++ b/lib/strerror.c @@ -298,6 +298,9 @@ curl_easy_strerror(CURLcode error) case CURLE_NO_CONNECTION_AVAILABLE: return "The max connection limit is reached"; + case CURLE_SSL_PINNEDPUBKEYNOTMATCH: + return "SSL public key does not matched pinned public key"; + /* error codes not used by current libcurl */ case CURLE_OBSOLETE20: case CURLE_OBSOLETE24: diff --git a/lib/url.c b/lib/url.c index 89c3fd5..b8b6560 100644 --- a/lib/url.c +++ b/lib/url.c @@ -1983,6 +1983,14 @@ CURLcode Curl_setopt(struct SessionHandle *data, CURLoption option, data->set.ssl.certinfo = (0 != va_arg(param, long))?TRUE:FALSE; break; #endif + case CURLOPT_PINNEDPUBLICKEY: + /* + * Set pinned public key for SSL connection. + * Specify file name of the public key in DER format. + */ + result = setstropt(&data->set.str[STRING_SSL_PINNEDPUBLICKEY], + va_arg(param, char *)); + break; case CURLOPT_CAINFO: /* * Set CA info for SSL connection. Specify file name of the CA certificate diff --git a/lib/urldata.h b/lib/urldata.h index 8594c2f..fd59d78 100644 --- a/lib/urldata.h +++ b/lib/urldata.h @@ -1385,6 +1385,7 @@ enum dupstring { STRING_SET_URL, /* what original URL to work on */ STRING_SSL_CAPATH, /* CA directory name (doesn't work on windows) */ STRING_SSL_CAFILE, /* certificate file to verify peer against */ + STRING_SSL_PINNEDPUBLICKEY, /* public key file to verify peer against */ STRING_SSL_CIPHER_LIST, /* list of ciphers to use */ STRING_SSL_EGDSOCKET, /* path to file containing the EGD daemon socket */ STRING_SSL_RANDOM_FILE, /* path to file containing "random" data */ diff --git a/lib/vtls/openssl.c b/lib/vtls/openssl.c index da92854..855bdee 100644 --- a/lib/vtls/openssl.c +++ b/lib/vtls/openssl.c @@ -2357,6 +2357,108 @@ 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) +{ + /* Scratch */ + FILE* fp = NULL; + int len1 = 0, len2 = 0; + unsigned char *buff1 = NULL, *buff2 = NULL, *temp = NULL; + + /* Result is returned to caller */ + int ret = 0, result = FALSE; + + /* if a path wasn't specified, don't pin */ + if(NULL == pinnedpubkey) return TRUE; + if(NULL == cert) return FALSE; + + do { + /* Begin Gyrations to get the subjectPublicKeyInfo */ + /* Thanks to Viktor Dukhovni on the OpenSSL mailing list */ + + /* http://groups.google.com/group/mailing.openssl.users/browse_thread + /thread/d61858dae102c6c7 */ + len1 = i2d_X509_PUBKEY(X509_get_X509_PUBKEY(cert), NULL); + if(len1 < 1) + break; /* failed */ + + /* http://www.openssl.org/docs/crypto/buffer.html */ + buff1 = temp = OPENSSL_malloc(len1); + if(NULL == buff1) + break; /* failed */ + + /* http://www.openssl.org/docs/crypto/d2i_X509.html */ + len2 = i2d_X509_PUBKEY(X509_get_X509_PUBKEY(cert), &temp); + + /* + * These checks are verifying we got back the same values as when we + * sized the buffer.Its pretty weak since they should always be the + * same. But it gives us something to test. + */ + if(!((len1 == len2) && (temp != NULL) && ((temp - buff1) == len1))) + break; /* failed */ + + /* End Gyrations */ + + /* See the warning above!!! */ + fp = fopen(pinnedpubkey, "rx"); + if(NULL == fp) + 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 */ + len2 = (int)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(len1 != len2) + 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)); + + } 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); + + return result; +} + +/* * Get the server cert, verify it and show it etc, only call failf() if the * 'strict' argument is TRUE as otherwise all this is for informational * purposes only! @@ -2479,6 +2581,13 @@ 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; + } + X509_free(connssl->server_cert); connssl->server_cert = NULL; connssl->connecting_state = ssl_connect_done; diff --git a/src/tool_cfgable.c b/src/tool_cfgable.c index 2fdae07..bd8707e 100644 --- a/src/tool_cfgable.c +++ b/src/tool_cfgable.c @@ -101,6 +101,7 @@ static void free_config_fields(struct OperationConfig *config) Curl_safefree(config->cacert); Curl_safefree(config->capath); Curl_safefree(config->crlfile); + Curl_safefree(config->pinnedpubkey); Curl_safefree(config->key); Curl_safefree(config->key_type); Curl_safefree(config->key_passwd); diff --git a/src/tool_cfgable.h b/src/tool_cfgable.h index 4ef2690..11a6a98 100644 --- a/src/tool_cfgable.h +++ b/src/tool_cfgable.h @@ -110,6 +110,7 @@ struct OperationConfig { char *cacert; char *capath; char *crlfile; + char *pinnedpubkey; char *key; char *key_type; char *key_passwd; diff --git a/src/tool_getparam.c b/src/tool_getparam.c index 180878b..242a2d8 100644 --- a/src/tool_getparam.c +++ b/src/tool_getparam.c @@ -215,6 +215,7 @@ static const struct LongShort aliases[]= { {"Em", "tlsauthtype", TRUE}, {"En", "ssl-allow-beast", FALSE}, {"Eo", "login-options", TRUE}, + {"Ep", "pinnedpubkey", TRUE}, {"f", "fail", FALSE}, {"F", "form", TRUE}, {"Fs", "form-string", TRUE}, @@ -1358,6 +1359,11 @@ ParameterError getparameter(char *flag, /* f or -long-flag */ GetStr(&config->login_options, nextarg); break; + case 'p': /* Pinned public key DER file */ + /* Pinned public key DER file */ + GetStr(&config->pinnedpubkey, nextarg); + break; + default: /* certificate file */ { char *certname, *passphrase; diff --git a/src/tool_help.c b/src/tool_help.c index c255be0..cad12a3 100644 --- a/src/tool_help.c +++ b/src/tool_help.c @@ -51,6 +51,7 @@ static const char *const helptext[] = { " --basic Use HTTP Basic Authentication (H)", " --cacert FILE CA certificate to verify peer against (SSL)", " --capath DIR CA directory to verify peer against (SSL)", + " --pinnedpubkey FILE Public key (DER) to verify peer against (SSL)", " -E, --cert CERT[:PASSWD] Client certificate file and password (SSL)", " --cert-type TYPE Certificate file type (DER/PEM/ENG) (SSL)", " --ciphers LIST SSL ciphers to use (SSL)", diff --git a/src/tool_operate.c b/src/tool_operate.c index fd2fd6d..488fb08 100644 --- a/src/tool_operate.c +++ b/src/tool_operate.c @@ -1025,6 +1025,9 @@ static CURLcode operate_do(struct GlobalConfig *global, if(config->crlfile) my_setopt_str(curl, CURLOPT_CRLFILE, config->crlfile); + if(config->pinnedpubkey) + my_setopt_str(curl, CURLOPT_PINNEDPUBLICKEY, config->pinnedpubkey); + if(curlinfo->features & CURL_VERSION_SSL) { if(config->insecure_ok) { my_setopt(curl, CURLOPT_SSL_VERIFYPEER, 0L); diff --git a/tests/data/Makefile.am b/tests/data/Makefile.am index 4546135..8ed4404 100644 --- a/tests/data/Makefile.am +++ b/tests/data/Makefile.am @@ -138,7 +138,7 @@ test2000 test2001 test2002 test2003 test2004 test2005 test2006 test2007 \ test2008 test2009 test2010 test2011 test2012 test2013 test2014 test2015 \ test2016 test2017 test2018 test2019 test2020 test2021 test2022 test2023 \ test2024 test2025 test2026 test2027 test2028 test2029 test2030 test2031 \ -test2032 test2033 +test2032 test2033 test2034 test2035 EXTRA_DIST = $(TESTCASES) DISABLED diff --git a/tests/data/test2034 b/tests/data/test2034 new file mode 100644 index 0000000..80a2e8d --- /dev/null +++ b/tests/data/test2034 @@ -0,0 +1,57 @@ +<testcase> +<info> +<keywords> +HTTPS +HTTP GET +PEM certificate +</keywords> +</info> + +# +# Server-side +<reply> +<data> +HTTP/1.1 200 OK +Date: Thu, 09 Nov 2010 14:49:00 GMT +Server: test-server/fake +Content-Length: 7 + +MooMoo +</data> +</reply> + +# +# Client-side +<client> +<features> +SSL +</features> +<server> +https Server-localhost-sv.pem +</server> + <name> +simple HTTPS GET + </name> + <command> +--pinnedpubkey %SRCDIR/certs/Server-localhost-sv.der https://localhost:%HTTPSPORT/2034 +</command> +# Ensure that we're running on localhost because we're checking the host name +<precheck> +perl -e "print 'Test requires default test server host' if ( '%HOSTIP' ne '127.0.0.1' );" +</precheck> +</client> + +# +# Verify data after the test has been "shot" +<verify> +<strip> +^User-Agent:.* +</strip> +<protocol> +GET /310 HTTP/1.1 +Host: localhost:%HTTPSPORT +Accept: */* + +</protocol> +</verify> +</testcase> diff --git a/tests/data/test2035 b/tests/data/test2035 new file mode 100644 index 0000000..c1351b8 --- /dev/null +++ b/tests/data/test2035 @@ -0,0 +1,43 @@ +<testcase> +<info> +<keywords> +HTTPS +HTTP GET +PEM certificate +</keywords> +</info> + +# +# Server-side +<reply> +</reply> + +# +# Client-side +<client> +<features> +SSL +</features> +<server> +https Server-localhost-sv.pem +</server> + <name> +HTTPS wrong subjectAltName but right CN + </name> + <command> +--pinnedpubkey %SRCDIR/certs/Server-localhost0h-sv.der https://localhost:%HTTPSPORT/2035 +</command> +# Ensure that we're running on localhost because we're checking the host name +<precheck> +perl -e "print 'Test requires default test server host' if ( '%HOSTIP' ne '127.0.0.1' );" +</precheck> +</client> + +# +# Verify data after the test has been "shot" +<verify> +<errorcode> +90 +</errorcode> +</verify> +</testcase> -- 1.9.2
------------------------------------------------------------------- List admin: http://cool.haxx.se/list/listinfo/curl-library Etiquette: http://curl.haxx.se/mail/etiquette.html
