ARMmbed / wifi-ism43362

ISM43362 WiFi driver
17 stars 22 forks source link

padding 0x15 is sometimes in read response #89

Closed jarvte closed 3 years ago

jarvte commented 3 years ago

I logged what's happening and I saw that there is sometimes burst of padding '0x15' in the beginning when read from the driver. I tested to remove leading padding in here https://github.com/ARMmbed/wifi-ism43362/blob/master/ISM43362/ISM43362.cpp#L819 and shift buffer content. It seemed to help a bit but then invalid records started to come again from tls stack. It might be that some real data was removed.

   // looks like there is also padding at the start, clean up those also
    while ((read_amount > 0) && (cleanup[0] == 0x15)) {
        //debug_if(_ism_debug, "\tISM43362 check_recv_status: leading spurious 0X15 trashed\r\n");
        /* Remove the leading char then search again */
        read_amount--;
        // shift data by one to remove '0x15' from the start
        memmove(cleanup, cleanup + 1 , read_amount);
    }

Also noticed that mutex unlock was missing in atparser from two places https://github.com/ARMmbed/wifi-ism43362/blob/master/ISM43362/ATParser/ATParser.cpp#L110 https://github.com/ARMmbed/wifi-ism43362/blob/master/ISM43362/ATParser/ATParser.cpp#L271

teetak01 commented 3 years ago

@jeromecoutant @JeanMarcR could you please take a look at this?

JeanMarcR commented 3 years ago

@jarvte Hi Please, could you give me the AT commands you send to the modem before and after receiving \015.

jarvte commented 3 years ago

Hi, @JeanMarcR

From attached log is you search 'MBEDTLS_ERR_SSL_INVALID_RECORD' then the leading padding is just above that and that's the reason why tls fails. This does not happen every time but quite often.

leading_padding.txt

JeanMarcR commented 3 years ago

@jarvte If I well understand you use TCP-SSL and RANDOMLY an error (MBEDTLS_ERR_SSL_INVALID_RECORD) occurs. It seems also that after receiving the error the connection is not interrupted. Data continue to be exchanged. The root cause seems to be an invalid ssl version: mbedtls_ssl_read_version, major: 21, minor: 21 (attended major: 3, minor: 3)

I do not know SSL but sometimes TCP is able to recover from an error by retransmitting data. Maybe you could check that.

jarvte commented 3 years ago

Yes, in this case issue was invalid ssl version (because of extra 0x15) as 0x15 == 21 decimal. We really should not get those extra 0x15 paddings, they should be filtered away in driver level. This might corrupt any stage in ssl handshake in UDP/TCP or invalid response data. IoT devices most likely uses UDP so retransmission is not the answer.

JeanMarcR commented 3 years ago

If I well understand, at client site (STM32) the software layer TLS (Session layer) receives erroneous data from the software layer TCP (transport layer) (see the joined file). That leads to a problem(extra 0x15) with ssl handshake. "We really should not get those extra 0x15 paddings, they should be filtered away in driver level" Yes. I think the filter belongs to the software layer TLS at the client site.

Do you agree ? TLS.docx

teetak01 commented 3 years ago

TLS is platform agnostic, and so it the client-code using it. I do not see why either should have to consider driver-specific implementation.

The driver has already identified that this kind of issues so to me it would make sense to cover also the remaining areas where it can return extra paddings.

https://github.com/ARMmbed/wifi-ism43362/blob/master/ISM43362/ISM43362.cpp#L819

The DISCO-L475VG-IOT01A is one of the boards we promote in our applications at Pelion and it would be nice to stabilize those remaining driver issues. Our stack can recover from the issues, but the connection is not very stable.

JeanMarcR commented 3 years ago

Please, could you tell me how I can reproduce your problem:

I think the board is DISCO-L475VG-IOT01A. Best regards

teetak01 commented 3 years ago

Hi @JeanMarcR

We reproduced it with pelion-client-lite-example. There is tutorial available here.

Target was DISCO-L475VG-IOT01A.

Using https://portal.mbedcloud.com/login for Pelion cloud-connectivity. Free-tier accounts are available, for. example with Mbed Account.

JeanMarcR commented 3 years ago

@jarvte Please, could you check if the problem is still present with the following improved files:

JeanMarcR commented 3 years ago

Sorry. The files are joined. ISM43362Interface.cpp.GZ ATParser.cpp.GZ Before using the files, do not forget to remove the suffix GZ.

teetak01 commented 3 years ago

@JeanMarcR maybe you could open a PR with the proposed fixes so its easier to verify it/comment on it.

JeanMarcR commented 3 years ago

@teetak01 https://github.com/ARMmbed/wifi-ism43362/pull/90

JeanMarcR commented 3 years ago

Hi, @jarvte @teetak01

Concerning the error 'MBEDTLS_ERR_SSL_INVALID_RECORD' described in your log file :leading_padding.txt, a possible cause could be an overflow of the receiver buffer of mbedTLS. I suggest you to put define MBEDTLS_SSL_MAX_CONTENT_LEN -> 6000 for access to PELION.

In file pelion-client-lite-example\mbedtls_mbed_client_config.h #define MBEDTLS_SSL_MAX_CONTENT_LEN 6000

Let me know if the problem is resolved ?

Best regards.

jeromecoutant commented 3 years ago

93 merged

Waiting for feedback about mbedtls configuration before closing this ticket Thx