Is certificate pinning possible?

[0xDBFB7]

I'm sorry, I don't know enough about mbedtls or the MicroPython SSL implementation to be able to make this change, but it looks like the certificate information is exposed in a mbedtls_x509_crt struct (https://tls.mbed.org/discussions/generic/public-key-pinning) - would it be possible to access this struct though the python interface when wrapping a socket in SSL, in order to hash it for cert pinning? This would be an exceedingly useful feature, at least for me.

[MrSurly]

I'm no expert on mbedtls (or crypto), but I'll take a stab at it. From the link you posted, it looks like it's fairly straightforward to extract the pub key, only question is how to get the peer cert.

[0xDBFB7]

@MrSurly Oh, great, thank you so much.

[MrSurly]

Working on this here: https://github.com/MrSurly/micropython-esp32/tree/dev-cert-pinning

• Getting the peer cert is easy
• Print a cert in DER format is easy
• Copying the fields from the peer cert to the object that prints certs is cumbersome, as the data formats are different, and need to be converted

I only have a little bit of time to work on this every day. I know about point A and point B, I'm just trying to learn how to get there with in the mbedtls API.

[MrSurly]

I reached out to Malte Janduda; here's his response:

Hi Eric,

at the moment this is on hold in my project. But we will work on this within the next two to three months.

In the end the whole thing goes like this:

1. Get the peer's TLS certificate
2. Extract the Public Key in DER format
3. Hash the extracted key using SHA256
4. Base64 encode the hash
5. Check if the current public key is equal to the pinned key (and save the hash for the next time) Hope this helps. Best, Malte

This approach makes sense; the fields in the DER format shouldn't change, thus you'll have a consistent hash.

When you get the peer cert using the library, it's returned as a binary structure. It's almost guaranteed that hashing this structure will not result in a consistent hash, as many data structures have internal pointers, which will be different depending on where the data is stored in memory.

[MrSurly]

Hmm, I just noticed that the cert has a field called raw which is documented as:

The raw certificate data (DER).

This sounds promising.

[annejan]

On the SHA2017 badge we are doing full chain certificate validation for *.sha2017.org

Check from https://github.com/SHA2017-badge/micropython-esp32/blob/master/extmod/modussl_mbedtls.c#L149 and especially https://github.com/SHA2017-badge/micropython-esp32/blob/master/extmod/modussl_mbedtls.c#L184

Hope this helps . .

[MrSurly]

@BasedOnTechnology: Is full-cert pinning acceptable? The DER returned by mbedtls looks legit. If whomever you're connecting to doesn't rotate certs, you're probably ok.

[MrSurly]

Would work like this:

import binascii
import hashlib

# ... establish SSL socket as 'socket'

cert = socket.get_peer_certificate()
certHash = binascii.hexlify(hashlib.sha256(cert).digest())

[0xDBFB7]

@MrSurly That's perfect, exactly what I need!

I control the certs in my application and they're just static self-signed certs, so it's fine.

[MrSurly]

Ok, done.

https://github.com/MrSurly/micropython-esp32/tree/dev-cert-pinning

@dpgeorge Would this be a welcome PR for mainline?

[0xDBFB7]

@MrSurly Finally got around to setting up the esp-idf and compiling your branch - that function works beautifully, even returns the same fingerprint as chrome. Thanks again for the help.

I feel this would indeed be a good addition to master, though probably couldn't be merged upstream due to the mbedtls requirement, right?

[MrSurly]

@BasedOnTechnology

Glad it worked. In the past, @dpgeorge has asked that mbedtls PRs (e.g. microtpython-32 #133 became microtpython #3226) be done against the upstream MicroPython, hence my question directed at him, above.

[dpgeorge]

Would this be a welcome PR for mainline?

I would need to understand more about this public key pinning thing first... how would one do such a thing with CPython (is it even possible)?

[0xDBFB7]

@dpgeorge Looks like it, the requests library seems to include a verify option:

r = requests.get('https://yoursite.com/', verify='certfile.crt')

And also a very similar implementation as here, conn.getpeercert(True):

http://bugs.python.org/issue18735

[dpgeorge]

@BasedOnTechnology thanks for the pointer, it looks like getpeercert() is what we want to implement (on an SSL socket).

[MrSurly]

@dpgeorge

it looks like getpeercert() is what we want to implement (on an SSL socket).

Yes, that's exactly the change I made, though the function name is get_peer_cert() -- should I change the name to getpeercert() and submit to mainline µPy?

diff --git a/extmod/modussl_mbedtls.c b/extmod/modussl_mbedtls.c
index 597eaee..0847d7b 100644
--- a/extmod/modussl_mbedtls.c
+++ b/extmod/modussl_mbedtls.c
@@ -189,6 +189,13 @@ STATIC mp_obj_ssl_socket_t *socket_new(mp_obj_t sock, struct ssl_args *args) {
     return o;
 }

+STATIC mp_obj_t get_peer_cert(mp_obj_t o_in) {
+    mp_obj_ssl_socket_t *o = MP_OBJ_TO_PTR(o_in);
+    const mbedtls_x509_crt* peer_cert = mbedtls_ssl_get_peer_cert(&o->ssl);
+    return mp_obj_new_bytearray(peer_cert->raw.len, peer_cert->raw.p);
+}
+STATIC MP_DEFINE_CONST_FUN_OBJ_1(get_peer_cert_obj, get_peer_cert);
+
 STATIC void socket_print(const mp_print_t *print, mp_obj_t self_in, mp_print_kind_t kind) {
     (void)kind;
     mp_obj_ssl_socket_t *self = MP_OBJ_TO_PTR(self_in);
@@ -259,6 +266,7 @@ STATIC const mp_rom_map_elem_t ussl_socket_locals_dict_table[] = {
     { MP_ROM_QSTR(MP_QSTR_write), MP_ROM_PTR(&mp_stream_write_obj) },
     { MP_ROM_QSTR(MP_QSTR_setblocking), MP_ROM_PTR(&socket_setblocking_obj) },
     { MP_ROM_QSTR(MP_QSTR_close), MP_ROM_PTR(&socket_close_obj) },
+    { MP_ROM_QSTR(MP_QSTR_get_peer_cert), MP_ROM_PTR(&get_peer_cert_obj) },
 };

 STATIC MP_DEFINE_CONST_DICT(ussl_socket_locals_dict, ussl_socket_locals_dict_table);

[dpgeorge]

should I change the name to getpeercert() and submit to mainline µPy?

@MrSurly Yes please. To make it compat with CPython it would need to take an extra arg and we'd only support that arg being True (so it returns data in raw DER format). And the returned object should be a bytes (no bytearray).

[MrSurly]

Will do.

[MrSurly]

@dpgeorge

Behavior

Please verify that this is the desired behavior. C Python defines: SSLSocket.getpeercert(binary_form=False)

• [X] get_peer_cert() --> getpeercert()
• [X] returns bytes rather than bytearray
• [X] has a single (optionally kw) arg binary_form which defaults to False, but must be True, since we're not supporting the dict return type.
• [X] returns cert in DER form, per C Python

>>> cert = c.sock.getpeercert()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ValueError: binary_form must be True
>>> cert = c.sock.getpeercert(True)
>>> cert = c.sock.getpeercert(binary_form = True)
>>> cert[:5]
b'0\x82\x04\x8e0'

Implementation:

STATIC mp_obj_t mod_ssl_getpeercert(size_t n_args, const mp_obj_t *pos_args, mp_map_t *kw_args) {
    static const mp_arg_t allowed_args[] = {
        { MP_QSTR_binary_form, MP_ARG_OBJ, {.u_obj = mp_const_false} }
    };
    mp_obj_ssl_socket_t *o = MP_OBJ_TO_PTR(pos_args[0]);
    mp_arg_val_t args[1];
    mp_arg_parse_all(n_args - 1, pos_args + 1, kw_args,
        MP_ARRAY_SIZE(allowed_args), allowed_args, args);
    if (args[0].u_obj != mp_const_true) {
        mp_raise_ValueError("binary_form must be True");
    }
    const mbedtls_x509_crt* peer_cert = mbedtls_ssl_get_peer_cert(&o->ssl);
    return mp_obj_new_bytes(peer_cert->raw.p, peer_cert->raw.len);
}
STATIC MP_DEFINE_CONST_FUN_OBJ_KW(mod_ssl_getpeercert_obj, 1, mod_ssl_getpeercert);

NB

This is unfortunate (argument ordering) =P:

mp_obj_t mp_obj_new_bytes(const byte* data, size_t len);
mp_obj_t mp_obj_new_bytearray(size_t n, void *items);

[MrSurly]

@BasedOnTechnology Note API changes which will land in micropython-esp32

[dpgeorge]

@MrSurly thanks! But generally uPy does not implement keyword args for built-in functions, especially one like this that is so simple (ie only 0 or 1 arg). So please save the code/bytes and make the function just take exactly 1 arg, which is verified to be True (else raises the exception).

This is unfortunate (argument ordering) =P:

Yeah, there are some mild inconsistencies here. It seems that the pattern is usually (buf, len) for a string or byte buffer, and (n, items) for an array of objects.

[MrSurly]

I'll get it changed and submit at PR Monday. Tuesday.

On Aug 12, 2017 5:14 AM, "Damien George" notifications@github.com wrote:

@MrSurly https://github.com/mrsurly thanks! But generally uPy does not implement keyword args for built-in functions, especially one like this that is so simple (ie only 0 or 1 arg). So please save the code/bytes and make the function just take exactly 1 arg, which is verified to be True (else raises the exception).

This is unfortunate (argument ordering) =P:

Yeah, there are some mild inconsistencies here. It seems that the pattern is usually (buf, len) for a string or byte buffer, and (n, items) for an array of objects.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/micropython/micropython-esp32/issues/144#issuecomment-321977468, or mute the thread https://github.com/notifications/unsubscribe-auth/AAYHCBi1igZ7S8m2PfJJjfluBfpUPipAks5sXZc7gaJpZM4Onrng .

[MrSurly]

@BasedOnTechnology I think this can be closed?

[0xDBFB7]

@MrSurly Yep, sorry.