Skip to content

runtests: generate certs dynamically, bump to EC-256, tidy up #16824

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

Closed
wants to merge 80 commits into from

Conversation

vszakats
Copy link
Member

@vszakats vszakats commented Mar 24, 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 #16593
Follow-up to fa461b4 #14486


w/o ws https://github.com/curl/curl/pull/16824/files?w=1

TODO:

  • drop certs from repo.
  • drop clean-certs step from CI workflows.
  • integrate cert gen/clean steps with the test targets (runtests and pytest).
  • consider generating ECC certs for better performance.

@vszakats vszakats added TLS tests CI Continuous Integration labels Mar 24, 2025
@vszakats
Copy link
Member Author

vszakats commented Mar 24, 2025

Fallouts:

  • Alpine: doesn't have bash, script fails to start.
  • msys/mingw/autotools: odd failure in a single job:
    + openssl genrsa -out test-ca.key -passout fd:0 2048
    Invalid password argument, starting with "fd:"
    
    https://github.com/curl/curl/actions/runs/14046261629/job/39327676774?pr=16824#step:15:22
  • msys/mingw/cmake: wants to launch the script with Windows cmd.exe.
  • dl-mingw: same issue.
  • MSVC: MSBuild says:
    MSBUILD : error MSB1009: Project file does not exist.
    Switch: clean-certs.vcxproj
    

@vszakats
Copy link
Member Author

vszakats commented Mar 25, 2025

This is working. A handful of local issues aside, it was reasonably smooth to implement.
Cert generation takes 1-2 seconds or Unix platforms, and a few seconds more on
Windows (up to 5-7s). This isn't nothing, but probably fine, and well compensated by
recently (e.g. yesterday) applied speed-ups.

Downside is that it makes running tests require POSIX shell, also with CMake.
This may affect Windows users. With (a POSIX) Perl already being a requirement, this
seems passable. To avoid it scripts/gen*.sh logic can be moved within CMake, with
the cost of maintaining two implementations onwards.

It's also to be seen how annoying the cert generation is when running local tests.

We might want to make the build-cert step automatic to not break everyones' workflow.
And perhaps migrate the clean-cert step in the standard clean target.

@vszakats vszakats changed the title GHA: try generating certs certs: generate in CI, drop pre-built cert files/binaries Mar 25, 2025
@icing
Copy link
Contributor

icing commented Mar 25, 2025

Excellent!

@vszakats
Copy link
Member Author

vszakats commented Mar 25, 2025

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:

gmake[2]: Leaving directory '/home/runner/work/curl/curl/bld'
[ 33%] Built target curl-man
[ 66%] Linking C shared library libcurl.so
cd /home/runner/work/curl/curl/bld/lib && /usr/local/bin/cmake -E cmake_link_script CMakeFiles/libcurl_shared.dir/link.txt --verbose=1
ld.lld: error: undefined symbol: nl_langinfo
>>> referenced by psl.c:1819 (../src/0.21.5-062ce9d927.clean/src/psl.c:1819)
>>>               psl.c.o:(psl_str_to_utf8lower) in archive /usr/local/share/vcpkg/installed/arm64-android/lib/pkgconfig/../../lib/libpsl.a
>>> referenced by localcharset.c:847
>>>               libunistring_la-localcharset.o:(locale_charset) in archive /usr/local/share/vcpkg/installed/arm64-android/lib/pkgconfig/../../lib/libidn2.a

ld.lld: error: undefined symbol: iconv_open
>>> referenced by psl.c:1831 (../src/0.21.5-062ce9d927.clean/src/psl.c:1831)
>>>               psl.c.o:(psl_str_to_utf8lower) in archive /usr/local/share/vcpkg/installed/arm64-android/lib/pkgconfig/../../lib/libpsl.a
>>> referenced by striconveh.c:64 (.././../src/v1.2-f6a8d99457.clean/lib/striconveh.c:64)
>>>               libunistring_la-striconveh.o:(libunistring_iconveh_open) in archive /usr/local/share/vcpkg/installed/arm64-android/lib/pkgconfig/../../lib/libunistring.a
>>> referenced by striconveh.c:70 (.././../src/v1.2-f6a8d99457.clean/lib/striconveh.c:70)
>>>               libunistring_la-striconveh.o:(libunistring_iconveh_open) in archive /usr/local/share/vcpkg/installed/arm64-android/lib/pkgconfig/../../lib/libunistring.a
>>> referenced 1 more times

ld.lld: error: undefined symbol: iconv
>>> referenced by psl.c:1842 (../src/0.21.5-062ce9d927.clean/src/psl.c:1842)
>>>               psl.c.o:(psl_str_to_utf8lower) in archive /usr/local/share/vcpkg/installed/arm64-android/lib/pkgconfig/../../lib/libpsl.a
>>> referenced by psl.c:1843 (../src/0.21.5-062ce9d927.clean/src/psl.c:1843)
>>>               psl.c.o:(psl_str_to_utf8lower) in archive /usr/local/share/vcpkg/installed/arm64-android/lib/pkgconfig/../../lib/libpsl.a
>>> referenced by striconveh.c:414 (.././../src/v1.2-f6a8d99457.clean/lib/striconveh.c:414)
>>>               libunistring_la-striconveh.o:(mem_cd_iconveh_internal) in archive /usr/local/share/vcpkg/installed/arm64-android/lib/pkgconfig/../../lib/libunistring.a
>>> referenced 15 more times

ld.lld: error: undefined symbol: iconv_close
>>> referenced by psl.c:1867 (../src/0.21.5-062ce9d927.clean/src/psl.c:1867)
>>>               psl.c.o:(psl_str_to_utf8lower) in archive /usr/local/share/vcpkg/installed/arm64-android/lib/pkgconfig/../../lib/libpsl.a
>>> referenced by striconveh.c:0 (.././../src/v1.2-f6a8d99457.clean/lib/striconveh.c:0)
>>>               libunistring_la-striconveh.o:(libunistring_iconveh_open) in archive /usr/local/share/vcpkg/installed/arm64-android/lib/pkgconfig/../../lib/libunistring.a
>>> referenced by striconveh.c:97 (.././../src/v1.2-f6a8d99457.clean/lib/striconveh.c:97)
>>>               libunistring_la-striconveh.o:(libunistring_iconveh_open) in archive /usr/local/share/vcpkg/installed/arm64-android/lib/pkgconfig/../../lib/libunistring.a
>>> referenced 6 more times

ld.lld: error: undefined symbol: stderr
>>> referenced by nghttp2_map.c:102 (/usr/local/share/vcpkg/buildtrees/nghttp2/src/v1.65.0-395188afe2.clean/lib/nghttp2_map.c:102)
>>>               nghttp2_map.c.o:(nghttp2_map_print_distance) in archive /usr/local/share/vcpkg/installed/arm64-android/lib/pkgconfig/../../lib/libnghttp2.a
>>> referenced by nghttp2_map.c:102 (/usr/local/share/vcpkg/buildtrees/nghttp2/src/v1.65.0-395188afe2.clean/lib/nghttp2_map.c:102)
>>>               nghttp2_map.c.o:(nghttp2_map_print_distance) in archive /usr/local/share/vcpkg/installed/arm64-android/lib/pkgconfig/../../lib/libnghttp2.a
>>> referenced by ui_openssl.c:402 (../src/nssl-3.4.1-9e512b8cf5.clean/crypto/ui/ui_openssl.c:402)
>>>               libcrypto-lib-ui_openssl.o:(open_console) in archive /usr/local/share/vcpkg/installed/arm64-android/lib/pkgconfig/../../lib/libcrypto.a
>>> referenced 15 more times

ld.lld: error: undefined symbol: __memchr_chk
>>> referenced by string.h:153 (/usr/local/lib/android/sdk/ndk/27.2.12479018/toolchains/llvm/prebuilt/linux-x86_64/sysroot/usr/include/bits/fortify/string.h:153)
>>>               libssl-lib-statem_srvr.o:(tls_early_post_process_client_hello) in archive /usr/local/share/vcpkg/installed/arm64-android/debug/lib/libssl.a
>>> referenced by string.h:153 (/usr/local/lib/android/sdk/ndk/27.2.12479018/toolchains/llvm/prebuilt/linux-x86_64/sysroot/usr/include/bits/fortify/string.h:153)
>>>               libssl-lib-statem_lib.o:(ssl_has_cert_type) in archive /usr/local/share/vcpkg/installed/arm64-android/debug/lib/libssl.a
>>> referenced by string.h:153 (/usr/local/lib/android/sdk/ndk/27.2.12479018/toolchains/llvm/prebuilt/linux-x86_64/sysroot/usr/include/bits/fortify/string.h:153)
>>>               libssl-lib-extensions_srvr.o:(PACKET_contains_zero_byte) in archive /usr/local/share/vcpkg/installed/arm64-android/debug/lib/libssl.a
>>> referenced 6 more times

ld.lld: error: undefined symbol: __poll_chk
>>> referenced by poll.h:59 (/usr/local/lib/android/sdk/ndk/27.2.12479018/toolchains/llvm/prebuilt/linux-x86_64/sysroot/usr/include/bits/fortify/poll.h:59)
>>>               libssl-lib-quic_reactor.o:(poll_two_fds) in archive /usr/local/share/vcpkg/installed/arm64-android/debug/lib/libssl.a
>>> referenced by poll.h:59 (/usr/local/lib/android/sdk/ndk/27.2.12479018/toolchains/llvm/prebuilt/linux-x86_64/sysroot/usr/include/bits/fortify/poll.h:59)
>>>               session.c.o:(libssh2_poll) in archive /usr/local/share/vcpkg/installed/arm64-android/lib/pkgconfig/../../lib/libssh2.a

ld.lld: error: undefined symbol: strchrnul
>>> referenced by lookup.c:561
>>>               lookup.o:(idn2_lookup_u8) in archive /usr/local/share/vcpkg/installed/arm64-android/lib/pkgconfig/../../lib/libidn2.a

ld.lld: error: undefined symbol: __write_chk
>>> referenced by unistd.h:202 (/usr/local/lib/android/sdk/ndk/27.2.12479018/toolchains/llvm/prebuilt/linux-x86_64/sysroot/usr/include/bits/fortify/unistd.h:202)
>>>               libcrypto-lib-bss_dgram.o:(dgram_write) in archive /usr/local/share/vcpkg/installed/arm64-android/lib/pkgconfig/../../lib/libcrypto.a
>>> referenced by unistd.h:202 (/usr/local/lib/android/sdk/ndk/27.2.12479018/toolchains/llvm/prebuilt/linux-x86_64/sysroot/usr/include/bits/fortify/unistd.h:202)
>>>               libcrypto-lib-bss_conn.o:(conn_write) in archive /usr/local/share/vcpkg/installed/arm64-android/lib/pkgconfig/../../lib/libcrypto.a
>>> referenced by unistd.h:202 (/usr/local/lib/android/sdk/ndk/27.2.12479018/toolchains/llvm/prebuilt/linux-x86_64/sysroot/usr/include/bits/fortify/unistd.h:202)
>>>               libcrypto-lib-bss_sock.o:(sock_write) in archive /usr/local/share/vcpkg/installed/arm64-android/lib/pkgconfig/../../lib/libcrypto.a
>>> referenced 1 more times

ld.lld: error: undefined symbol: stdin
>>> referenced by ui_openssl.c:400 (../src/nssl-3.4.1-9e512b8cf5.clean/crypto/ui/ui_openssl.c:400)
>>>               libcrypto-lib-ui_openssl.o:(open_console) in archive /usr/local/share/vcpkg/installed/arm64-android/lib/pkgconfig/../../lib/libcrypto.a
>>> referenced by ui_openssl.c:400 (../src/nssl-3.4.1-9e512b8cf5.clean/crypto/ui/ui_openssl.c:400)
>>>               libcrypto-lib-ui_openssl.o:(open_console) in archive /usr/local/share/vcpkg/installed/arm64-android/lib/pkgconfig/../../lib/libcrypto.a
>>> referenced by ui_openssl.c:556 (../src/nssl-3.4.1-9e512b8cf5.clean/crypto/ui/ui_openssl.c:556)
>>>               libcrypto-lib-ui_openssl.o:(close_console) in archive /usr/local/share/vcpkg/installed/arm64-android/lib/pkgconfig/../../lib/libcrypto.a
>>> referenced 1 more times

ld.lld: error: undefined symbol: __fwrite_chk
>>> referenced by stdio.h:120 (/usr/local/lib/android/sdk/ndk/27.2.12479018/toolchains/llvm/prebuilt/linux-x86_64/sysroot/usr/include/bits/fortify/stdio.h:120)
>>>               knownhost.c.o:(libssh2_knownhost_writefile) in archive /usr/local/share/vcpkg/installed/arm64-android/lib/pkgconfig/../../lib/libssh2.a
clang: error: linker command failed with exit code 1 (use -v to see invocation)
/usr/local/lib/android/sdk/ndk/27.2.12479018/toolchains/llvm/prebuilt/linux-x86_64/bin/clang --target=aarch64-none-linux-android21 --sysroot=/usr/local/lib/android/sdk/ndk/27.2.12479018/toolchains/llvm/prebuilt/linux-x86_64/sysroot -fPIC -g -DANDROID -fdata-sections -ffunction-sections -funwind-tables -fstack-protector-strong -no-canonical-prefixes -D_FORTIFY_SOURCE=2 -Wformat -Werror=format-security  -pedantic-errors -Werror-implicit-function-declaration -Wextra -Wall -pedantic -Wbad-function-cast -Wconversion -Wmissing-declarations -Wmissing-prototypes -Wnested-externs -Wno-long-long -Wno-multichar -Wpointer-arith -Wshadow -Wsign-compare -Wundef -Wunused -Wwrite-strings -Waddress -Wattributes -Wcast-align -Wcast-qual -Wdeclaration-after-statement -Wdiv-by-zero -Wempty-body -Wendif-labels -Wfloat-equal -Wformat-security -Wignored-qualifiers -Wmissing-field-initializers -Wmissing-noreturn -Wno-format-nonliteral -Wno-sign-conversion -Wno-system-headers -Wold-style-definition -Wredundant-decls -Wstrict-prototypes -Wtype-limits -Wunreachable-code -Wunused-parameter -Wvla -Wshift-sign-overflow -Wshorten-64-to-32 -Wformat=2 -Wlanguage-extension-token -Wdouble-promotion -Wenum-conversion -Wheader-guard -Wpragmas -Wsometimes-uninitialized -Wunused-const-variable -Wcomma -Wmissing-variable-declarations -Wassign-enum -Wextra-semi-stmt -Wimplicit-fallthrough -I/usr/local/share/vcpkg/installed/arm64-android/lib/pkgconfig/../../include -I/usr/local/share/vcpkg/installed/arm64-android/lib/pkgconfig/../../include -I/usr/local/share/vcpkg/installed/arm64-android/lib/pkgconfig/../../include -I/usr/local/share/vcpkg/installed/arm64-android/lib/pkgconfig/../../include -I/usr/local/share/vcpkg/installed/arm64-android/lib/pkgconfig/../../include -I/usr/local/share/vcpkg/installed/arm64-android/lib/pkgconfig/../../include -Werror -Xlinker --dependency-file=CMakeFiles/libcurl_shared.dir/link.d -static-libstdc++ -Wl,--build-id=sha1 -Wl,--no-rosegment -Wl,--no-undefined-version -Wl,--fatal-warnings -Wl,--no-undefined -Qunused-arguments  -shared -Wl,-soname,libcurl.so -o libcurl.so CMakeFiles/libcurl_shared.dir/Unity/unity_0_c.c.o   -L/usr/local/share/vcpkg/installed/arm64-android/lib/pkgconfig/../../lib  -lssh2 -lcrypto -ldl -lz -lidn2 -lunistring -pthread /usr/local/share/vcpkg/installed/arm64-android/debug/lib/libssl.a /usr/local/share/vcpkg/installed/arm64-android/debug/lib/libcrypto.a /usr/local/share/vcpkg/installed/arm64-android/lib/libz.a -lbrotlidec -lbrotlicommon -lzstd -lnghttp2 -lpsl -lidn2 -lunistring -lbrotlidec -lbrotlicommon -lzstd -lnghttp2 -lpsl -ldl -pthread -latomic -lm
gmake[2]: *** [lib/CMakeFiles/libcurl_shared.dir/build.make:104: lib/libcurl.so] Error 1
gmake[1]: *** [CMakeFiles/Makefile2:826: lib/CMakeFiles/libcurl_shared.dir/all] Error 2
gmake: *** [Makefile:156: all] Error 2

https://github.com/curl/curl/actions/runs/14058024728/job/39361854217?pr=16824#step:11:81

(gone after a rebuild)

@vszakats
Copy link
Member Author

vszakats commented Mar 25, 2025

Hah, the question now is how to tell Makefile.am not to parallelize building leaf certs (via genserv.sh).
One crude way is just to make it a single rule (as in CMake). Going with that.

https://github.com/curl/curl/actions/runs/14059113100/job/39365322136?pr=16824#step:39:22

@vszakats vszakats changed the title certs: generate in CI, drop pre-built cert files/binaries certs: replace blobs with dynamic generation, bump to EC256, tidy-ups Mar 25, 2025
@vszakats
Copy link
Member Author

vszakats commented Mar 25, 2025

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.

make clean might need updates. I haven't looked into it much, as it's pretty opaque to me. Help much welcome!

edit: dependency handling and local use may also need updates to limit re-runs only when necessary. I haven't tested.

@vszakats
Copy link
Member Author

vszakats commented Mar 25, 2025

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 gen directory and copying files around, a not leaving gen on disk after clean.

(Though generating them is quite quick, and the log is just a few lines now.)

autotools is still to be tested.

@vszakats
Copy link
Member Author

Automake, cmake, local, repeated builds are covered now.

This is ready to merge. Likely fit for RC3 or the release.

@vszakats vszakats changed the title certs: replace blobs with dynamic generation, bump to EC256, tidy-ups certs: generate dynamically, bump to EC256, tidy-ups Mar 25, 2025
@vszakats vszakats changed the title certs: generate dynamically, bump to EC256, tidy-ups certs: generate dynamically, bump to EC-256, tidy up Mar 25, 2025
@vszakats vszakats changed the title certs: generate dynamically, bump to EC-256, tidy up tests: generate certs dynamically, bump to EC-256, tidy up Mar 26, 2025
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:arm64-android@1.18#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 vszakats changed the title tests: generate certs dynamically, bump to EC-256, tidy up runtests: generate certs dynamically, bump to EC-256, tidy up Mar 26, 2025
Copy link
Member

@bagder bagder left a comment

Choose a reason for hiding this comment

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

I think getting rid of those big "magic" files from git is a huge benefit, let's do this!

@vszakats vszakats closed this in 44341e7 Mar 27, 2025
@vszakats vszakats deleted the cicert branch March 27, 2025 09:24
@vszakats
Copy link
Member Author

This is merged now!

Does anybody know how to generate srp-verifier-conf and srp-verifier-db for TLS-SRP? via 59cf93c (in 2011) Something openssl srp?

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
Copy link
Member Author

The obvious just hit me: We might rewrite genserv.sh in Perl
to avoid the bash dependency. → #16858

As for SRP, the GnuTLS srptool might be the answer. GnuTLS
disabled SRP support in 3.8.0 (2023-02-09), and it's no longer
packaged by Debian and Ubuntu (and Homebrew):
https://www.gnutls.org/manual/html_node/srptool-Invocation.html
https://packages.ubuntu.com/jammy/gnutls-bin
https://gitlab.com/gnutls/gnutls/blob/master/NEWS

We may create a generatotr script, but running it dynamically will
not work. TLS-SRP is also insecure, so perhaps curl may officially
deprecate it too?

@bagder
Copy link
Member

bagder commented Mar 28, 2025

so perhaps curl may officially deprecate it too?

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 Mar 29, 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 #16824
Closes #16858
vszakats added a commit that referenced this pull request Apr 3, 2025
Reported-by: Tomas Volf
Fixes #16926
Follow-up to 44341e7 #16824
Ref: #16928
Co-authored-by: Daniel Stenberg
Closes #16929
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI Continuous Integration tests TLS
Development

Successfully merging this pull request may close these issues.

3 participants