Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added BR_OPT_NO_RENEGOTIATION flag to forbid TLS renegociation #6165

Merged
merged 1 commit into from
May 30, 2019

Conversation

s-hadinger
Copy link
Contributor

Since PR #6065 x509 memory structure are dropped after succesful handshake. This is ok as long as there is no TLS renegociation, otherwise it will crash.

Adding this flag to avoid any TLS renego, as described here: https://bearssl.org/x509.html

@d-a-v d-a-v requested a review from earlephilhower May 30, 2019 08:58
@@ -827,6 +827,7 @@ extern "C" {
uint16_t suites[cipher_cnt];
memcpy_P(suites, cipher_list, cipher_cnt * sizeof(cipher_list[0]));
br_ssl_client_zero(cc);
br_ssl_engine_add_flags(&cc->eng, BR_OPT_NO_RENEGOTIATION); // forbid SSL renegociation, as we free the Private Key after handshake
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good catch. I'm not quite sure whether it's the right solution or if we should undo the dropping of X509 after connection.

After looking at the RFC I see this could cause two problem in two ways (and dropping X509 would need to be undone):

  1. When more 2^64 messages are sent and the sequence num needs to overflow. This one we can probably ignore.
  2. Servers are free to reject connections with this option sent in the HELLO. Unfortunately I don't know how prevalent this would be.

I'm inclined to say no. 2 happens very infrequently and go with this fix. If we get feedback otherwise we can undo this and the original drop-x509-after-handshake patch.

@earlephilhower earlephilhower merged commit 69311c8 into esp8266:master May 30, 2019
@s-hadinger
Copy link
Contributor Author

I think it's safe to reject renegociation. This is what Bearssl mentions:

As described above, the BR_OPT_NO_RENEGOTIATION flag can be used to prohibit renegotiations. Note that TLS-1.3 bans renegotiations, and when HTTP/2 is used with TLS-1.2, renegotiations are not permitted either except at the very beginning of the connection.

Btw, thanks a lot for all the great work you did on TLS and BearSSL.

This is off-topic, I have been working on a trimmed down version for Tasmota and connection to AWS IoT. The goal was to reduce even more the memory and code footprint.

AWS IoT requires client certificate. I reduced to a single cipher: BR_TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256, which means a lot less code (SH256 only, AES GCM/CTR only, ECDHE/RSA only, P256 EC curve only, max RSA key 2048 and max EC key 256, EC private key). I'm currently consuming only 6.0k, and an additional 6.1k of heap during handshake. I'm not sure how this could be reverted to a more general version.

@s-hadinger s-hadinger deleted the fix/tls_no_renego branch May 30, 2019 15:10
@earlephilhower
Copy link
Collaborator

@s-hadinger, If AWS IoT connections are well defined, we can see about adding a cipher option in the menu (where now it's basic and all) for this. The main work is done, you could add the menu in the boards.py and the defines can be used in WiFiClientSecureBSSL.c to trim down even further.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants