runtests: generate certs dynamically, bump to EC-256, tidy up#16824
Closed
vszakats wants to merge 80 commits intocurl:masterfrom
Closed
runtests: generate certs dynamically, bump to EC-256, tidy up#16824vszakats wants to merge 80 commits intocurl:masterfrom
vszakats wants to merge 80 commits intocurl:masterfrom
Conversation
Member
Author
|
Fallouts:
|
Member
Author
|
This is working. A handful of local issues aside, it was reasonably smooth to implement. Downside is that it makes running tests require POSIX shell, also with CMake. It's also to be seen how annoying the cert generation is when running local tests. We might want to make the |
vszakats
changed the title
Contributor
|
Excellent! |
Member
Author
|
edit: Possibly caused by an upstream update to vcpkg libiconv: microsoft/vcpkg#44424 (comment) Odd, unrelated-looking, Android 21 CMake build fallout popping in and out: (gone after a rebuild) |
Member
Author
|
Hah, the question now is how to tell |
vszakats
changed the title
Member
Author
|
This seems complete. Now integrated into the test run targets. Also bumped keys to EC-256, and cut down on log output to make it faster. Now it's 3-4 seconds on the slowest MSVC/Cygwin jobs, which is fine. The step could be dropped for non-stunnel jobs to save idle work (though it'd probably be better to have stunnel everywhere for coverage, e.g. on Cygwin). Bash dependency still stands. For stunnel-enabled runs, bash mush be available to avoid failing test cases. If we get reports of this combo causing confusion, we can add dedicated logic and an early error message for example. Or the non-bash alternative if that case turns out to be important.
edit: dependency handling and local use may also need updates to limit re-runs only when necessary. I haven't tested. |
Member
Author
|
Update: not ready yet. Local tests kept regenerating certs. Trying to fix things now. Also moving certs to the build directory from the source, which seems to be natural place for them, when generated dynamically. This allows dropping the (Though generating them is quite quick, and the log is just a few lines now.) autotools is still to be tested. |
Member
Author
|
Automake, cmake, local, repeated builds are covered now. This is ready to merge. Likely fit for RC3 or the release. |
vszakats
changed the title
vszakats
changed the title
vszakats
changed the title
vszakats
added a commit
that referenced
this pull request
Mar 26, 2025
vcpkg requires Android 28 by default after a recent update that's being deployed onto CI runs (with `libiconv:[email protected]#1`). Revert to bare, no-ssl, no-psl configuration for Android 21 jobs to make them work again. Bug: #16824 (comment) Ref: microsoft/vcpkg#44424 (comment) Closes #16832
vszakats
changed the title
To avoid regenerating certs always.
It'd much nicer to use a multi-target rule, but that'd require GNU Make 4.3 for "Grouped explicit targets" with `&:`.
bagder
approved these changes
Mar 27, 2025
Member
bagder
left a comment
bagder
left a comment
There was a problem hiding this comment.
I think getting rid of those big "magic" files from git is a huge benefit, let's do this!
Member
Author
|
This is merged now! Does anybody know how to generate |
vszakats
added a commit
to vszakats/curl
that referenced
this pull request
Mar 28, 2025
To remove POSIX shell as an extra dependency for runtests. Follow-up to 44341e7 curl#16824
Member
Author
|
The obvious just hit me: We might rewrite As for SRP, the GnuTLS We may create a generatotr script, but running it dynamically will |
Member
Yes I think we should to that. SRP also does not work with TLS 1.3 and was never used much. |
vszakats
added a commit
to vszakats/curl
that referenced
this pull request
Mar 28, 2025
To remove POSIX shell as an extra dependency for runtests. Follow-up to 44341e7 curl#16824
vszakats
added a commit
that referenced
this pull request
Apr 3, 2025
pps83
pushed a commit
to pps83/curl
that referenced
this pull request
Apr 26, 2025
vcpkg requires Android 28 by default after a recent update that's being deployed onto CI runs (with `libiconv:[email protected]#1`). Revert to bare, no-ssl, no-psl configuration for Android 21 jobs to make them work again. Bug: curl#16824 (comment) Ref: microsoft/vcpkg#44424 (comment) Closes curl#16832
pps83
pushed a commit
to pps83/curl
that referenced
this pull request
Apr 26, 2025
Before this patch the curl repository and source tarball distribution contained test certificates as binary blobs. Used by runtests. Drop these certificates in favor of generating them dynamically as part of the build process. Both via autotools and CMake. As part of this, improve certificates, the generator script and process, file layout, and fix any issue to make it work fast and smooth both in CI and local builds. Note, cert generator scripts require OpenSSL >=1.0.2 (or LibreSSL >=3.1.0). Generation requires POSIX shell, also with CMake. Without a POSIX shell tests relying on TLS (and stunnel) will fail. Details: - build: generate certs as part of the test run process. - build, tests: generate certs in the build directory. - binarycheck: drop concept of known binary files with hashes. - binarycheck: move binary check logic into spacecheck and drop this separate checker tool. - build: fix to clean all cert files. - autotools: fix to not run leaf cert generators in parallel. To avoid confusion when updating the revocation database and counter. - scripts: drop `scripts` subdir, merge two scripts into one, auto-generate root cert, allow generating multiple leafs at once. - scripts: switch to EC-256 keys (was: RSA-2048). For key size and perf. - scripts: drop `-x` echo, text dumps, most other output. To avoid log noise and make it quicker in CI. - scripts: make it non-RSA-specific. - scripts: delete unused code. - scripts: use POSIX shell shebang. Some envs don't have bash (Alpine). - scripts: pass test pseudo-secrets via the command-line. To avoid: ``` + openssl genrsa -out test-ca.key -passout fd:0 2048 Invalid password argument, starting with "fd:" ``` - cmake: fix to launch generator scripts via the detected POSIX shell. - cmake: fix `build-certs` rule to not depend on `SRPFILES` (`srp-verifier-*`). - cmake: drop `EXCLUDE_FROM_ALL` for the cert subdir. It makes the Visual Studio generator miss to create the `clean-certs`, `build-certs` targets. No target depend on them, so they don't execute implicitly anyway. Fixes: ``` MSBUILD : error MSB1009: Project file does not exist. Switch: clean-certs.vcxproj ``` - cmake: add `VERBATIM USES_TERMINAL` to `build-certs` target. - GHA/linux: install openssl on Alpine, for the cert generator scripts. Follow-up to 556f722 curl#16593 Follow-up to fa461b4 curl#14486 Closes curl#16824
pps83
pushed a commit
to pps83/curl
that referenced
this pull request
Apr 26, 2025
To remove POSIX shell as an extra dependency for runtests. Also fix to `chmod 0600` the `.pem` file (was: `.prm`), and apply it _before_ writing the keys. Follow-up to 44341e7 curl#16824 Closes curl#16858
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
3 participants
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.
Before this patch the curl repository and source tarball distribution
contained test certificates as binary blobs. Used by runtests.
Drop these certificates in favor of generating them dynamically as
part of the build process. Both via autotools and CMake.
As part of this, improve certificates, the generator script and process,
file layout, and fix any issue to make it work fast and smooth both in
CI and local builds.
Note, cert generator scripts require OpenSSL >=1.0.2
(or LibreSSL >=3.1.0). Generation requires POSIX shell, also with CMake.
Without a POSIX shell tests relying on TLS (and stunnel) will fail.
Details:
separate checker tool.
confusion when updating the revocation database and counter.
scriptssubdir, merge two scripts into one,auto-generate root cert, allow generating multiple leafs at once.
-xecho, text dumps, most other output. To avoid lognoise and make it quicker in CI.
build-certsrule to not depend onSRPFILES(
srp-verifier-*).EXCLUDE_FROM_ALLfor the cert subdir. It makesthe Visual Studio generator miss to create the
clean-certs,build-certstargets. No target depend on them, so they don't executeimplicitly anyway. Fixes:
VERBATIM USES_TERMINALtobuild-certstarget.Follow-up to 556f722 #16593
Follow-up to fa461b4 #14486
w/o ws https://github.com/curl/curl/pull/16824/files?w=1
TODO:
clean-certsstep from CI workflows.