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

feat(websocket): Support DS peripheral for mutual TLS (IDFGH-12285) #520

Merged

Conversation

johanstokking
Copy link
Contributor

@johanstokking johanstokking commented Mar 6, 2024

Fantastic project guys. Here's something I use that you might want to take into consideration: supporting the DS peripheral for mutual TLS.

Example usage with https://github.com/espressif/esp_secure_cert_mgr/:

    char *client_cert = NULL;
    uint32_t client_cert_len = 0;
    esp_err_t ret = esp_secure_cert_get_device_cert(&client_cert, &client_cert_len);
    assert(esp_ret == ESP_OK);

    esp_ds_data_ctx_t *ds_data = esp_secure_cert_get_ds_ctx();
    assert(ds_data != NULL);

    esp_websocket_client_config_t config = {
        .uri = "wss://echo.websocket.org",
        .client_cert = client_cert, /* in this example it's PEM so client_cert_len is not set */
        .client_ds_data = ds_data,
        .crt_bundle_attach = esp_crt_bundle_attach,
    };

I considered adding this to the example project, but I feel like it gets bloated quickly with dependencies and ifdefs.

@CLAassistant
Copy link

CLAassistant commented Mar 6, 2024

CLA assistant check
All committers have signed the CLA.

@github-actions github-actions bot changed the title feat(websocket): Support DS peripheral for mutual TLS feat(websocket): Support DS peripheral for mutual TLS (IDFGH-12285) Mar 6, 2024
@johanstokking
Copy link
Contributor Author

@gabsuren Can you shed some light on this? Thanks!

@gabsuren
Copy link
Contributor

@gabsuren Can you shed some light on this? Thanks!

Sorry for the late response, it's on the line with #621

@david-cermak
Copy link
Collaborator

@johanstokking Thanks for implementing this useful feature. I've tested locally and it works as expected.

I also think that we shouldn't add more configs and user options to the current example. It's already quite hard to follow for first-time users.
It would be helpful though to add a few sentences to the README.

Could you please make the following changes:

idf.py add-dependency esp_secure_cert_mgr

@johanstokking johanstokking force-pushed the feature/websocket/client_ds_data branch from 6ada58d to 44d476f Compare January 28, 2025 13:59
@johanstokking
Copy link
Contributor Author

johanstokking commented Jan 28, 2025

@david-cermak I implemented your suggestions. I got myself briefly in the rabbit hole of rendering per-target documentation pages (to render the DS peripheral section conditionally on SOC_DIG_SIGN_SUPPORTED, like supported in esp-idf repo), but I have too little experience with this docs system to implement that properly.

This is what the TLS section looks like:

Screenshot 2025-01-28 at 14-56-09 ESP WebSocket Client - ESP32 - — ESP-Protocols latest documentation

Copy link
Collaborator

@david-cermak david-cermak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, thanks for the updates (and those little fixes to the docs)!

@david-cermak david-cermak merged commit e069ae7 into espressif:master Jan 28, 2025
55 checks passed
@johanstokking johanstokking deleted the feature/websocket/client_ds_data branch January 28, 2025 16:01
@david-cermak
Copy link
Collaborator

This is what the TLS section looks like:

sorry, hit edit instead of reply. wanted to say that we don't generate the docs for each target, only one static page.

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

Successfully merging this pull request may close these issues.

7 participants