Skip to content

crypto: switch rdkafka from OpenSSL to AWS-LC (ssl-awslc) - #35941

Closed
jasonhernandez wants to merge 3 commits into
mainfrom
jh/crypto-pr4-rdkafka-awslc
Closed

jasonhernandez wants to merge 3 commits into
mainfrom
jh/crypto-pr4-rdkafka-awslc

Conversation

@jasonhernandez

@jasonhernandez jasonhernandez commented Apr 10, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Switch rdkafka from ssl-vendored (OpenSSL) to ssl-awslc (AWS-LC) using MaterializeInc fork
  • CI: Add AWS-LC build to Docker builder image (Go + ninja + cmake)
  • CI: Simplify macOS clippy (fork auto-builds AWS-LC when /opt/aws-lc not present)
  • Update Kafka TLS and MySQL CDC test expectations

Part 4 of 7 in the crypto migration from native-tls/OpenSSL to rustls/aws-lc-rs.
Depends on PR1 (#35940).

Test plan

  • cargo check --workspace passes
  • cargo build -p rdkafka-sys links successfully
  • Kafka auth tests pass in CI

🤖 Generated with Claude Code

Part of SEC-237.

@github-actions

Copy link
Copy Markdown
Contributor

Thanks for opening this PR! Here are a few tips to help make the review process smooth for everyone.

PR title guidelines

  • Use imperative mood: "Fix X" not "Fixed X" or "Fixes X"
  • Be specific: "Fix panic in catalog sync when controller restarts" not "Fix bug" or "Update catalog code"
  • Prefix with area if helpful: compute: , storage: , adapter: , sql:

Pre-merge checklist

  • The PR title is descriptive and will make sense in the git log.
  • This PR has adequate test coverage / QA involvement has been duly considered. (trigger-ci for additional test/nightly runs)
  • If this PR includes major user-facing behavior changes, I have pinged the relevant PM to schedule a changelog post.
  • This PR has an associated up-to-date design doc, is a design doc (template), or is sufficiently small to not require a design.
  • If this PR evolves an existing $T ⇔ Proto$T mapping (possibly in a backwards-incompatible way), then it is tagged with a T-proto label.
  • If this PR will require changes to cloud orchestration or tests, there is a companion cloud PR to account for those changes that is tagged with the release-blocker label (example).

jasonhernandez and others added 3 commits July 7, 2026 01:31
Add rustls, aws-lc-rs, hyper-rustls, rcgen, tokio-rustls workspace deps
and the mz-ore `crypto` feature with FIPS-aware CryptoProvider helpers.
Azure SDK forks patched to support reqwest-rustls-no-provider.
Removes the rustls ban from deny.toml.

Does NOT bump LaunchDarkly SDK (deferred to end of migration).

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Switch rdkafka from ssl-vendored (vendored OpenSSL) to ssl-awslc
(AWS-LC) using the MaterializeInc fork. AWS-LC provides FIPS 140-3
validation and eliminates the vendored OpenSSL dependency.

- Workspace: rdkafka/rdkafka-sys fork → jasonhernandez/ssl-awslc branch
- kafka-util: rdkafka features ssl-vendored → ssl-awslc
- CI: Add Go, ninja-build, build AWS-LC static library in Docker builder
- CI: Simplify macOS clippy (fork auto-builds when /opt/aws-lc absent)
- Update Kafka TLS and MySQL CDC test expectations

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Adapt the rdkafka -> AWS-LC branch to current main after a ~1155-commit
rebase:

- Cargo.lock: regenerated via `cargo metadata` from the merged manifest.
  Only source redirects, no version bumps: rdkafka/rdkafka-sys now pin the
  `jasonhernandez/ssl-awslc` fork branch, and the five azure-sdk crates pin
  the `mz/enable-reqwest-rustls-no-provider` fork branch.

- src/ore/Cargo.toml: drop `default-features = false` on the inherited
  `rustls` dependency. Main's workspace root declares `rustls = "0.23.38"`
  with default features (already aws_lc_rs, no ring), and orchestratord
  relies on them. Cargo forbids a member from disabling default features the
  workspace enables, so mz-ore now inherits them and just adds `aws_lc_rs`.

- deny.toml: keep only the duplicate-version skips that current main's tree
  actually hits via the azure fork (core-foundation, openssl-probe,
  security-framework). Drop the stale skips and the unused hyper-rustls
  wrapper that no longer match any crate.

- test/kafka-auth schema-registry and test/mysql-cdc TLS tests: revert to
  main. Those paths still use reqwest/native-tls and mysql_async/native-tls
  in this branch, so their rustls-specific error assertions (UnknownIssuer,
  invalid peer certificate, failed to parse private key PEM) cannot match.
  The genuine librdkafka -> AWS-LC error-string changes are retained in the
  four test-kafka-*.td files.

Co-Authored-By: Claude Fable 5 <[email protected]>
@jasonhernandez
jasonhernandez force-pushed the jh/crypto-pr4-rdkafka-awslc branch from cc6c732 to 443a34d Compare July 7, 2026 08:45
jasonhernandez added a commit that referenced this pull request Jul 7, 2026
… 8.x) (#37470)

## Summary

Re-enables the Kafka auth test suite, disabled since #36405 (SS-115),
and fixes the two breakages that accumulated while it was dark.

## What broke while the suite was disabled

The Confluent Platform 7.9.4 → 8.2.0 bump (#36683) landed three weeks
after the suite was skipped, and broke its setup in two ways nothing
could catch:

1. **Schema registry basic auth returned 401 for everyone.** CP 8.x
ships Jetty 12, which moved the JAAS login modules from
`org.eclipse.jetty.jaas.spi` to `org.eclipse.jetty.security.jaas.spi`.
Our `schema-registry.jaas.config` referenced the old class, so the login
module never loaded and every request, correct credentials included, got
401. Verified directly: `curl -u materialize:sekurity` against the
container → 401 before, 200 after the one-line class-path fix
(wrong/missing creds still 401).
2. **Kafka mssl negative tests asserted TLS 1.2 alert text.** CP 8.x
brokers negotiate TLS 1.3, where a missing/untrusted client certificate
fails with `tlsv13 alert certificate required` (alert 116) instead of
`ssl/tls alert bad certificate` (alert 42). The assertions now check
only that the broker rejected the connection with a TLS alert. The exact
wording is a function of TLS version and OpenSSL build that we do not
control. This is the third exact-text loosening in this suite's history
(#30501, #36232), so the loosening deliberately goes all the way rather
than chasing the new strings.

## On the original SS-115 flake

The `PEM routines:get_name:no start line` flake that got the suite
disabled did not reproduce in local full-suite runs against current main
images (reqwest 0.12) or against the reqwest 0.13.4 branch (#37469).
Timeline evidence from SS-115 itself shows the flake continued after the
reqwest 0.13 revert (#36241), so reqwest was likely misattributed. If it
resurfaces, the `ci-regexp` in SS-115 will link it; the suite being
enabled is what gives us the data to root-cause it.

## Why now

Two in-flight workstreams change exactly the surface this suite covers:
the reqwest 0.13 re-application (#37469) and the rdkafka → AWS-LC switch
(#35941). Landing those with this suite dark is how auth/TLS regressions
reach production unobserved.

## Test plan

- [x] Full suite green locally against main images (all fixes applied)
- [x] Repeated local runs to probe for the SS-115 flake: 8 consecutive
local full-suite passes (5-run loop + 3 individual), plus 2 clean runs
on the reqwest-0.12 main baseline. The PEM flake did not reproduce.
- [ ] Kafka auth 1–3 green in this PR's CI (the un-skip makes them run
here)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Jason Hernandez <[email protected]>
Co-authored-by: Claude Fable 5 <[email protected]>
@jasonhernandez

Copy link
Copy Markdown
Contributor Author

Closing as superseded. This was part of the original 7-part crypto migration series, which was replaced by the additive rustls TLS stack (#36085 and its follow-ups) and the merged primitives migration (#35895). Any remaining pieces will be reopened as fresh, smaller PRs against current main.

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.

1 participant