SSL_get_cipher_list: enumerate the configured cipher list - #11003
Draft
julek-wolfssl wants to merge 1 commit into
Draft
SSL_get_cipher_list: enumerate the configured cipher list#11003julek-wolfssl wants to merge 1 commit into
julek-wolfssl wants to merge 1 commit into
Conversation
SSL_get_cipher_list() mapped to wolfSSL_get_cipher_list_ex(), which returns the negotiated cipher at index 0 and NULL at every other index. OpenSSL returns the name of the n-th cipher in the SSL's configured list, so callers walking the list only ever saw one entry. wolfSSL_get_cipher_list_ex() has a fallback that enumerates, but it is unreachable. It triggers when wolfSSL_get_cipher_name_internal() returns NULL, and GetCipherNameInternal() has returned the literal "None" instead of NULL since ee13dfd. Before a handshake that made SSL_get_cipher_list(ssl, 0) report "None". The fallback also indexed the global cipher_names[] table rather than the suites configured on the SSL, so it did not match OpenSSL either. Add wolfSSL_get_cipher_list_compat() and point the macro at it. It walks WOLFSSL_SUITES(ssl) with the same SCSV and min/max version filtering wolfSSL_get_ciphers_compat() uses, so SSL_get_cipher_list() and SSL_get_ciphers() report the same suites in the same order, and it names them the way SSL_CIPHER_get_name() does. wolfSSL_get_cipher_list_ex() is left unchanged for direct callers.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes the OpenSSL-compat behavior of SSL_get_cipher_list() so it enumerates the configured cipher list (not just the negotiated cipher at index 0), aligning its output and ordering with SSL_get_ciphers()/SSL_CIPHER_get_name().
Changes:
- Added
wolfSSL_get_cipher_list_compat()that iteratesWOLFSSL_SUITES(ssl)with the same filtering used bywolfSSL_get_ciphers_compat(). - Remapped the
SSL_get_cipher_listmacro to use the new compat function. - Added an API test verifying
SSL_get_cipher_list()matches the entries returned bySSL_get_ciphers().
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
wolfssl/ssl.h |
Declares the new wolfSSL_get_cipher_list_compat() API under OpenSSL-compat feature guards. |
wolfssl/openssl/ssl.h |
Updates SSL_get_cipher_list macro mapping to the new compat enumerator. |
src/ssl.c |
Implements wolfSSL_get_cipher_list_compat() to enumerate configured suites with filtering and OpenSSL-compatible naming. |
tests/api.c |
Adds a test ensuring SSL_get_cipher_list() walks the configured list and matches SSL_get_ciphers() entry-for-entry. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
248
to
252
| #define SSL_get_client_random(ssl,out,outSz) \ | ||
| wolfSSL_get_client_random((ssl),(out),(outSz)) | ||
| #define SSL_get_cipher_list(ctx,i) wolfSSL_get_cipher_list_ex((ctx),(i)) | ||
| #define SSL_get_cipher_list(ssl,i) \ | ||
| wolfSSL_get_cipher_list_compat((ssl),(i)) | ||
| #define SSL_get_cipher_name(ctx) wolfSSL_get_cipher((ctx)) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
SSL_get_cipher_list()was mapped towolfSSL_get_cipher_list_ex(), whichonly returned the negotiated cipher at index 0 and NULL for every other
index, so callers walking the list saw a single entry instead of the full
configured list as OpenSSL does.
wolfSSL_get_cipher_list_ex()had a fallback meant to enumerate the list,but it was unreachable (it only triggers when the internal cipher name
lookup returns NULL, which hasn't happened since the lookup started
returning the literal "None" instead). Even if reached, it indexed the
global
cipher_names[]table rather than the suites actually configured onthe SSL, so it wouldn't have matched OpenSSL's behavior anyway.
Add
wolfSSL_get_cipher_list_compat()and point theSSL_get_cipher_listmacro at it instead. It walks
WOLFSSL_SUITES(ssl)with the same SCSV andmin/max version filtering used by
wolfSSL_get_ciphers_compat(), soSSL_get_cipher_list()andSSL_get_ciphers()now report the same suitesin the same order, named the way
SSL_CIPHER_get_name()does.wolfSSL_get_cipher_list_ex()itself is left unchanged for direct callers.