Skip to content

Refs #39518 - Pin tls_ciphers to OpenSSL defaults in HTTPS integration tests - #954

Merged
adamruzicka merged 1 commit into
theforeman:developfrom
ogajduse:test/pin-tls-ciphers-in-https-integration-tests
Jul 14, 2026
Merged

Refs #39518 - Pin tls_ciphers to OpenSSL defaults in HTTPS integration tests#954
adamruzicka merged 1 commit into
theforeman:developfrom
ogajduse:test/pin-tls-ciphers-in-https-integration-tests

Conversation

@ogajduse

Copy link
Copy Markdown
Member

Problem

smart-proxy-develop-source-release's ruby=3.0.4 CI leg fails SSLClientVerificationIntegrationTest because it launches a real WEBrick HTTPS server without setting tls_ciphers, so it falls through to launcher.rb's crypto-policies auto-detection and inherits whatever the host's OpenSSL build happens to support. On that leg, rbenv/ruby-build vendors a plain openssl-1.1.1w that doesn't understand the 'PROFILE=SYSTEM' cipher-list alias, even though the host's crypto-policies files are genuinely present — so the test fails with a RuntimeError before ever reaching the SSL client verification behavior it's actually meant to check. Root cause: https://projects.theforeman.org/issues/39518

Fix

Pin tls_ciphers to '' (OpenSSL defaults, no cipher restriction) by default for HTTPS integration tests in test_helper.rb's shared launch method — one place, so any current or future HTTPS integration test gets this automatically, not just this one file. A test can still override tls_ciphers explicitly via its own settings: hash if it wants to exercise a specific cipher value.

Auto-detection of tls_ciphers itself is already covered deterministically by the mocked unit tests in test/launcher_test.rb (File.exist?/OpenSSL probes are stubbed there) — this integration test only needs a working TLS handshake to check client-cert verification, not any particular cipher policy, so it shouldn't be coupled to the host's crypto-policies/OpenSSL combination at all.

This is a CI-only workaround to unblock the pipeline. It does not touch production code or resolve_tls_ciphers's behavior — see #39518 / a follow-up PR for the underlying robustness fix in launcher.rb itself.

Testing

  • Verified against the unpatched develop launcher.rb (i.e. without any other fix applied): SSLClientVerificationIntegrationTest goes from 6 errors → 4/4 passing.
  • Full suite (bundle exec rake test, excluding bmc/libvirt which need unrelated native system libs): 869/869 pass.

…n tests

SSLClientVerificationIntegrationTest launches a real WEBrick HTTPS server
without setting tls_ciphers, so it falls through to launcher.rb's
crypto-policies auto-detection and inherits whatever the host's OpenSSL
build happens to support. On a host where /etc/crypto-policies files are
present but the linked OpenSSL doesn't understand the 'PROFILE=SYSTEM'
cipher-list alias (e.g. a vendored, non-RHEL-patched OpenSSL), the test
fails with a RuntimeError before ever reaching the actual SSL client
verification behavior it's meant to check.

Pin tls_ciphers to '' (OpenSSL defaults, no restriction) by default for
HTTPS integration tests in test_helper.rb, so this test suite doesn't
depend on the host's crypto-policies/OpenSSL combination. Auto-detection
itself is already covered deterministically by the mocked unit tests in
test/launcher_test.rb; this integration test only needs a working TLS
handshake, not any particular cipher policy.

This is a CI-only workaround; see #39518 for the underlying
resolve_tls_ciphers robustness fix.
@ogajduse
ogajduse force-pushed the test/pin-tls-ciphers-in-https-integration-tests branch from d3b2ff8 to ca74355 Compare July 14, 2026 10:56
@adamruzicka
adamruzicka merged commit d2663e2 into theforeman:develop Jul 14, 2026
10 checks passed
@adamruzicka

Copy link
Copy Markdown
Contributor

Thank you @ogajduse !

@ogajduse
ogajduse deleted the test/pin-tls-ciphers-in-https-integration-tests branch July 14, 2026 16:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants