Skip to content

Receiving 'invalid handshake' during establishing SSL connection to my web server #3661

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

Closed
sukretniy opened this issue Sep 29, 2017 · 24 comments

Comments

@sukretniy
Copy link

sukretniy commented Sep 29, 2017

Basic Infos

Hardware

Hardware: ?ESP8266?
Core Version: ?git version?

Description

I'm receiving 'invalid handshake' message during receiving cert from my(www.gsmcounters.com and portal.gsmcounters.com) web servers, but when I try to connect to 'api.github.com', everything goes well.
I've checked supported ciphers on my web server and i have this
SNI: www.gsmcounters.com
TLSv1.0:
server selection: uses client preferences
3-- (key: RSA) RSA_WITH_AES_128_CBC_SHA
3-- (key: RSA) RSA_WITH_AES_256_CBC_SHA
3f- (key: RSA) ECDHE_RSA_WITH_AES_128_CBC_SHA
3f- (key: RSA) ECDHE_RSA_WITH_AES_256_CBC_SHA
TLSv1.1: idem
TLSv1.2:
server selection: uses client preferences
3-- (key: RSA) RSA_WITH_AES_128_CBC_SHA
3-- (key: RSA) RSA_WITH_AES_256_CBC_SHA
3-- (key: RSA) RSA_WITH_AES_128_CBC_SHA256
3-- (key: RSA) RSA_WITH_AES_256_CBC_SHA256
3-- (key: RSA) RSA_WITH_AES_128_GCM_SHA256
3-- (key: RSA) RSA_WITH_AES_256_GCM_SHA384
3f- (key: RSA) ECDHE_RSA_WITH_AES_128_CBC_SHA
3f- (key: RSA) ECDHE_RSA_WITH_AES_256_CBC_SHA
3f- (key: RSA) ECDHE_RSA_WITH_AES_128_CBC_SHA256
3f- (key: RSA) ECDHE_RSA_WITH_AES_256_CBC_SHA384

As I understand ESP8266 supports following ciphers:

TLS_RSA_WITH_AES_128_CBC_SHA256
TLS_RSA_WITH_AES_256_CBC_SHA256
TLS_RSA_WITH_AES_256_CBC_SHA
TLS_RSA_WITH_AES_128_CBC_SHA

What I'm missing?

Settings in IDE

Module: ?Generic ESP8266 Module?
Flash Size: ?1MB?
CPU Frequency: ?80Mhz?
Flash Mode: ?dio?
Flash Frequency: ?40Mhz?
Upload Using: ?SERIAL?
Reset Method: ?ck?

Sketch

#include <time.h>
#include <ESP8266WiFi.h>
#include <WiFiClient.h> 

WiFiClientSecure client;
char* host = "www.gsmcounters.com";
const int httpsPort = 443;

void setup() {  
  testSSL();
}

void testSSL(){
  Serial.begin(115200);
  Serial.setDebugOutput(true);

  Serial.println("Starting SSL testing. Connecting to wifi.");
  WiFi.begin();
  while (WiFi.status() != WL_CONNECTED) {
    delay(500);
    Serial.print(".");
  }  

  Serial.println("");
  Serial.println("WiFi connected");
  Serial.println("IP address: ");
  Serial.println(WiFi.localIP());

  // Synchronize time useing SNTP. This is necessary to verify that
  // the TLS certificates offered by the server are currently valid.
  Serial.println("Setting time using SNTP");
  configTime(8 * 3600, 0, "pool.ntp.org", "time.nist.gov");
  time_t now = time(nullptr);
  while (now < 1000) {
    delay(500);
    Serial.print(".");
    now = time(nullptr);
  }
  Serial.println("");
  struct tm timeinfo;
  gmtime_r(&now, &timeinfo);
  Serial.print("Current time: ");
  Serial.print(asctime(&timeinfo));

  
  host = "www.gsmcounters.com"; 
  
  Serial.print("connecting to ");
  Serial.println(host);
  if (!client.connect(host, httpsPort)) {
    Serial.println("connection failed");    
  } else {
    Serial.println("connection succeed");
  }

  host = "portal.gsmcounters.com";

  Serial.print("connecting to ");
  Serial.println(host);
  if (!client.connect(host, httpsPort)) {
    Serial.println("connection failed");
  } else {
    Serial.println("connection succeed");
  }

  host = "api.github.com";

  Serial.print("connecting to ");
  Serial.println(host);
  if (!client.connect(host, httpsPort)) {
    Serial.println("connection failed");
  } else {
    Serial.println("connection succeed");
  }  
}
void loop() {
    
}

Debug Messages

Starting SSL testing. Connecting to wifi.
scandone
scandone
state: 0 -> 2 (b0)
state: 2 -> 3 (0)
state: 3 -> 5 (10)
add 0
aid 2
cnt

connected with TheKGWNetwork, channel 7
dhcp client start...
wifi evt: 0
.ip:192.168.0.114,mask:255.255.255.0,gw:192.168.0.1
wifi evt: 3
.
WiFi connected
IP address:
192.168.0.114
Setting time using SNTP
please start sntp first !
.
Current time: Sat Sep 30 06:16:52 2017
connecting to www.gsmcounters.com
[hostByName] request IP for: www.gsmcounters.com
[hostByName] Host: www.gsmcounters.com IP: 191.235.177.30
:ref 1
State: sending Client Hello (1)
:wr 106 106 0
:wrc 106 106 0
:sent 106
:rn 1460
:rch 1460, 1219
:rd 5, 2679, 0
:rdi 1460, 5
:rd 2674, 2679, 5
:rdi 1455, 1455
:c 1455, 1460, 2679
:rdi 1219, 1219
:c0 1219, 1219
State: receiving Server Hello (2)
State: receiving Certificate (11)
Error: invalid handshake
:wr 7 7 0
:wrc 7 7 0
Alert: unexpected message
:wr 7 7 0
:wrc 7 7 0
Alert: close notify
connection failed
connecting to portal.gsmcounters.com
[hostByName] request IP for: portal.gsmcounters.com
:er -9 00000000
[hostByName] Host: portal.gsmcounters.com IP: 191.235.177.30
:ur 1
:del
:ref 1
State: sending Client Hello (1)
:wr 109 109 0
:wrc 109 109 0
:sent 109
:rn 1460
:rd 5, 1460, 0
:rdi 1460, 5
:rd 1455, 1460, 5
:rdi 1455, 1455
:c0 1455, 1460
:rn 1240
:rd 1240, 1240, 0
:rdi 1240, 1240
:c0 1240, 1240
State: receiving Server Hello (2)
State: receiving Certificate (11)
Error: invalid handshake
:wr 7 7 0
:wrc 7 7 0
Alert: unexpected message
:wr 7 7 0
:wrc 7 7 0
Alert: close notify
connection failed
connecting to api.github.com
[hostByName] request IP for: api.github.com
:er -9 00000000
[hostByName] Host: api.github.com IP: 192.30.253.117
:ur 1
:del
:ref 1
State: sending Client Hello (1)
:wr 101 101 0
:wrc 101 101 0
:sent 101
:rn 1436
:rch 1436, 1436
:rd 5, 2872, 0
:rdi 1436, 5
:rd 80, 2872, 5
:rdi 1431, 80
State: receiving Server Hello (2)
:rd 5, 2872, 85
:rdi 1351, 5
:rch 2872, 318

:rd 3091, 3190, 90
:rdi 1346, 1346
:c 1346, 1436, 3190
:rdi 1436, 1436
:c 1436, 1436, 1754
:rdi 318, 309
State: receiving Certificate (11)
:rd 5, 318, 309
:rdi 9, 5
:rd 4, 318, 314
:rdi 4, 4
:c0 4, 318
State: receiving Server Hello Done (14)
State: sending Client Key Exchange (16)
:wr 267 267 0
:wrc 256 267 0
:wrc 11 11 0
:wr 6 6 0
:wrc 6 6 0
State: sending Finished (16)
:wr 85 85 0
:wrc 85 85 0
:sent 267
:sent 91
:rn 91
:rd 5, 91, 0
:rdi 91, 5
:rd 1, 91, 5
:rdi 86, 1
:rd 5, 91, 6
:rdi 85, 5
:rd 80, 91, 11
:rdi 80, 80
:c0 80, 91
State: receiving Finished (16)
connection succeed
pm open,type:2 0
:rn 69
:rcl
:abort

messages here
@igrr
Copy link
Member

igrr commented Oct 1, 2017

From a very brief check, it looks like axTLS fails while parsing the server certificate handshake message. Wireshark is able to parse the same message correctly, so it must be a bug on axTLS side.

Further debugging needs to be done in process_certificate function in tls1.c.

@igrr igrr added this to the 2.5.0 milestone Oct 1, 2017
@sukretniy
Copy link
Author

Tried to investigate a bit in Wireshark

The main difference between api.github.com and www.gsmcounters.com, that
api.github.com auto selects Cipher Suite: TLS_RSA_WITH_AES_128_CBC_SHA256 (0x003c) among four supported
and www.gsmcounters.com auto selects Cipher Suite: TLS_RSA_WITH_AES_256_CBC_SHA256 (0x003d)

Is there any way to force axTLS to say to web server that it supports only TLS_RSA_WITH_AES_128_CBC_SHA256 (I want server not to choose among the other three)?

@igrr
Copy link
Member

igrr commented Oct 2, 2017

Did you get past the server certificate message when connecting to gsmcounters with axTLS? I couldn't, and for the record same behavior happens in the Linux port of axTLS.

@sukretniy
Copy link
Author

How can I debug axTLS? Or compile it on windows 10 at least.

@sukretniy
Copy link
Author

sukretniy commented Oct 3, 2017

I managed to setup debug environment (compile axTLS lib). This bug has nothing with my previous assumption. I'll try to investigate deeper to see difference between server handshake messages from my server and api.github.com.

@sukretniy
Copy link
Author

sukretniy commented Oct 3, 2017

Bug in this line

uint8_t *buf = &ssl->bm_data[ssl->dc->bm_proc_index];

It's related to features of handshake protocol:

axTLS sends Client Hello message to server and expects Server Hello message after that Certificate and Server Hello Done messages (api.git.hub behaviour) at this point ssl->dc->bm_proc_index = 0 always.
But instead it receives Multiple Handshake Messages, which contains Server Hello, Certificate and Server Hello Done messages at this point ssl->dc->bm_proc_index=74, that points to the end of previous (Server Hello) message. After that total_cert_len calculated as 11 (because the last byte of previous package=0 and 11 - is type of Certificate message and start of the whole message package)

It works, if that line replace with
uint8_t *buf = &ssl->bm_data[ssl->dc->bm_proc_index > 0 ? ssl->dc->bm_proc_index + 6 : ssl->dc->bm_proc_index];

@jtmoderate876
Copy link

+1

I think I am experiencing this problem but don't (yet) have the skills/experience/setup to try and recompile from:
uint8_t *buf = &ssl->bm_data[ssl->dc->bm_proc_index];

To:
uint8_t *buf = &ssl->bm_data[ssl->dc->bm_proc_index > 0 ? ssl->dc->bm_proc_index + 6 : ssl->dc->bm_proc_index];

@jtmoderate876
Copy link

jtmoderate876 commented Mar 10, 2018

@sukretniy - Any chance that you can fix this in github.com/igrr/axtls-8266.
I think the process is something like:
Fork the above, make changes, commit, and wait for @igrr to accept?

@sukretniy
Copy link
Author

I can send you compiled axtls lib with correction, so you can check if this is your issue.

@jtmoderate876
Copy link

So sorry for delays and I really appreciate your help.
I'm travelling with spotty internet for a a few weeks.
Can you post it somewhere public for a couple weeks or email me or are there other ways here at github (I'm a novice)?

@sukretniy
Copy link
Author

sukretniy commented Mar 25, 2018

You can download from here

@sukretniy sukretniy reopened this Mar 25, 2018
@jtmoderate876
Copy link

@sukretniy you fixed my handshake error - thank you!
Is there something I should do to "vote" to get this into the main code?
Again - thanks.

@earlephilhower
Copy link
Collaborator

@sukretniy Can you share your changed source code? I'm unable to reproduce your success with the change you suggested, but if you can share it I can get it merged into the axtls-8266 repo. Otherwise we'll have to close this as a wont-fix due to it being an axtls bug (and axtls is no longer supported by its authors).

@earlephilhower earlephilhower added the waiting for feedback Waiting on additional info. If it's not received, the issue may be closed. label Aug 4, 2018
@sukretniy
Copy link
Author

Just replacing mentioned earlier line shold fix the issue. If you can not reproduce, it means, that that you faced some other issue or problem with wrong lib location. You need place compiled file libaxtls.a into the right place an after that restart your IDE and recomplie the project.

@earlephilhower
Copy link
Collaborator

Can you give the exact .c diff location, @sukretniy? I've done other committed fixes to axtls and wrote the new BearSSL stuff, so I'm quite sure I've got the file locations right and the change was applied (tried both of the 2 possible locations that match in the code). You can even just attach your modified file to a comment here in github if that's easier.

@devyte
Copy link
Collaborator

devyte commented Sep 6, 2018

I think @earlephilhower means the modified source file, not the prebuilt .a.

@earlephilhower
Copy link
Collaborator

Yes, @devyte. Actually, if @sukretniy could tar or zip up his entire axtls source tree, I can use that for diffs to get the patch. No need for the .a or .os, of course.

@sukretniy
Copy link
Author

sukretniy commented Sep 6, 2018

Here you have axtls lib

@earlephilhower
Copy link
Collaborator

Thanks, @sukretniy ! I'll give it a go this weekend and see about getting your patch into the axTLS code.

@earlephilhower earlephilhower removed the waiting for feedback Waiting on additional info. If it's not received, the issue may be closed. label Sep 9, 2018
@earlephilhower
Copy link
Collaborator

I just got a chance to do the diffs and I see that you've patched an old version of axtls (2.1.2 vs 2.1.4), and that is probably why it didn't work.

The one-line difference I see is here:

diff --git a/ssl/tls1.c b/ssl/tls1.c
index 10b592c..8ced3b7 100644
--- a/ssl/tls1.c
+++ b/ssl/tls1.c
@@ -2123,6 +2123,10 @@ int process_certificate(SSL *ssl, X509_CTX **x509_ctx)
 {
     int ret = SSL_OK;
     uint8_t *buf = &ssl->bm_data[ssl->dc->bm_proc_index];
+    // Adjust for a multi-message fragment
+    if (buf[6] == 11) {
+        buf = &ssl->bm_data[ssl->dc->bm_proc_index+6];
+    }
     int pkt_size = ssl->bm_index;
     int cert_size, offset = 5, offset_start;
     int total_cert_len = (buf[offset]<<8) + buf[offset+1];

With 2.1.4 that change doesn't work, but 2.1.2 it does. The magic constant doesn't match, most likely because we add an extension to the client hello message in 2.1.4 that's not there in 2.1.2, and this logic is hard-coded for the 2.1.2 message format. I'll need to go back and see what the 2.1.2 message sequence looks like to figure out what you're keying off of here to do the adjustment, and see if the same logic can work in 2.1.4.

@earlephilhower
Copy link
Collaborator

earlephilhower commented Sep 9, 2018

It looks like your original code was hardcoding the move to the start of the ASN.1 certificate to workaround a bug where axtls was not incrementing the current byte pointer past the last extension in the server response. The final (only) extension field was 6 bytes in 2.1.2, but now the last extension response is the MFLN extension which is only 1-byte in length.

---edit---The root cause is something in tls1_clnt.c:process_server_hello. The logic looks good after an eyeballing, but I'll throw in some debugs and see what's going on.

Nope, that's actually looking good and MFLN is not being sent, only SNI. The offset, however, for the index is different in 2.1.4 vs 2.1.2, so there's something else fishy in the code. I'm loathe to hardcode a shift if I can't explain where it's coming from. :(

@earlephilhower
Copy link
Collaborator

The root of the problem was in process_server_hello(). The issue is the packet size != the amount of data processed by the functions.

That is, you can legally pad individual messages to 16 or 32 bit boundaries if you want, but axtls was only adding in the actual bytes processed. So the next call (to cert verify) started with several of the padding bytes from the last packet, not the first byte of the new one.

With that, no special magic offsets needed. Will sent in a PR to axtls in a bit...

@earlephilhower
Copy link
Collaborator

See igrr/axtls-8266#58 for the 1-line fix. When @igrr commits he can then rebuild and do a PR into this repo.

@earlephilhower
Copy link
Collaborator

Adding @igrr since we need him to commit the fix in his repo to bring into the Arduino repo.

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

No branches or pull requests

5 participants