MbedTlsError(-30592) after upgrading to commit d5e29f8

[yanshay]

This is an issue as followup to discussion #57, so it's not forgotten.

I have a program that works using esp-mbedtls (async)
I upgraded to the latest commit, it required adding SHA parameter to the Session::new and did the same as the example.
I started receiving MbedTlsError(-30592) which based on error codes I found mean:
0x7780 SSL - A fatal alert message was received from our peer.

Note that I'm using TLS1.2 (that's why I'm using esp-mbedtls).

The issue is probably due to the use of ca_chain: None which wasn't considered in that change.

        let tls_starter = match esp_mbedtls::asynch::Session::new(
            socket,
            "",
            esp_mbedtls::Mode::Client,
            esp_mbedtls::TlsVersion::Tls1_2,
            esp_mbedtls::Certificates {
                ca_chain: None,
                ..Default::default()
            },
            tls_rx_buffer,
            tls_tx_buffer,
            &mut sha
        ) {
            Ok(tls_starter) => tls_starter.with_hardware_rsa(&mut rsa),
            Err(e) => {
                error!(boot, "Error establishing TLS Connection {:?}",e);
                Timer::after(Duration::from_millis(500)).await;
                continue; // to external loop
            }
        };

[yanshay]

Following the guidelines in the discussion that originated this issue I activated logs.
Attached are the logs (good.txt and bad.txt) I managed to extract with those steps. I also had to set ESP_LOGS to trace to see something. good is before the change. bad is after the change.
Are these the logs to expect? There's not much there. The two files are almost identical except at the end.
At line 56 the good sends 6 bytes and the bad 7 bytes. And close after that the bad session ends with an error.

bad.txt
good.txt

[yanshay]

Can anyone assist with this?

@ivmarkov I noticed you referenced this on your PR, maybe you know what could cause this?

[yanshay]

@AnthonyGrondin / @ivmarkov

I made some progress.

I took the commit with the SHA acceleration which doesn't work for my use case, I removed all the SHA functions from the rs files and used the binaries from the previous commit that included their mbedtls versions, and ... it works.

So it can be either that these functions that were replaced are the root cause or something different about the binary in terms of compilation/configuration options or something with the hardware acceleration generating different data ?

Any ideas?

Attached are the logs of the faulty session that fails.
tls-session-bad.txt

I fed the logs to ChatGPT to see what it has to say, this is what it came up with, but I don't know how to continue from here:

Any assistance would be appreciated.

--
The logs provided are extensive, but the key sections point towards issues with the TLS handshake and subsequent integrity verification (likely in the "Finished" message or key setup). Here's what stands out:

Observations

1. Negotiation Details:

   • The client sends a ClientHello offering a wide range of cipher suites, including TLS 1.2 and TLS 1.3 options.
   • The handshake progresses to a point where the "Finished" message is sent and encrypted.

2. Server Response:

   • The server transitions to sending its "Change Cipher Spec" and responds with a fatal alert (code 20, bad_record_mac) during the processing of encrypted handshake data.

---

Likely Causes

1. Cipher Suite Mismatch or Unsupported Algorithm

• Ensure the server supports one of the cipher suites offered by the client. It seems the client offers many, including modern AEAD suites (AES-GCM, Chacha20-Poly1305) and older CBC-based options.

2. Key Exchange Issue

• There might be an issue in how the shared secret is derived from the key exchange mechanism (e.g., ECDHE or PSK).
• Double-check the curve and public parameters if using ECDHE.

3. Encryption or MAC Calculation

• The logs show AEAD algorithms (e.g., AES-GCM, Chacha20-Poly1305) being negotiated. If the MAC is calculated incorrectly or the key/IV is mismatched, the server will reject the message.

4. Data Corruption or Implementation Bugs

• Any corruption in the message during transmission or incorrect handling of handshake message buffers can cause this.

---

Next Steps

1. Enable Detailed Server Logs

• Review server logs to identify if it provides more context about the MAC failure.

2. Reduce Cipher Suites

• Limit the list of cipher suites in the ClientHello to simplify debugging and ensure compatibility with the server. Start with a basic suite like TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256.

3. Verify Handshake Hash and Finished Message

• Ensure the hash of handshake messages and the final "Finished" message matches the expected value.

4. Analyze Network Traffic

• Use Wireshark to capture the TLS handshake and verify that messages are being transmitted correctly and in the expected order.

5. Configuration Check

• Confirm that mbedtls is configured correctly, including:
  • Cipher suite availability
  • Key exchange and signature algorithm settings
  • TLS version compatibility (confirm server accepts TLS 1.2/1.3).

---

If you can narrow down the logs to a specific area (or share server-side logs), further diagnosis can be provided!

[ivmarkov]

I think the most likely root cause is that the HW accel functions have a bug.
Fortunately, testing the HW sha-* functions is easy, as computing a sha hash over a given input should yield the same result each time for the same input. So what you could do is to compare the sha digests generated with hw accel with sha digests generated without hw accel.

Here's a short action plan:

• Disable the RSA accel (do not call .with_hardware_rsa) to make sure RSA is not the culprit
• Create an example, crypto_hw_accel_test.rs by copy-pasting - say - crypto_self_test.rs
• In the example, copy-paste the hash-computation functions for sha1, sha256 (start with that as it is the most popular), sha224, sha384 and sha512. I mean, copy those without the mbedtls surroundings. For example, for sha1, you need to copy:

#[no_mangle]
pub unsafe extern "C" fn mbedtls_sha1_starts(_ctx: *mut mbedtls_sha1_context) -> c_int {
    0
}

#[no_mangle]
pub unsafe extern "C" fn mbedtls_sha1_update(
    ctx: *mut mbedtls_sha1_context,
    input: *const c_uchar,
    ilen: usize,
) -> c_int {
    let mut data = core::ptr::slice_from_raw_parts(input as *const u8, ilen as usize);
    critical_section::with(|cs| {
        // ===========> Copy roughly from here below
        let mut sha = SHARED_SHA.borrow_ref_mut(cs);
        let mut hasher = ShaDigest::restore(sha.as_mut().unwrap(), (*ctx).hasher.as_mut().unwrap());
        while !data.is_empty() {
            data = nb::block!(hasher.update(&*data)).unwrap();
        }
        nb::block!(hasher.save((*ctx).hasher.as_mut().unwrap())).unwrap();
        // <=========== Until here
    });
    0
}

#[no_mangle]
pub unsafe extern "C" fn mbedtls_sha1_finish(
    ctx: *mut mbedtls_sha1_context,
    output: *mut c_uchar,
) -> c_int {
    let mut data: [u8; 20] = [0u8; 20];
    critical_section::with(|cs| {
        // ===========> Copy roughly from here below
        let mut sha = SHARED_SHA.borrow_ref_mut(cs);
        let mut hasher = ShaDigest::restore(sha.as_mut().unwrap(), (*ctx).hasher.as_mut().unwrap());
        nb::block!(hasher.finish(&mut data)).unwrap();
        nb::block!(hasher.save((*ctx).hasher.as_mut().unwrap())).unwrap();
        // <=========== Until here
    });
    core::ptr::copy_nonoverlapping(data.as_ptr(), output, data.len());
    0
}

• Once you extract the HW-accelerated sha computations, implement their alternatives in pure Rust using the sha* crates from Rust-Crypto (the sha-1 and sha-2 crates)
• Implement code that for a given output, compares the HW result with the software computation

=> If you get differences, we have a problem, and we need to figure out where in the HW computation we are making a mistake
=> If you don't get any differences, then the problem is in the mbedtls surrounding code most likely, which still means we made progress

... and it is good to have such a test anyway.

[ivmarkov]

NOTE: You could also even temporarily replace/comment-out the HW computation with the algorithms from the sha1/sha2 crates to see if it works with them.

[yanshay]

Disable the RSA accel (do not call .with_hardware_rsa) to make sure RSA is not the culprit

Disabled, still doesn't work.

Create an example, crypto_hw_accel_test.rs by copy-pasting - say - crypto_self_test.rs
In the example, copy-paste the hash-computation functions for sha1, sha256 (start with that as it is the most popular), sha224, sha384 and sha512. I mean, copy those without the mbedtls surroundings. For example, for sha1, you need to copy:

I'll try to follow your action plan and see if I understand your guidelines, I've never been into this crypto coding territory before.

What I can say is that in the logs it looks like sha384 was chosen.

[yanshay]

What I can say is that in the logs it looks like sha384 was chosen.

Which sounds strange to me actually. It's a pretty primitive device, supporting only TLS 1.2, security there isn't that critical there really, so why would they choose sha384?

Maybe there's some misunderstanding during the negotiation phase and one side thinks it's sha384 and the other sha256?

That's why I wanted to see debug logs of the session that work but I can't figure how to build the mbedtls with the sha functions in debug mode.

[ivmarkov]

Btw, you don't have to do crypto-coding, just call existing algos.

[yanshay]

I've followed your general action plan but differently and solved it !!!

What I did is as follows:

1. I used the original functions in the SHA accelerated revision, to do that I needed to make a few things public for testing but eventually got it to work. Code I used is below. (I did it mostly because ctx was required to get the code working so was easier to use the functions as is)
2. I found that in the working revision, mbedtls already has simple functions to do the SHA so I used the.

The hash for sha1, 224, 256, 384, 512 seemed the same initially, but I (accidentally) passed the large buffer to 224/256 and 284/512 since it's the same function for each pair.
I noticed that in the mbedtls version the extra bytes left zeroed while the sha accelerated was filled till the end (the beginning was the same between outputs). see below.

So I checked the code and noticed that in each function, in both cases the large internal data size is copied instead of smaller one in cases of 224 and 384. Once I changed that to 48 bytes in the case of sha384 it worked.

So that's what needs to be fixed for both functions.

---

Here are the hashes for abcde:

SHA acceleration:

sha1=[3, de, 6c, 57, b, fe, 24, bf, c3, 28, cc, d7, ca, 46, b7, 6e, ad, af, 43, 34]
sha224=[bd, d0, 3d, 56, 9, 93, e6, 75, 51, 6b, a5, a5, 6, 38, b6, 53, 1a, c2, ac, 3d, 58, 47, c6, 19, 16, cf, ce, d6, a, a1, 3d, f6]
sha256=[36, bb, e5, e, d9, 68, 41, d1, 4, 43, bc, b6, 70, d6, 55, 4f, a, 34, b7, 61, be, 67, ec, 9c, 4a, 8a, d2, c0, c4, 4c, a4, 2c]
sha384=[4c, 52, 5c, be, ac, 72, 9e, af, 4b, 46, 65, 81, 5b, c5, db, c, 84, fe, 63, 0, 6, 8a, 72, 7c, f7, 4e, 28, 13, 52, 15, 65, ab, c0, ec, 57, a3, 7e, e4, d8, be, 89, d0, 97, c0, d2, ad, 52, f0, e6, 30, e0, b1, 42, df, 4b, 42, 53, 49, d0, 7a, e4, 17, a0, ae]
sha512=[87, 8a, e6, 5a, 92, e8, 6c, ac, 1, 1a, 57, d, 4c, 30, a7, ea, ec, 44, 2b, 85, ce, 8e, ca, c, 29, 52, b5, e3, cc, 6, 28, c2, e7, 9d, 88, 9a, d4, d5, c7, c6, 26, 98, 6d, 45, 2d, d8, 63, 74, b6, ff, aa, 7c, d8, b6, 76, 65, be, f2, 28, 9a, 5c, 70, b0, a1]

mbedtls:

sha1=[3, de, 6c, 57, b, fe, 24, bf, c3, 28, cc, d7, ca, 46, b7, 6e, ad, af, 43, 34]
sha224=[bd, d0, 3d, 56, 9, 93, e6, 75, 51, 6b, a5, a5, 6, 38, b6, 53, 1a, c2, ac, 3d, 58, 47, c6, 19, 16, cf, ce, d6, 0, 0, 0, 0]
sha256=[36, bb, e5, e, d9, 68, 41, d1, 4, 43, bc, b6, 70, d6, 55, 4f, a, 34, b7, 61, be, 67, ec, 9c, 4a, 8a, d2, c0, c4, 4c, a4, 2c]
sha384=[4c, 52, 5c, be, ac, 72, 9e, af, 4b, 46, 65, 81, 5b, c5, db, c, 84, fe, 63, 0, 6, 8a, 72, 7c, f7, 4e, 28, 13, 52, 15, 65, ab, c0, ec, 57, a3, 7e, e4, d8, be, 89, d0, 97, c0, d2, ad, 52, f0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0]
sha512=[87, 8a, e6, 5a, 92, e8, 6c, ac, 1, 1a, 57, d, 4c, 30, a7, ea, ec, 44, 2b, 85, ce, 8e, ca, c, 29, 52, b5, e3, cc, 6, 28, c2, e7, 9d, 88, 9a, d4, d5, c7, c6, 26, 98, 6d, 45, 2d, d8, 63, 74, b6, ff, aa, 7c, d8, b6, 76, 65, be, f2, 28, 9a, 5c, 70, b0, a1]

[AnthonyGrondin]

Amazing! Could you try the following and see if this fixes the issue for hardware acceleration:

diff --git a/esp-mbedtls/src/esp_hal/sha/sha256.rs b/esp-mbedtls/src/esp_hal/sha/sha256.rs
index 023641e..797a8f7 100644
--- a/esp-mbedtls/src/esp_hal/sha/sha256.rs
+++ b/esp-mbedtls/src/esp_hal/sha/sha256.rs
@@ -114,6 +114,7 @@ pub unsafe extern "C" fn mbedtls_sha256_finish(
             );
             nb::block!(hasher.finish(&mut data)).unwrap();
             nb::block!(hasher.save((*ctx).sha224_hasher.as_mut().unwrap())).unwrap();
+            core::ptr::copy_nonoverlapping(data.as_ptr(), output, 28);
         } else {
             let mut hasher = ShaDigest::restore(
                 sha.as_mut().unwrap(),
@@ -121,8 +122,8 @@ pub unsafe extern "C" fn mbedtls_sha256_finish(
             );
             nb::block!(hasher.finish(&mut data)).unwrap();
             nb::block!(hasher.save((*ctx).sha256_hasher.as_mut().unwrap())).unwrap();
+            core::ptr::copy_nonoverlapping(data.as_ptr(), output, 32);
         }
     });
-    core::ptr::copy_nonoverlapping(data.as_ptr(), output, data.len());
     0
 }
diff --git a/esp-mbedtls/src/esp_hal/sha/sha512.rs b/esp-mbedtls/src/esp_hal/sha/sha512.rs
index e1cc4c9..dd270ab 100644
--- a/esp-mbedtls/src/esp_hal/sha/sha512.rs
+++ b/esp-mbedtls/src/esp_hal/sha/sha512.rs
@@ -114,6 +114,7 @@ pub unsafe extern "C" fn mbedtls_sha512_finish(
             );
             nb::block!(hasher.finish(&mut data)).unwrap();
             nb::block!(hasher.save((*ctx).sha384_hasher.as_mut().unwrap())).unwrap();
+            core::ptr::copy_nonoverlapping(data.as_ptr(), output, 48);
         } else {
             let mut hasher = ShaDigest::restore(
                 sha.as_mut().unwrap(),
@@ -121,8 +122,8 @@ pub unsafe extern "C" fn mbedtls_sha512_finish(
             );
             nb::block!(hasher.finish(&mut data)).unwrap();
             nb::block!(hasher.save((*ctx).sha512_hasher.as_mut().unwrap())).unwrap();
+            core::ptr::copy_nonoverlapping(data.as_ptr(), output, 64);
         }
     });
-    core::ptr::copy_nonoverlapping(data.as_ptr(), output, data.len());
     0
 }

[yanshay]

These were the exact changes I've made, I tested only the SHA384 since it's what I have but the rest looks good as well.

BTW: from looking at the code, specifically 👍

pub mod sha1;
#[cfg(any(feature = "esp32s2", feature = "esp32s3"))]
pub mod sha256;
#[cfg(any(feature = "esp32s2", feature = "esp32s3"))]
pub mod sha512;

It looks like the HW SHA is supported only on esp32s2 and esp32s3, so now esp-mbedtls won't work any longer on esp32 for example and all other chips?
Or are there plans to support those in other ways?
Maybe it's possible to keep the mbedtls functions and find some solution for the linking to have the rust functions linked and call the original mbedtls functions with the same name? Or maybe modify mbedtls to include access to these functions under a different name as backup for when hw sha is not available?

Also, it seemed to me like the way the code works is that espmbedtls initializes some memory area for the sha context, but it is based on its own internal structures for sha contexts, and the assumption is that there's enough memory there for the rust structures so the sha_init functions initializes this memory as if it was the rust context, is that indeed the case?
So if in the future, either the mbedtls structures become smaller or the rust context become larger it may fail (and difficult to trace the root cause)?

[ivmarkov]

These were the exact changes I've made, I tested only the SHA384 since it's what I have but the rest looks good as well.

I do not understand. Are you saying, that with the changes suggested by Anthony, it does work for you now, with HW accel, or not yet?

BTW: from looking at the code, specifically 👍

pub mod sha1;
#[cfg(any(feature = "esp32s2", feature = "esp32s3"))]
pub mod sha256;
#[cfg(any(feature = "esp32s2", feature = "esp32s3"))]
pub mod sha512;

It looks like the HW SHA is supported only on esp32s2 and esp32s3, so now esp-mbedtls won't work any longer on esp32 for example and all other chips? Or are there plans to support those in other ways? Maybe it's possible to keep the mbedtls functions and find some solution for the linking to have the rust functions linked and call the original mbedtls functions with the same name? Or maybe modify mbedtls to include access to these functions under a different name as backup for when hw sha is not available?

For esp32 and the others where sha256 and sha512 HW accel is missing, the library is compiled with the software mbedtls fallbacks for these algorithms, so it should all work (albeit slowly) on these chips.

Also, it seemed to me like the way the code works is that espmbedtls initializes some memory area for the sha context, but it is based on its own internal structures for sha contexts, and the assumption is that there's enough memory there for the rust structures so the sha_init functions initializes this memory as if it was the rust context, is that indeed the case? So if in the future, either the mbedtls structures become smaller or the rust context become larger it may fail (and difficult to trace the root cause)?

My understanding is that there is no problem here. Mbedtls just tells you "initialize your internal buffers" and then the HW accel does that (with its own internal structures of course). Then mbedtls only carries around these internal HW accel structures, and passes them back to the HW accel when hashing. When the hashing is complete, mbedtls passes again these structures to the HW accel code, so that they can be released. But it does not assume anything about the layout and size of these structures at any point in time.

[ivmarkov]

Ah ok you have updated your original comment and I did not notice that...