mirror of
https://github.com/moparisthebest/curl
synced 2024-12-21 23:58:49 -05:00
mbedtls: Fix pinned key return value on fail
- Switch from verifying a pinned public key in a callback during the certificate verification to inline after the certificate verification. The callback method had three problems: 1. If a pinned public key didn't match, CURLE_SSL_PINNEDPUBKEYNOTMATCH was not returned. 2. If peer certificate verification was disabled the pinned key verification did not take place as it should. 3. (related to #2) If there was no certificate of depth 0 the callback would not have checked the pinned public key. Though all those problems could have been fixed it would have made the code more complex. Instead we now verify inline after the certificate verification in mbedtls_connect_step2. Ref: http://curl.haxx.se/mail/lib-2016-01/0047.html Ref: https://github.com/bagder/curl/pull/601
This commit is contained in:
parent
d566371130
commit
d58ba66eec
@ -148,45 +148,7 @@ const mbedtls_x509_crt_profile mbedtls_x509_crt_profile_fr =
|
||||
#define ECP_PUB_DER_MAX_BYTES (30 + 2 * MBEDTLS_ECP_MAX_BYTES)
|
||||
|
||||
#define PUB_DER_MAX_BYTES (RSA_PUB_DER_MAX_BYTES > ECP_PUB_DER_MAX_BYTES ? \
|
||||
RSA_PUB_DER_MAX_BYTES : ECP_PUB_DER_MAX_BYTES)
|
||||
|
||||
static int
|
||||
mbedtls_verify_pinned_crt(void *p, mbedtls_x509_crt *crt,
|
||||
int depth, unsigned int *flags)
|
||||
{
|
||||
struct SessionHandle *data = p;
|
||||
unsigned char pubkey[PUB_DER_MAX_BYTES];
|
||||
int ret;
|
||||
int size;
|
||||
char *pinned_cert = data->set.str[STRING_SSL_PINNEDPUBLICKEY];
|
||||
|
||||
/* Skip intermediate and root certificates */
|
||||
if(depth) {
|
||||
return 0;
|
||||
}
|
||||
|
||||
if(pinned_cert == NULL || crt == NULL) {
|
||||
*flags |= MBEDTLS_X509_BADCERT_NOT_TRUSTED;
|
||||
return 1;
|
||||
}
|
||||
|
||||
/* Extract pubkey */
|
||||
size = mbedtls_pk_write_pubkey_der(&crt->pk, pubkey, PUB_DER_MAX_BYTES);
|
||||
if(size <= 0) {
|
||||
*flags |= MBEDTLS_X509_BADCERT_NOT_TRUSTED;
|
||||
return 1;
|
||||
}
|
||||
|
||||
/* mbedtls_pk_write_pubkey_der writes data at the end of the buffer. */
|
||||
ret = Curl_pin_peer_pubkey(data, pinned_cert,
|
||||
&pubkey[PUB_DER_MAX_BYTES - size], size);
|
||||
if(ret == CURLE_OK) {
|
||||
return 0;
|
||||
}
|
||||
|
||||
*flags |= MBEDTLS_X509_BADCERT_NOT_TRUSTED;
|
||||
return 1;
|
||||
}
|
||||
RSA_PUB_DER_MAX_BYTES : ECP_PUB_DER_MAX_BYTES)
|
||||
|
||||
static Curl_recv mbedtls_recv;
|
||||
static Curl_send mbedtls_send;
|
||||
@ -455,7 +417,7 @@ mbedtls_connect_step2(struct connectdata *conn,
|
||||
int ret;
|
||||
struct SessionHandle *data = conn->data;
|
||||
struct ssl_connect_data* connssl = &conn->ssl[sockindex];
|
||||
char buffer[1024];
|
||||
const mbedtls_x509_crt *peercert;
|
||||
|
||||
#ifdef HAS_ALPN
|
||||
const char* next_protocol;
|
||||
@ -510,13 +472,72 @@ mbedtls_connect_step2(struct connectdata *conn,
|
||||
return CURLE_PEER_FAILED_VERIFICATION;
|
||||
}
|
||||
|
||||
if(mbedtls_ssl_get_peer_cert(&(connssl->ssl))) {
|
||||
/* If the session was resumed, there will be no peer certs */
|
||||
memset(buffer, 0, sizeof(buffer));
|
||||
peercert = mbedtls_ssl_get_peer_cert(&connssl->ssl);
|
||||
|
||||
if(mbedtls_x509_crt_info(buffer, sizeof(buffer), (char *)"* ",
|
||||
mbedtls_ssl_get_peer_cert(&(connssl->ssl))) != -1)
|
||||
if(peercert && data->set.verbose) {
|
||||
const size_t bufsize = 16384;
|
||||
char *buffer = malloc(bufsize);
|
||||
|
||||
if(!buffer)
|
||||
return CURLE_OUT_OF_MEMORY;
|
||||
|
||||
if(mbedtls_x509_crt_info(buffer, bufsize, "* ", peercert) > 0)
|
||||
infof(data, "Dumping cert info:\n%s\n", buffer);
|
||||
else
|
||||
infof(data, "Unable to dump certificate information.\n");
|
||||
|
||||
free(buffer);
|
||||
}
|
||||
|
||||
if(data->set.str[STRING_SSL_PINNEDPUBLICKEY]) {
|
||||
int size;
|
||||
CURLcode result;
|
||||
mbedtls_x509_crt *p;
|
||||
unsigned char pubkey[PUB_DER_MAX_BYTES];
|
||||
|
||||
if(!peercert || !peercert->raw.p || !peercert->raw.len) {
|
||||
failf(data, "Failed due to missing peer certificate");
|
||||
return CURLE_SSL_PINNEDPUBKEYNOTMATCH;
|
||||
}
|
||||
|
||||
p = calloc(1, sizeof(*p));
|
||||
|
||||
if(!p)
|
||||
return CURLE_OUT_OF_MEMORY;
|
||||
|
||||
mbedtls_x509_crt_init(p);
|
||||
|
||||
/* Make a copy of our const peercert because mbedtls_pk_write_pubkey_der
|
||||
needs a non-const key, for now.
|
||||
https://github.com/ARMmbed/mbedtls/issues/396 */
|
||||
if(mbedtls_x509_crt_parse_der(p, peercert->raw.p, peercert->raw.len)) {
|
||||
failf(data, "Failed copying peer certificate");
|
||||
mbedtls_x509_crt_free(p);
|
||||
free(p);
|
||||
return CURLE_SSL_PINNEDPUBKEYNOTMATCH;
|
||||
}
|
||||
|
||||
size = mbedtls_pk_write_pubkey_der(&p->pk, pubkey, PUB_DER_MAX_BYTES);
|
||||
|
||||
if(size <= 0) {
|
||||
failf(data, "Failed copying public key from peer certificate");
|
||||
mbedtls_x509_crt_free(p);
|
||||
free(p);
|
||||
return CURLE_SSL_PINNEDPUBKEYNOTMATCH;
|
||||
}
|
||||
|
||||
/* mbedtls_pk_write_pubkey_der writes data at the end of the buffer. */
|
||||
result = Curl_pin_peer_pubkey(data,
|
||||
data->set.str[STRING_SSL_PINNEDPUBLICKEY],
|
||||
&pubkey[PUB_DER_MAX_BYTES - size], size);
|
||||
if(result) {
|
||||
mbedtls_x509_crt_free(p);
|
||||
free(p);
|
||||
return result;
|
||||
}
|
||||
|
||||
mbedtls_x509_crt_free(p);
|
||||
free(p);
|
||||
}
|
||||
|
||||
#ifdef HAS_ALPN
|
||||
@ -683,10 +704,6 @@ mbedtls_connect_common(struct connectdata *conn,
|
||||
long timeout_ms;
|
||||
int what;
|
||||
|
||||
if(data->set.str[STRING_SSL_PINNEDPUBLICKEY]) {
|
||||
mbedtls_ssl_conf_verify(&connssl->config, mbedtls_verify_pinned_crt, data);
|
||||
}
|
||||
|
||||
/* check if the connection has already been established */
|
||||
if(ssl_connection_complete == connssl->state) {
|
||||
*done = TRUE;
|
||||
|
Loading…
Reference in New Issue
Block a user