mbedtls: Implement CURLOPT_PINNEDPUBLICKEY

[sithglan]

Hello,
this implements CURLOPT_PINNEDPUBLICKEY for mbedtls. I tested it with sha256// on mingw-win32 and Linux. I have it in production for 20 users since one week. Please review and merge. I'm happy to work in any improvements.

Cheers,
Thomas

[mention-bot]

By analyzing the blame information on this pull request, we identified @moparisthebest, @sasq64 and @bagder to be potential reviewers

[bagder]

Lovely, is there a particular mbedtls version required?

[moparisthebest]

It looks like Curl_pin_peer_pubkey is used correctly, and curlssl_sha256sum is defined so sha256 pins should work.

My only concern is due to 'unsigned char pubkey[1024];' in mbedtls_verify_pinned_crt it looks like there is a hard-coded upper limit to the number of bits in a public key? Does it work with 4096 or 8192 bit RSA keys? The other backends malloc a char* with the exact size needed.

Edit:
Actually due to the call to mbedtls_pk_write_pubkey_der(&crt->pk, pubkey, 1024); it looks like maybe ONLY 1024 bit keys work? How about a 256-bit EC key?

You can use moparisthebest.com for testing against a 4096-bit RSA key, and any site with cloudflare TLS enabled (like cloudflare.com) for a 256-bit EC key.

[sithglan]

Hello @bagder,
the interface is available starting with mbedtls version mbedtls-1.4-dtls-preview-306-g2cf5a7c according to the git repo. This was the version where the great renaming happened.

Hello @moparisthebest,
I will fix this. I'm pretty sure that 2048 bit key works. Because I currently use it. The 1024 is Bytes, not bits. So It is 1024 Byte * 8 Bit = 8192 bit keys is the maximum. I'll fix this as well and resubmit my patch.

Cheers,
Thomas

[sithglan]

I was unable to determine the exact size necessary for the buffer and will ask the mbedtls people to tell me how to determine the size of the buffer. Once I have an answer, I'll complete my patch and push it.

[sithglan]

Related: https://tls.mbed.org/discussions/generic/howto-determine-exact-buffer-len-for-mbedtls_pk_write_pubkey_der

[sithglan]

Manuel answered:

Good question! I'm afraid we currently don't offer the function you're looking for. Unfortunately, I'm > not sure it would be easy to add.

The code you're currently using is a good idea and should work with most real-world RSA keys
(that use a short public exponent), but it will unfortunately not work with EC keys as is (not tested,
but I don't think it will work). For formulas that should work of all keys (that is, unless I made a
mistake), you can look here for RSA and here for ECC.

Actually I'm starting to wonder if we should just move those definitions from pkwrite.c to pk.h so that > you can use them directly...

You can either use that as the size of a static buffer as we do in pk_write_pubkey_pem() or replace > the MAX_SIZE part of the formulas with the return value of pk_get_len() for your particular key and > then allocate a buffer of the corresponding size. Please note that this will not be the optimal size,
especially for RSA keys (the public exponent is usually 24 bits tough it can be as large as the size
of the key in theory).

Another, entirely different approach, is to start by allocating a buffer of a reasonable size, try
writing the key to it, if it works you're done, otherwise double the size of the buffer and iterate.

I calculated the worst case size. Please review. If you're ready to pull let me know if you want to keep the whole history or if I should make a small single patch ready that you're pulling.

Cheers,
Thomas

[moparisthebest]

Seems like an odd API that will write something to a buffer for you, yet not be able to tell you how to size the buffer.

Anyhow your code looks like the best that can be done given the circumstances, good work!

I think they generally like a single patch/commit per feature like this, but bagder is the final word on that. :)

[bagder]

Yes please, unless the history has some special significance, we prefer to merge a single logic change in a single commit.

[sithglan]

Hello Daniel,
I sent the newest version of the patch as [PATCHv2] mbedtls: Implement CURLOPT_PINNEDPUBLICKEY to the list.

Cheers,
Thomas