Skip to content

Commit

Permalink
mbedtls: Fix pinned key return value on fail. draft3
Browse files Browse the repository at this point in the history
- More correct cleanup pattern for the copy of peer cert *p.

- Add some descriptive error messages where appropriate.

- Change name ret => result to eliminate variable shadowing warning.

- Remove comment about no peer cert on session resume, since that's not
true of mbedTLS (the comment was carried over from PolarSSL code).

- Allocate cert info buffer on heap instead of stack.


Prior to these changes: In draft 2 the code worked fine, however I
marked it as 'FAIL' because all my tests failed due to what I later
realized was just a copy&paste error on my part: I hadn't enabled
curlssl_sha256sum even though I was running the sha256// tests.
  • Loading branch information
jay committed Jan 18, 2016
1 parent 94ff5e6 commit 5972e52
Showing 1 changed file with 30 additions and 17 deletions.
47 changes: 30 additions & 17 deletions lib/vtls/mbedtls.c
Expand Up @@ -148,7 +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)
RSA_PUB_DER_MAX_BYTES : ECP_PUB_DER_MAX_BYTES)

static Curl_recv mbedtls_recv;
static Curl_send mbedtls_send;
Expand Down Expand Up @@ -472,26 +472,33 @@ mbedtls_connect_step2(struct connectdata *conn,
return CURLE_PEER_FAILED_VERIFICATION;
}

/* If the session was resumed, there will be no peer cert */
peercert = mbedtls_ssl_get_peer_cert(&connssl->ssl);

if(peercert && data->set.verbose) {
char buffer[16384];
const size_t bufsize = 16384;
char *buffer = malloc(bufsize);

if(mbedtls_x509_crt_info(buffer, sizeof(buffer), "* ", peercert) > 0)
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 ret;
CURLcode result;
mbedtls_x509_crt *p;
unsigned char pubkey[PUB_DER_MAX_BYTES];

if(!peercert || !peercert->raw.p || !peercert->raw.len)
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));

Expand All @@ -504,27 +511,33 @@ mbedtls_connect_step2(struct connectdata *conn,
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 parsing peer certificate");
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);

mbedtls_x509_crt_free(p);
free(p);
p = NULL;

if(size <= 0)
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. */
ret = Curl_pin_peer_pubkey(data,
data->set.str[STRING_SSL_PINNEDPUBLICKEY],
&pubkey[PUB_DER_MAX_BYTES - size], size);
if(ret)
return ret;
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
Expand Down

0 comments on commit 5972e52

Please sign in to comment.