Start of SSL_CONF_* API support, Ubuntu 24.04 in CI #29
Conversation
This file contains 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
Will need to admin merge this one since the job names change out from under the branch protection rules. |
simondeziel
reviewed
Jun 21, 2024
ctz
approved these changes
Jun 24, 2024
This commit introduces the scaffolding for the `SSL_CONF_*` APIs that were previously stubbed. All of the API fns with the exception of `SSL_CONF_cmd_argv` are now implemented as of this commit. Note: we do not yet suppor the vast majority of possible `SSL_CONF_cmd` options. To start, only `MinProtocol` and `MaxProtocol` are supported. Additional commands will be supported in subsequent commits.
* Replace `ubuntu-latest` with `ubuntu-22.04` - we expect that soon that tag will point at 24.04 and we want to keep testing w/ 22.04 explicitly * Add `ubuntu-24.04` - in the future, once the switch mentioned above has happened, we can replace this with `ubuntu-latest` once more. * Add a helper to `tests/runner.rs` for checking the Nginx version. We use this to conditionally skip the TLS session resumption nginx tests when running on 24.04 with Nginx 1.24+ - there's an outstanding issue where resumption doesn't work with this nginx version.
We want to test the `ssl_conf_command` directive, but this is only available in nginx 1.24+. This commit adds a 1.24 specific config file and updates the test runner so we can spin up and test a nginx 1.24 server with this config when available. For now we test the `MinProtocol` and `MaxProtocol` OpenSSL CONF_CTX commands that the compat shim supports.
|
@ctz Do you want to re-review before I merge? |
ctz
approved these changes
Jun 25, 2024
|
Admin merged + fixed up the branch protection rules afterwards. |
This was referenced Jun 25, 2024
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.
I had started to implement more of the commands and then realized my diff was getting pretty chonky so to start with I've pulled out all of the scaffolding and just two initial supported commands:
MinProtocolandMaxProtocol.There's an included C program/unit test that runs through some various
SSL_CONF_CTX_set1_prefixandSSL_CONF_CTX_set_flagssetups, matching theSSL_CONF_cmd_value_typeoutput for supported options between our impl and OpenSSL. This was a useful exercise as I discovered this API has some interesting nuance to it. Initially I intended to not support setting a prefix, but it turned out the command matching/selection logic is fairly tied to the notion of the prefix in use (default or otherwise) and so I implemented it after all. In sum we now support all theSSL_CONF_*APIs in some manner exceptSSL_CONF_cmd_argv(this one felt too niche to care about at this stage.).For Nginx this API surface is used to support the
ssl_conf_commanddirective. Using that for an integration test is fairly straightforward, but the Nginx version on Ubuntu 22.04 doesn't support this directive :-( To resolve this I tacked on an update to CI that adds Ubuntu 24.04. When we added initial 24.04 support there wasn't a runner image available. That's since been resolved. The newssl_conf_commandis then written to run conditionally based on thenginxversion installed, skipping on 22.04 and running on 24.04.There's one additional rub with the 24.04 testing: as mentioned in another issue session resumption w/ Nginx 24.04 and our compat
.sodoesn't seem to be working correctly. We should fix this, but in the meantime I've updated the nginx unit tests to skip the resumption tests when using Nginx 1.24+.Updates #22