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...

[yanshay]

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

Yes, many times ... made so many mistakes along the way.

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.

This binary usage raised another point I had in mind (maybe worth a separate discussion since I'm hijacking a thread again 😄 ), the crate depends on binaries as the source (at least the sha version which is the latest I was able to use).
Especially given this is a security component, I don't think such approach would pass a security audit. Getting a crate with binaries that were compiled by a practically unknown source on an unknown machine (that could be even unknwingly compromised) will be an issue. That's an easy supply chain atta

I think the correct approach should be (not sure if possible) to have every build (also on developer machine) compile mbedtls from source. But given I wasn't able to compile it myself I'm not sure if the toolset on all systems can support this.
Or have these files only built on Github build and prevent from being added through a commit so it is possible to review how they were generated. Again, not sure if doable.

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.

Here is how I understand it, could be I'm missing something:

If I get it right, the rust side of initialization starts in the shaXXX_init function as below.

#[no_mangle]
pub unsafe extern "C" fn mbedtls_sha512_init(ctx: *mut mbedtls_sha512_context) {
    let sha384_mem =
        crate::calloc(1, core::mem::size_of::<Context<Sha384>>() as u32) as *mut Context<Sha384>;
    let sha512_mem =
        crate::calloc(1, core::mem::size_of::<Context<Sha512>>() as u32) as *mut Context<Sha512>;
    (*ctx).sha384_hasher = sha384_mem;
    (*ctx).sha512_hasher = sha512_mem;
}

It assumes it receives a pointer to memory buffer that can fit:

#[repr(C)]
pub struct mbedtls_sha512_context {
    pub sha384_hasher: *mut Context<Sha384>,
    pub sha512_hasher: *mut Context<Sha512>,
    pub is384: c_int,
}

And only allocates memory for the Context pointers and fills pointers to them in that struct. But the struct itself is allocated by the client.

In mbedtls, here is one usage which is easiest to show of the struct allocation and usage of init function:

/*
 * output = SHA-512( input buffer )
 */
int mbedtls_sha512(const unsigned char *input,
                   size_t ilen,
                   unsigned char *output,
                   int is384)
{
    int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED;
    mbedtls_sha512_context ctx;     // <---------------------------- Here is memory 'allocated', see below the structure

#if defined(MBEDTLS_SHA384_C) && defined(MBEDTLS_SHA512_C)
    if (is384 != 0 && is384 != 1) {
        return MBEDTLS_ERR_SHA512_BAD_INPUT_DATA;
    }
#elif defined(MBEDTLS_SHA512_C)
    if (is384 != 0) {
        return MBEDTLS_ERR_SHA512_BAD_INPUT_DATA;
    }
#else /* defined MBEDTLS_SHA384_C only */
    if (is384 == 0) {
        return MBEDTLS_ERR_SHA512_BAD_INPUT_DATA;
    }
#endif

    mbedtls_sha512_init(&ctx); // <------------- call to the (now rust) init function

    if ((ret = mbedtls_sha512_starts(&ctx, is384)) != 0) {
        goto exit;
    }

    if ((ret = mbedtls_sha512_update(&ctx, input, ilen)) != 0) {
        goto exit;
    }

    if ((ret = mbedtls_sha512_finish(&ctx, output)) != 0) {
        goto exit;
    }

exit:
    mbedtls_sha512_free(&ctx);

    return ret;
}

Luckily it is a larger structure but future implementation may change how it works.

/**
 * \brief          The SHA-512 context structure.
 *
 *                 The structure is used both for SHA-384 and for SHA-512
 *                 checksum calculations. The choice between these two is
 *                 made in the call to mbedtls_sha512_starts().
 */
typedef struct mbedtls_sha512_context {
    uint64_t MBEDTLS_PRIVATE(total)[2];          /*!< The number of Bytes processed. */
    uint64_t MBEDTLS_PRIVATE(state)[8];          /*!< The intermediate digest state. */
    unsigned char MBEDTLS_PRIVATE(buffer)[128];  /*!< The data block being processed. */
#if defined(MBEDTLS_SHA384_C)
    int MBEDTLS_PRIVATE(is384);                  /*!< Determines which function to use:
                                                      0: Use SHA-512, or 1: Use SHA-384. */
#endif
}

[ivmarkov]

This binary usage raised another point I had in mind (maybe worth a separate discussion since I'm hijacking a thread again 😄 ), the crate depends on binaries as the source (at least the sha version which is the latest I was able to use). Especially given this is a security component, I don't think such approach would pass a security audit. Getting a crate with binaries that were compiled by a practically unknown source on an unknown machine (that could be even unknwingly compromised) will be an issue. That's an easy supply chain atta

Baremetal folks do not want to deal with C toolchains just to compile the mbedtls C library. Hence we provide .a objects out of the box.

BTW If you are not ok with using binary blobs, you have other places to worry about as well. Namely, the Wifi libs in ESP-IDF (and baremetal!) are only supplied by Espressif as pre-compiled blobs. These blobs do contain security-related stuff as well, like a custom copy of wpa_supplicant for your Wifi network auth and encryption. So esp-mbedtls is not unique in that regard.

With that said, if you are security-savy, you can always re-compile your own libs using the supplied xtasks which are actually well documented. But yes, that requires dealing with C toolchains provided by espup and as you noticed, that's not easy.

I think the correct approach should be (not sure if possible) to have every build (also on developer machine) compile mbedtls from source. But given I wasn't able to compile it myself I'm not sure if the toolset on all systems can support this. Or have these files only built on Github build and prevent from being added through a commit so it is possible to review how they were generated. Again, not sure if doable.

As I said, this is possible. In fact, for the host and ESP-IDF targets, mbedtls IS compiled from source, always. But these targets do have the C toolchains installed by default (100% for ESP-IDF and almost always for the host) so that's a non-issue there.

[ivmarkov]

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.

Here is how I understand it, could be I'm missing something:

If I get it right, the rust side of initialization starts in the shaXXX_init function as below.

#[no_mangle]
pub unsafe extern "C" fn mbedtls_sha512_init(ctx: *mut mbedtls_sha512_context) {
    let sha384_mem =
        crate::calloc(1, core::mem::size_of::<Context<Sha384>>() as u32) as *mut Context<Sha384>;
    let sha512_mem =
        crate::calloc(1, core::mem::size_of::<Context<Sha512>>() as u32) as *mut Context<Sha512>;
    (*ctx).sha384_hasher = sha384_mem;
    (*ctx).sha512_hasher = sha512_mem;
}

It assumes it receives a pointer to memory buffer that can fit:

#[repr(C)]
pub struct mbedtls_sha512_context {
    pub sha384_hasher: *mut Context<Sha384>,
    pub sha512_hasher: *mut Context<Sha512>,
    pub is384: c_int,
}

And only allocates memory for the Context pointers and fills pointers to them in that struct. But the struct itself is allocated by the client.

In mbedtls, here is one usage which is easiest to show of the struct allocation and usage of init function:

/*
 * output = SHA-512( input buffer )
 */
int mbedtls_sha512(const unsigned char *input,
                   size_t ilen,
                   unsigned char *output,
                   int is384)
{
    int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED;
    mbedtls_sha512_context ctx;     // <---------------------------- Here is memory 'allocated', see below the structure

#if defined(MBEDTLS_SHA384_C) && defined(MBEDTLS_SHA512_C)
    if (is384 != 0 && is384 != 1) {
        return MBEDTLS_ERR_SHA512_BAD_INPUT_DATA;
    }
#elif defined(MBEDTLS_SHA512_C)
    if (is384 != 0) {
        return MBEDTLS_ERR_SHA512_BAD_INPUT_DATA;
    }
#else /* defined MBEDTLS_SHA384_C only */
    if (is384 == 0) {
        return MBEDTLS_ERR_SHA512_BAD_INPUT_DATA;
    }
#endif

    mbedtls_sha512_init(&ctx); // <------------- call to the (now rust) init function

    if ((ret = mbedtls_sha512_starts(&ctx, is384)) != 0) {
        goto exit;
    }

    if ((ret = mbedtls_sha512_update(&ctx, input, ilen)) != 0) {
        goto exit;
    }

    if ((ret = mbedtls_sha512_finish(&ctx, output)) != 0) {
        goto exit;
    }

exit:
    mbedtls_sha512_free(&ctx);

    return ret;
}

Luckily it is a larger structure but future implementation may change how it works.

/**
 * \brief          The SHA-512 context structure.
 *
 *                 The structure is used both for SHA-384 and for SHA-512
 *                 checksum calculations. The choice between these two is
 *                 made in the call to mbedtls_sha512_starts().
 */
typedef struct mbedtls_sha512_context {
    uint64_t MBEDTLS_PRIVATE(total)[2];          /*!< The number of Bytes processed. */
    uint64_t MBEDTLS_PRIVATE(state)[8];          /*!< The intermediate digest state. */
    unsigned char MBEDTLS_PRIVATE(buffer)[128];  /*!< The data block being processed. */
#if defined(MBEDTLS_SHA384_C)
    int MBEDTLS_PRIVATE(is384);                  /*!< Determines which function to use:
                                                      0: Use SHA-512, or 1: Use SHA-384. */
#endif
}

Your analysis is incorrect, because you are looking at the wrong stuff.
Clone the esp-mbedtls repo with recursive initialization of the mbedtls GIt submodule and then search inside it for MBEDTLS_SHA512_ALT.

Then look also here.

[ivmarkov]

This binary usage raised another point I had in mind (maybe worth a separate discussion since I'm hijacking a thread again 😄 ), the crate depends on binaries as the source (at least the sha version which is the latest I was able to use).
Especially given this is a security component, I don't think such approach would pass a security audit. Getting a crate with binaries that were compiled by a practically unknown source on an unknown machine (that could be even unknwingly compromised) will be an issue. That's an easy supply chain atta

BTW "always compiling from source" is not a panacea either. The recent injections of backdoors directly in GH source code repos masquerading themselves as "innocent" commits prove that. I guess it is not black and white, but levels of trust you want to accept to the upstream source code (or .a libs) after all.

[ivmarkov]

BTW If you are not ok with using binary blobs, you have other places to worry about as well. Namely, the Wifi libs in ESP-IDF (and baremetal!) are only supplied by Espressif as pre-compiled blobs. These blobs do contain security-related stuff as well, like a custom copy of wpa_supplicant for your Wifi network auth and encryption. So esp-mbedtls is not unique in that regard.

I must take this back, partially. With ESP-IDF, the wpa_supplicant is provided in source code. I guess only the PHY wifi layer is a BLOB.

For esp-wifi though I think all of it is blobs, for the same reasons why mbedtls in esp-mbedtls is a BLOB - hard to deal with C toolchains...

[bjoernQ]

Providing pre-compiled wpa-supplicant and mbedtls static libraries is for convenience since setting up everything needed to compile those is ..... extremely inconvenient especially if the user won't need the C toolchains for anything else. (as Ivan already said)

All other static libraries in esp-wifi-sys are not available in source (for reasons)

I'd really wish we will see a pure-Rust replacement of wpa-supplicant some day (for a couple of reasons).
For mbedtls, someday RusTLS might become a viable alternative

(And we are currently going the same route for OpenThread - I'd love to see a pure Rust Thread implementation)

However, all the scripts to compile all of that are available and people could build those on a trusted machine and cargo-patch those dependencies if they really want

[yanshay]

However, all the scripts to compile all of that are available and people could build those on a trusted machine and cargo-patch those dependencies if they really want

Maybe worth adding some instructions for poor Mac users like me on how to setup the toolchain to work on Mac. I couldn't get it to compile, had some issue with lld not supporting ZLib.

[AnthonyGrondin]

Closing as fixed by d6e5041

Feel free to open a new issue addressing the build concerns, if any. I had a long-term goal of automatically building the libs in CI, whenever there's a change related to esp-mbedtls-sys/ but there would need to be more attraction and maintainers, for this to be viable and worth it.