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

Unable to reference MBEDTLS_ERR_NET_SEND_FAILED and MBEDTLS_ERR_NET_RECV_FAILED after upgrade to ESP-IDF v5.3.2 / esp-idf-svc 0.50.1 #361

Open
acmorrow opened this issue Jan 6, 2025 · 3 comments
Labels
bug Something isn't working

Comments

@acmorrow
Copy link

acmorrow commented Jan 6, 2025

Bug description

I'm attempting to update an esp-rs based project which is currently using ESP-IDF v4.4.8 / esp-idf-svc 0.48.1 / esp-idf-sys 0.34.1 to ESP-IDF v5.3.2 / esp-idf-svc 0.50.1 / esp-idf-sys 0.36.0.

Things are mostly building, but I've found that references to esp_idf_svc::sys::MBEDTLS_ERR_NET_SEND_FAILED and esp_idf_svc::sys::MBEDTLS_ERR_NET_RECV_FAILED no longer work.

The symbols do still seem to be part of mbedtls as bundled in ESP-IDF v5.3.2:

esp-idf/v5.3.2/components/mbedtls/mbedtls/include/mbedtls/net_sockets.h
50:#define MBEDTLS_ERR_NET_SEND_FAILED                       -0x004E
esp-idf/v5.3.2/components/mbedtls/mbedtls/include/mbedtls/net_sockets.h
48:#define MBEDTLS_ERR_NET_RECV_FAILED                       -0x004C

But when I look for these symbols in bindings.rs generated with ESP-IDF v5.3.2 / esp-idf-svc 0.50.1 I don't get any matches:

$ grep -c MBEDTLS_ERR_NET `find . -name bindings.rs`
0

However, after reverting to an ESP-IDF v4.4.8 / esp-idf-svc 0.48.1 build I can see those symbols are generated into bindings.rs:

$ grep MBEDTLS_ERR_NET `find . -name bindings.rs`
pub const MBEDTLS_ERR_NET_SOCKET_FAILED: i32 = -66;
pub const MBEDTLS_ERR_NET_CONNECT_FAILED: i32 = -68;
pub const MBEDTLS_ERR_NET_BIND_FAILED: i32 = -70;
pub const MBEDTLS_ERR_NET_LISTEN_FAILED: i32 = -72;
pub const MBEDTLS_ERR_NET_ACCEPT_FAILED: i32 = -74;
pub const MBEDTLS_ERR_NET_RECV_FAILED: i32 = -76;
pub const MBEDTLS_ERR_NET_SEND_FAILED: i32 = -78;
  • Would you like to work on a fix?

Sure, if it is a real issue and I haven't messed something up, I'd be happy to work on a fix with some guidance if that would be helpful.

To Reproduce

Check out acmorrow/micro-rdk@0c7c2aa and run make build-esp32-bin, and you should see errors like:

error[E0432]: unresolved imports `crate::esp32::esp_idf_svc::sys::MBEDTLS_ERR_NET_RECV_FAILED`, `crate::esp32::esp_idf_svc::sys::MBEDTLS_ERR_NET_SEND_FAILED`
  --> micro-rdk/src/esp32/dtls.rs:32:56
   |
32 |     mbedtls_x509_crt_init, mbedtls_x509_crt_parse_der, MBEDTLS_ERR_NET_RECV_FAILED,
   |                                                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^ no `MBEDTLS_ERR_NET_RECV_FAILED` in `sys`
33 |     MBEDTLS_ERR_NET_SEND_FAILED, MBEDTLS_ERR_SSL_PEER_CLOSE_NOTIFY, MBEDTLS_ERR_SSL_TIMEOUT,
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^ no `MBEDTLS_ERR_NET_SEND_FAILED` in `sys`
   |
help: a similar name exists in the module
   |
32 |     mbedtls_x509_crt_init, mbedtls_x509_crt_parse_der, MBEDTLS_ERR_RSA_RNG_FAILED,
   |                                                        ~~~~~~~~~~~~~~~~~~~~~~~~~~
help: a similar name exists in the module
   |
33 |     MBEDTLS_ERR_RSA_RNG_FAILED, MBEDTLS_ERR_SSL_PEER_CLOSE_NOTIFY, MBEDTLS_ERR_SSL_TIMEOUT,
   |     ~~~~~~~~~~~~~~~~~~~~~~~~~~

Note that there are some other errors in there that I haven't investigated yet.

Expected behavior

Unless these symbols have been removed from mbedtls (doesn't seem like it) or purposely withdrawn from visibility as part of esp-idf / esp-idf-sys, I'd expect to be able to continue to use them. I looked but didn't see any sort of release note that seemed to hint at that. Also possible i'm missing some new kconfig option or esp-idf-[sys,svc] feature?

Environment

  • Crate (esp-idf-sys) version: 0.36.0
  • ESP-IDF branch or tag: v5.3.2
  • Target device (MCU): esp32
  • OS: macOS
@acmorrow acmorrow added the bug Something isn't working label Jan 6, 2025
@github-project-automation github-project-automation bot moved this to Todo in esp-rs Jan 6, 2025
@ivmarkov
Copy link
Collaborator

ivmarkov commented Jan 7, 2025

I think in 4.4.X you had those symbols, because 4.4.X uses an older/different mbedtls library and mbedtls porting layer, and the "net_sockets.h" header got included anyway even if we do not explicitly include it in our bindings header.

In 5.X, the ESP IDF folks did some includes' cleanup, and now some headers are missing unless we explicitly re-add them.

While I think it is not a problem to just add the "net_sockets.h" header to the list of mbedtls includes from above, I'm a bit more concerned whether you sould be using anything from that header (even #define-s) anyway, because - while the header with the defines and function signatures seems always available and safe to include - its function implementations are only available if MBEDTLS_NET_C is defined and if the ESP IDF mbedtls port layer (or rather, fork) is not doing something weird (as in providing its own socket layer by patching it in or using the mbedtls "bio" callbacks for connecting mbedtls to the LwIP network layer).

So before just including this header into the esp-idf-sys bindings, the question is more like, why do you need to refer anything from that header in the first place? Or in other words, what do you use mbedtls for? Implementing an alternative TLS socket layer to whatever ESP IDF exposes in the "esp-tls" component, or for...?

@acmorrow
Copy link
Author

acmorrow commented Jan 7, 2025

@ivmarkov -

Those are all fair points and questions, and thanks for the prompt reply. In this instance, we use these error values while implementing the three callbacks needed by mbedtls_ssl_set_bio, which we are using as part of implementing an async DTLS layer in our WebRTC implementation. If you are interested, the project is open source, so you can have a look here: https://github.com/viamrobotics/micro-rdk/blob/09ead6fb4b9d9029bcd139496c49a8e1d2d262b0/micro-rdk/src/esp32/dtls.rs#L49-L122.

Note that I'm not the original author of this code, I'm merely trying to position us to make the jump to ESP-IDF 5 as soon as we can, given that ESP-IDF 4 is EOL, and this is currently blocking me. I can get the original author involved if it would help clarify our usage.

I do understand your position about not exporting symbols that may not be meaningfully usable. On the other hand, in this case, an available symbol was revoked. On the other other hand, this is a major version bump for ESP-IDF, so, fair enough to do that, but it is a bit inconvenient and I don't have an obvious path forward.

@ivmarkov
Copy link
Collaborator

ivmarkov commented Jan 7, 2025

@ivmarkov -

Those are all fair points and questions, and thanks for the prompt reply. In this instance, we use these error values while implementing the three callbacks needed by mbedtls_ssl_set_bio, which we are using as part of implementing an async DTLS layer in our WebRTC implementation.

Interesting. The mbedtls documentation is unfortunately lacking the detail as to is it OK to return those error codes from the "bio" callbacks in the first place.

But if you believe returning these is OK, then why not, we can always include the header in our bindings.h file.

In fact, if you open a PR with that, I can merge immediately given that the CI completes.

If you are interested, the project is open source, so you can have a look here: https://github.com/viamrobotics/micro-rdk/blob/09ead6fb4b9d9029bcd139496c49a8e1d2d262b0/micro-rdk/src/esp32/dtls.rs#L49-L122.

It might be that we are implementing the same thing, except slightly differently. I mean, if you guys are implementing a TLS socket layer, and not some other custom encryption scheme based on mbedtls (I don't know WebRTC). Look at esp-mbedtls. It is a generic async DTLS socket layer based on mbedtls as well. Still a bit in its infancy, but perhaps we can join forces in the long run, and develop just one Rust mbedtls-based TLS-socket-layer thing?

Note that I'm not the original author of this code, I'm merely trying to position us to make the jump to ESP-IDF 5 as soon as we can, given that ESP-IDF 4 is EOL, and this is currently blocking me. I can get the original author involved if it would help clarify our usage.

Would be nice, specifically in relation to my observation from above.

I do understand your position about not exporting symbols that may not be meaningfully usable. On the other hand, in this case, an available symbol was revoked. On the other other hand, this is a major version bump for ESP-IDF, so, fair enough to do that, but it is a bit inconvenient and I don't have an obvious path forward.

You can always return the actual constants behind these #defines, as they are documented and not supposed to change. I mean, for the (very) short term at least.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: Todo
Development

No branches or pull requests

2 participants