Cannot use TLSSocket without enabling MBEDTLS_SHA1_C macro

[janjongboom]

Description

You cannot use TLSSocket in Mbed OS master without declaring MBEDTLS_SHA1_C=1 in your macros section in mbed_app.json. This is not good, everything should work out of the box. We should add it to the default TLS config, or add it to mbed_lib.json for TLSSocket (like it is in the standalone library).

Issue request type

[ ] Question
[ ] Enhancement
[X] Bug

@bulislaw @SenRamakri @sg-

[ciarmcom]

Internal Jira reference: https://jira.arm.com/browse/MBOCUSTRIA-137

[kjbracey]

How much does MBEDTLS_SHA1_C affect code size for people using the crypto but not full TLS? It adds it to the cipher list, but when is that typically pulled in apart from TLS? HMAC wrappers?

[marcuschangarm]

I think this depends on the root certificate you provide TLSSocket. I'm able to connect to AWS S3 using TLSSocket out of the box and a SHA256 signed certificate.

@janjongboom can you post the certificate you are trying to use?

[janjongboom]

@marcuschangarm os.mbed.com

See the code example in https://os.mbed.com/blog/entry/Adding-TLS-Sockets-to-Mbed-OS/

[SeppoTakalo]

LetsEncrypt certificate work without this MBEDTLS_SHA1_C so this issue is not valid or should be directed to Mbed TLS team.

COMODO certificate seems to require this SHA1.

[SeppoTakalo]

Tried a build without the macro and using Mbed OS Socket example modified to TLS

Total Static RAM memory (data + bss): 64264(+0) bytes
Total Flash memory (text + data): 294886(+0) bytes

With MBEDTLS_SHA1_C macro enabled:

Total Static RAM memory (data + bss): 64264(+0) bytes
Total Flash memory (text + data): 300271(+5385) bytes

So the flash size is affected by 5kB.

This is now up to TLS team to decide whether to enable that macro by default or not.
Mbed OS is not touching the default TLS configuration.

[janjongboom]

@SeppoTakalo @marcuschangarm Any idea on how we could print an error message to tell the user to enable this macro when we encounter this? If we could do that that'd be good by me.

[SeppoTakalo]

/**
 * \def MBEDTLS_SHA1_C
 *
 * Enable the SHA1 cryptographic hash algorithm.
 *
 * Module:  library/sha1.c
 * Caller:  library/md.c
 *          library/ssl_cli.c
 *          library/ssl_srv.c
 *          library/ssl_tls.c
 *          library/x509write_crt.c
 *
 * This module is required for SSL/TLS up to version 1.1, for TLS 1.2
 * depending on the handshake parameters, and for SHA1-signed certificates.
 *
 * \warning   SHA-1 is considered a weak message digest and its use constitutes
 *            a security risk. If possible, we recommend avoiding dependencies
 *            on it, and considering stronger message digests instead.
 *
 */
//#define MBEDTLS_SHA1_C

So it seems that SHA1 is required for old and weak certificates.

And again, I should not make any statements on how Mbed TLS is built, or should build.
It is their judgement call on what ciphers and configs to support.

CC: @ARMmbed/mbed-tls-gatekeepers can you comment here on how to handle this. Should we, or should Mbed TLS print out warning when building it with secure config. Feels a bit wrong...

As what comes to runtime error code, turning on the traces would print out this:

[ERR ][TLSW]: mbedtls_x509_crt_parse() failed: -0x262e (-9774): X509 - Signature algorithm (oid) is unsupported : OID - OID is not found

That is pretty much all we can get from TLS library.

[gilles-peskine-arm]

Hello,

I haven't investigated the issue(s) you're facing, but just to comment on MBEDTLS_SHA1_C, I want to emphasize that it's mainly needed for two things. It's needed to support older, weak certificates that are signed with SHA-1, which requires a further run-time option to enable because it's a security risk. And, independently, it's also needed to support TLS protocol versions 1.0 and 1.1.

The trace from mbedtls_x509_crt_parse shows that a certificate is using an unsupported algorithm. Just to confirm, are you getting a positive result with the same certificate if you compile with MBEDTLS_SHA1_C and no other changes? This surprises me because I'd have expected that the x509 module would then recognize SHA-1 but reject it as insecure.

[SeppoTakalo]

Yes, the same certificate works if SHA1 is enabled.

This was the error log:

[ERR ][TLSW]: mbedtls_x509_crt_parse() failed: -0x262e (-9774): X509 - Signature algorithm (oid) is unsupported : OID - OID is not found
[INFO][TLSW]: mbedtls_ssl_conf_rng()
[INFO][TLSW]: mbedtls_ssl_config_defaults()
[INFO][TLSW]: mbedtls_ssl_conf_authmode()
[INFO][TLSW]: mbedtls_ssl_setup()
[INFO][TLSW]: Starting TLS handshake with api.ipify.org
[ERR ][TLSW]: mbedtls_ssl_handshake() failed: -0x7680 (-30336): SSL - No CA Chain is set, but required to operate
Error! socket->connect() returned: -30336
[INFO][TLSW]: Closing TLS

This is when SHA1 is enabled:

Connecting to network
[INFO][TLSW]: mbedtls_ssl_conf_ca_chain()
[INFO][TLSW]: mbedtls_ssl_config_defaults()
[INFO][TLSW]: mbedtls_ssl_conf_authmode()
Connecting to api.ipify.org
[INFO][TLSW]: mbedtls_ssl_conf_rng()
[INFO][TLSW]: mbedtls_ssl_setup()
[INFO][TLSW]: Starting TLS handshake with api.ipify.org
[INFO][TLSW]: TLS connection to api.ipify.org established
[DBG ][TLSW]: Server certificate:
    cert. version     : 3
    serial number     : 92:0F:D1:B7:FE:4B:88:AE:B6:ED:5A:B0:C3:6C:56:68
    issuer name       : C=GB, ST=Greater Manchester, L=Salford, O=COMODO CA Limited, CN=COMODO RSA Domain Validation Secure Server CA
    subject name      : OU=Domain Control Validated, OU=PositiveSSL Wildcard, CN=*.ipify.org
    issued  on        : 2018-01-24 00:00:00
    expires on        : 2021-01-23 23:59:59
    signed using      : RSA with SHA-256
    RSA key size      : 2048 bits
    basic constraints : CA=false
    subject alt name  : *.ipify.org, ipify.org
    key usage         : Digital Signature, Key Encipherment
    ext key usage     : TLS Web Server Authentication, TLS Web Client Authentication


[INFO][TLSW]: Certificate verification passed
[DBG ][TLSW]: send 58
SRV: HTTP/1.1 200 OK

Log outputs are coming from the wrapper layer, not from the Mbed TLS library. Is there some way to enable some loggings from TLS library as well? Should I re-run the same app with those enabled?

edit: sorry, copy&pasted wrong logs.. but anyway those were similar.

[SeppoTakalo]

Both say that signed using : RSA with SHA-256

[Patater]

Since this is PKI, there is probably a certificate chain involved. All certificates in the chain would need to avoid SHA-1. Probably the root certificate for Let's Encrypt is (IdenTrust) DST Root CA X3 which is signed using PKCS #1 SHA-1 With RSA Encryption. (See https://letsencrypt.org/certificates/ for more details.)

[gilles-peskine-arm]

In Mbed TLS, you can get logs by compiling it with MBEDTLS_DEBUG_C enabled and using code like the following (replace fprintf and fflush by whatever works for you):

static void my_debug( void *ctx, int level,
                      const char *file, int line,
                      const char *str )
{
    ((void) level);
    fprintf( (FILE *) ctx, "%s:%04d: %s", file, line, str );
    fflush(  (FILE *) ctx  );
}
...
mbedtls_ssl_config ssl_conf;
...
mbedtls_ssl_conf_dbg(&ssl_conf, my_debug, stdout);
mbedtls_debug_set_threshold(3);

I don't know if the Mbed OS socket object has already done some of these steps for you.

[gilles-peskine-arm]

Ok, I checked https://api.ipify.org/, their certificate chain uses SHA-2 all the way to the root, but the root itself is self-signed with SHA-1. This is not a security problem since the signature of the root doesn't matter, but Mbed TLS still wants to verify that the signature is correct, and it needs SHA-1 for that.

It's unnecessary for security to calculate a SHA-1 value in this scenario, but I don't know if there's a way to make Mbed TLS understand this.

[simonbutcher]

Based on @gilles-peskine-arm's analysis of the certificate chain, this is effectively by design because your root certificate is using SHA-1, therefore you need SHA-1. We may be able to remove it's use on the root certificate, but to me that sounds like an optimisation, not a bug. Or to put it another way, this is a limitation of the design.

I haven't studied this issue beyond reading the thread, but the simple answer to @janjongboom is, if you don't want to build in SHA-1 into your image, make sure your entire certificate chain does not make use of SHA-1.

I do not see this as a bug - and to follow what @gilles-peskine-arm says, is really an enhancement or feature request to the original design.

As an aside - we're looking into completely redesigning the way configurations are made on Mbed TLS anyway.

cc: @RonEld, @ndevillard

[kjbracey]

if you don't want to build in SHA-1 into your image, make sure your entire certificate chain does not make use of SHA-1.

In this case, it is a simple test app using TLSSocket with default mbed OS config against os.mbed.com. So it's an out-of-the-box problem.

If not going to make the optimisation in mbed TLS, and os.mbed.com isn't particularly unusual in using this root CA, there may be an argument for enabling SHA-1 by default.

If we're going to give people an "easy-to-use" TLSSocket, it follows we should upgrade the default config to cover whatever it's likely to hit anywhere on the Internet. Current default config is largely based on what we need for mbed cloud connection specifically.

[simonbutcher]

This is probably a terribly naive question, but why can't we fix the cert chain? Why this CA? Why this root?

[janjongboom]

@sbutcher-arm But we don't control which CAs people use, and:

[ERR ][TLSW]: mbedtls_x509_crt_parse() failed: -0x262e (-9774): X509 - Signature algorithm (oid) is unsupported : OID - OID is not found
[INFO][TLSW]: mbedtls_ssl_conf_rng()
[INFO][TLSW]: mbedtls_ssl_config_defaults()
[INFO][TLSW]: mbedtls_ssl_conf_authmode()
[INFO][TLSW]: mbedtls_ssl_setup()
[INFO][TLSW]: Starting TLS handshake with api.ipify.org
[ERR ][TLSW]: mbedtls_ssl_handshake() failed: -0x7680 (-30336): SSL - No CA Chain is set, but required to operate
Error! socket->connect() returned: -30336

Is an unhelpful message with no indication that you need to enable something in your TLS config.

And further I second @kjbracey-arm's message. If we're adding an API like this it should 'just work'. Like everything else in Mbed OS. Or provide very clear guidance if people need to enable certain ciphers.

[hanno-becker]

The problem seems to be that the X.509 OID for SHA-1 is defined only if MBEDTLS_SHA1_C is defined, leading to CRT parsing failure as seen in the log. As far as I see, neither CRT parsing nor the CRT chain verification would actually need to calculate the hash of the self-signed trusted CRT, so there shouldn't be a need for MBEDTLS_SHA1_C.

I think that the OIDs should be known to the library even if the primitives themselves are not included. This would be in line with other parts of the library, e.g. all hash identifiers being defined in md.h even if the respective modules are not enabled.

This comes at a slight additional cost of ROM, but importantly doesn't require setting MBEDTLS_SHA1_C in the config. I removed the respective compile-time guards from oid.c and could successfully connect to os.mbed.com through Mbed TLS' programs/ssl/ssl_client2 with MBEDTLS_SHA1_C disabled.

[kjbracey]

Fishing for alternatives to increasing fixed table overhead.

Why does the signature algorithm OID need to be recognised anyway if you're not going to process the signature? Could the OID lookup be deferred until the point you know you're checking it?

Or, if it's parse first, process later, how about having a dummy "unknown" signature primitive that always fails operations? That could be assigned by the parser to defer error generation until the signature is used.

[hanno-becker]

@kjbracey-arm It should be possible to store the unparsed signature OID and defer parsing it to when the signature needs to be verified. However, it might be that the logic for this postponed parsing is larger in code than the OID table entries we're trying to eliminate - but this would need more careful evaluation.

[hanno-becker]

Or, if it's parse first, process later, how about having a dummy "unknown" signature primitive that always fails operations? That could be assigned by the parser to defer error generation until the signature is used.

Yes, that could indeed be a cheaper alternative, with only a small code overhead.

[simonbutcher]

If we're adding an API like this it should 'just work'.

@janjongboom - Then Add SHA-1 back into the configuration for sample. Makes it bigger, but it will also make it work.

[mpg]

@SeppoTakalo

[ERR ][TLSW]: mbedtls_x509_crt_parse() failed: -0x262e (-9774): X509 - Signature algorithm (oid) is unsupported : OID - OID is not found
[INFO][TLSW]: mbedtls_ssl_conf_rng()
[INFO][TLSW]: mbedtls_ssl_config_defaults()
[INFO][TLSW]: mbedtls_ssl_conf_authmode()
[INFO][TLSW]: mbedtls_ssl_setup()
[INFO][TLSW]: Starting TLS handshake with api.ipify.org
[ERR ][TLSW]: mbedtls_ssl_handshake() failed: -0x7680 (-30336): SSL - No CA Chain is set, but required to operate
Error! socket->connect() returned: -30336

I'm sorry, my comment is quite unrelated to the main issue here, but this log feels quite wrong to me: why doesn't it stop at the first error? As a matter of principle you should never attempt a handshake if anything went wrong while setting it up. This is bad security hygiene and could have very serious consequences if other errors are ignored. (For example, continuing after errors in your RNG setup voids any kind of security warranty.)

To all Mbed TLS users: please always abort on the first error and do not attempt to initiate or continue a handshake after an error occurred.

[kjbracey]

This is "user error" - it's the example application from ARMmbed/mbed-os-examples-docs_only#11 that fails to check the return code from TLSSocket::set_root_ca_cert().

Not sure we need to take any further action in TLSSocketWrapper - if there has been no successful call to set the CA chain, then we rely on mbed TLS itself complaining. The wrapper can't be expected to have all its own matching precondition checks and mirror of mbed TLS's internal state.

But conceivably you could have a "save users from themselves" lock which basically "killed" the wrapper permanently whenever any mbed TLS (or transport?) error was spotted. I wouldn't be opposed to that, as long as there was a way to suppress the logic for the rare cases where you are expecting a potential error.

[mpg]

@kjbracey-arm Thanks for the info! I agree the problem lies in the example application. Can you make sure it's fixed in the PR before it's merged? We want our examples to show good practices.

In the longer run, I agree it could be a good idea to have a "save users from themselves" lock in the wrapper as you say, but you guys can think about it later.

[ciarmcom]

We closed this issue because it has been inactive for quite some time and we believe it to be low priority. If you think that the priority should be higher, then please reopen with your justification for increasing the priority.