Certain certificates aren't valid due to missing struct fields

[Wertzui123]

Firstly, sorry if this issue is written poorly - I just noticed the symptoms of this, but I don't (yet) understand TLS or the TLSe code base well enough to actually understand what's the problem here.

Anyway, so I've tried to reach github.com (or www.github.com, it doesn't matter) on port 443 using the tlsclienthello.c example. It doesn't work though. So I tried to investigate the issue, and it looks like there's a problem with the certificate validation.

tls_certificate_verify_signature returns 0 due to cert->sign_key etc. being zeroed; this seems like a bug to me.

tls_certificate_verify_signature is called by tls_certificate_chain_is_valid.

Am I doing anything wrong? Or is this a known issue already? Thank you!

[Wertzui123]

This happens with TLS 1.2 as well as TLS 1.3.

[eduardsui]

You're wright! The problems seems to be with:

int tls_certificate_verify_signature(struct TLSCertificate *cert, struct TLSCertificate *parent) {
    if ((!cert) || (!parent) || (!cert->sign_key) || (!cert->fingerprint) || (!cert->sign_len) || (!parent->der_bytes) || (!parent->der_len)) {
        DEBUG_PRINT("CANNOT VERIFY SIGNATURE\n");
        return 0;
    }

If you replace return 0 with return 1, it will work. I think this is a certificate parsing issue.

It seems that the problem is in certificate parse.

[Wertzui123]

Hm, okay. Thanks for the workaround!

[eduardsui]

The correct solution is to have a full ANS1 X.509 implementation (mine is a minimalist-heuristic one). I will try to generate an alternative implementation with asn1c and select one or the other via flags. For the first test I've made, the X.509 implementation has 50% more lines of code than tlse.c :), but it works with any certificate. I want for TLSe to be as lightweight as possible while providing full TLS 1.2/1.3/DTLS/SRTP support.

[Wertzui123]

50% sounds quite a lot indeed. I don't care too much about size though, so if you could maybe deploy it as a seperate file in the TLSe repo that would be fine for me as well. Or if you could somehow trim it down, that would be even more amazing of course.

[eduardsui]

I understand, but I want TLSe to also run on chips (like ESP32). I'll figure something out :)

Thanks!

[Wertzui123]

Quick question: in ec575f9, you made TLSe simply accept such "broken" certificates without validation, right? Is this a potential security threat or is it safe to actually use that version in practice?

[eduardsui]

@Wertzui123 yes, it is a security issue. Make the connection vulnerable to Man-In-The-Middle attack (not validating the certificate means not validating the server or client identity). I'm working on an certificate parsing function, but it will take some time. If using a full ASN.1 parser, the library binaries will be about twice the size (not OK for chips). It is possible to implement just the "subset" needed for validating a certificate (just like the current implementation).

But, for things like low-value API calls, it should be ok.

I could add a callback for validating the certificate outside TLSe. Something like tlse_set_certificate_parse_function. This is very easy to do, if it helps, I'll add it these days.

[sjaeckel]

Have you thought of basing the cert parser on the existing ltc code?

[eduardsui]

@sjaeckel cool! Didn't know ltc had one :). That is great, thanks for the info! I'll try it this week-end.

[Wertzui123]

@Wertzui123 yes, it is a security issue. Make the connection vulnerable to Man-In-The-Middle attack (not validating the certificate means not validating the server or client identity). I'm working on an certificate parsing function, but it will take some time. If using a full ASN.1 parser, the library binaries will be about twice the size (not OK for chips). It is possible to implement just the "subset" needed for validating a certificate (just like the current implementation).

But, for things like low-value API calls, it should be ok.

I could add a callback for validating the certificate outside TLSe. Something like tlse_set_certificate_parse_function. This is very easy to do, if it helps, I'll add it these days.

Okay, thanks for the detailed answer. A tlse_set_certificate_parse_function surely couldn't hurt, but I don't think I could personally make much use of it. A LTC-based implementation would of course also be awesome, have you already had any success with it?