Skip to content

Conversation

@pppaulpeter
Copy link

Implement parse_cipher_list_to_ids() to convert IANA names to mbedtls IDs
Update SSL_set_cipher_list() to apply ciphersuites immediately when active
Add cipher list application in server vhost initialization
Add capability guards for mbedtls session functions (compatible with 2.16.3)
Verified: cipher restrictions properly enforced during TLS handshake"

@lws-team
Copy link
Member

Thanks... as I mentioned I was hoping to avoid a migraine.

  1. Was this done with the assistance of an LLM, if so which one? It's not necessarily a problem, I am using Gemini 3.0 for some months, but I would like to know.

  2. Please don't introduce using strtok() to lws. There's a robust lws tokenizer "lws_tokenize" include/libwebsockets/lws-tokenize.h that should be able to cope, if not, it's fairly easy to extend by flags.

  3. Please don't push your development history. Reverting something? Fiddling with gitignore? Nobody cares to know, just remove whatever it was from the sources, no need for another patch. If your development is like that, squash the revert patch into the patch that added it so there's an antimatter type simplification explosion.

In order to remove all the churn (your delta was 13 patches...) I did a git diff from the basis point from main you used to your HEAD, this is a single patch capturing your changes that is a lot simpler than what you pushed. Layered patches are good but if it just obscures the effect of your patch it is bad and I would much rather have one patch.

  1. Rebase your patches on HEAD main before pushing. When I applied the simplification patch, there were a handful of merge problems (even with -l) which I manually fixed up but one that implies your main basis and the current one are different enough.

  2. What's the situation with "IETF" alg names that we are introducing here, and eg openssl or now schannel? Openssl already liked them and we are just aligning mbedtls to use the same instead of nonstandard ones?

In summary: Please a) convert away from strtok to struct lws_tokenize, b) squash everything to one patch like I described above and c) push it on top of today's main branch, test then d) push again to your PR branch with that.

@pppaulpeter
Copy link
Author

Thanks... as I mentioned I was hoping to avoid a migraine.

  1. Was this done with the assistance of an LLM, if so which one? It's not necessarily a problem, I am using Gemini 3.0 for some months, but I would like to know.
  2. Please don't introduce using strtok() to lws. There's a robust lws tokenizer "lws_tokenize" include/libwebsockets/lws-tokenize.h that should be able to cope, if not, it's fairly easy to extend by flags.
  3. Please don't push your development history. Reverting something? Fiddling with gitignore? Nobody cares to know, just remove whatever it was from the sources, no need for another patch. If your development is like that, squash the revert patch into the patch that added it so there's an antimatter type simplification explosion.

In order to remove all the churn (your delta was 13 patches...) I did a git diff from the basis point from main you used to your HEAD, this is a single patch capturing your changes that is a lot simpler than what you pushed. Layered patches are good but if it just obscures the effect of your patch it is bad and I would much rather have one patch.

  1. Rebase your patches on HEAD main before pushing. When I applied the simplification patch, there were a handful of merge problems (even with -l) which I manually fixed up but one that implies your main basis and the current one are different enough.
  2. What's the situation with "IETF" alg names that we are introducing here, and eg openssl or now schannel? Openssl already liked them and we are just aligning mbedtls to use the same instead of nonstandard ones?

In summary: Please a) convert away from strtok to struct lws_tokenize, b) squash everything to one patch like I described above and c) push it on top of today's main branch, test then d) push again to your PR branch with that.

yes, i use chatgpt5.2, claude,grok etc. i will check your valuable comment carefully and since this is my first git push for pull request, there must be a lot of problem.

- Add IANA cipher suite parsing for mbedtls backend using lws_tokenize
- Implement ssl_pm_set_ciphersuites() to apply cipher IDs to SSL config
- Apply cipher list during TLS handshake in lws_tls_server_vhost_backend_init()
- Fix missing mbedtls_ssl_session_save() call in session update case
- Add CMake detection for mbedtls_ssl_conf_ciphersuites and mbedtls_ssl_session_save
- Standardize macro naming to follow lowercase function pattern

This implementation aligns the mbedtls backend with OpenSSL and Schannel by
supporting standard IETF/IANA cipher names. The lws_tokenize API provides
robust, non-destructive tokenization and eliminates hardcoded cipher lists,
allowing automatic updates when new ciphers are added to mbedtls.
@pppaulpeter pppaulpeter force-pushed the feat/mbedtls-iana-ciphers branch from c510a77 to f7c48e7 Compare January 23, 2026 06:07
@lws-team
Copy link
Member

Thanks.

I split this out into two patches, one to deal with handling missing session support and the other your main patch. I pushed where I am at with it on lws _temp branch, you can switch to patching on top of that.

I think it's generally good, but I noticed 1)

int SSL_set_cipher_list(SSL *ssl, const char *str)
{
    int ok;

    SSL_ASSERT1(ssl);

    ok = SSL_CTX_set_cipher_list(ssl->ctx, str);
...

This is a bit of a no-no, the api claims to affect the SSL but it actually affects the ctx, which is to say the vhost. If the guy wants to change the vhost in a sticky way, he should use the api with _ctx in the name, not one that looks like it only affects the one connection he passes to it.

For server, it makes sense to have the _ctx api and be able to adjust the vhost, since you are setting up your server and the whole vhost behaviours. If you want to control the client alg binding per connection, it doesn't make sense to touch the _ctx / vhost. IIUI the cipher list should be bound to the SSL, not the "SSL_CTX" in that case. At any rate it seems wrong as it is with the SSL api affecting the SSL_CTX.

  1. Exactly which LLM should we lay the blame on :-) Eg, I am using "Co-developed-by: Gemini 3.0 Pro" If we notice strange behaviours later, it will be useful to know where it came from.

  2. We should add some testing. The closes existing api test is ../minimal-examples-lowlevel/api-tests/api-test-gencrypto you can add some tests on main in there protected if LWS_WITH_MBEDTLS. It just needs to confirm the expected good matches and failed matches. We already run gencrypto tests in ctest so long as we have LWS_WITH_GENCRYPTO set this should take care of it.

@lws-team
Copy link
Member

OK I followed your removal of the SSL api.

  1. Exactly which LLM should we lay the blame on :-) Eg, I am using "Co-developed-by: Gemini 3.0 Pro" If we notice strange behaviours later, it will be useful to know where it came from.

Please ask whoever you have to ask to get permission to explain this in a word or two in the patch as shown above. My email address is everywhere in the code and I explain my LLM usage, since I don't take anyone's money because the code is given for free, I don't care about anyone else's policies except my own.

The test stuff can't build because it can't touch private apis / headers from lws. The test apps are essentially "user code" and link against the library without any special access inside. So it should just use whatever the public api is for your thing that's accessible to user code, using public headers etc. If there's no public api because you talk to it by an info struct or whatever, then you have to test it from that perspective.

@lws-team lws-team force-pushed the main branch 3 times, most recently from 260929d to f06bf53 Compare January 25, 2026 20:25
@pppaulpeter
Copy link
Author

OK I followed your removal of the SSL api.

  1. Exactly which LLM should we lay the blame on :-) Eg, I am using "Co-developed-by: Gemini 3.0 Pro" If we notice strange behaviours later, it will be useful to know where it came from.

Please ask whoever you have to ask to get permission to explain this in a word or two in the patch as shown above. My email address is everywhere in the code and I explain my LLM usage, since I don't take anyone's money because the code is given for free, I don't care about anyone else's policies except my own.

The test stuff can't build because it can't touch private apis / headers from lws. The test apps are essentially "user code" and link against the library without any special access inside. So it should just use whatever the public api is for your thing that's accessible to user code, using public headers etc. If there's no public api because you talk to it by an info struct or whatever, then you have to test it from that perspective.

sorry, i just update the unit test code.

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

Labels

None yet

2 participants