Skip to content

refactor: migrate crypto-refresh EC/Ed code from Botan native API to FFI - #2447

Closed
ronaldtse wants to merge 1 commit into
mainfrom
refactor/crypto-refresh-botan-ffi
Closed

refactor: migrate crypto-refresh EC/Ed code from Botan native API to FFI#2447
ronaldtse wants to merge 1 commit into
mainfrom
refactor/crypto-refresh-botan-ffi

Conversation

@ronaldtse

Copy link
Copy Markdown
Contributor

Summary

Addresses the Botan 3.12 ENABLE_CRYPTO_REFRESH build breakage by migrating the crypto-refresh EC/Ed code off Botan's unstable native C++ API and onto the stable FFI — the same discipline the rest of rnp already follows (which is why it rides across Botan 2.x/3.x untouched).

The native-API code bit-rotted on 3.12: tightened include hygiene made Botan::EC_Group / EC_AffinePoint / BigInt incomplete types, and public_point() + the Ed25519_PrivateKey(span) ctor are deprecated.

Migrated files (now contain zero Botan:: references):

  • src/lib/crypto/ec.cppec_generate_generic_native mirrors the existing FFI keygen in Key::generate() (botan_privkey_create + get_field); native bigint.h/ecdh.h includes dropped.
  • src/lib/crypto/ed25519_ed448.cpp — Ed25519/Ed448 keygen, sign, verify, validate via botan_privkey_create/load_*, botan_pk_op_sign_*/verify_*, botan_*_check_key.
  • src/lib/crypto/exdsa_ecdhkem.{cpp,h} — replaced the Botan::* key-object helpers with FFI key handles; ECDH KEM via botan_pk_op_key_agreement_*, ECDSA sign/verify, check_key, SEC1 loaders. Private key_ member moved from Botan::secure_vector to rnp::secure_bytes; all <botan/*> includes removed from the header.

FFI gap audit (for discussion with Jack Lloyd)

TODO.refactor/botan-ffi-migration.md documents a full audit: no Botan FFI gaps were found. Every native op rnp uses (ECDH/ECDSA/Ed25519/Ed448/X25519/X448 keygen+load, key agreement, KEM, sign/verify, check_key, ML-DSA/ML-KEM/SLH-DSA loaders, RFC 3394 keywrap) has a stable FFI equivalent. Nothing needs to be requested upstream. The doc also lays out the Phase 2 roadmap (migrate the remaining native-API files: x25519_x448.cpp, dilithium*, kyber*, sphincsplus.cpp).

Test plan

  • cmake -DCRYPTO_BACKEND=botan3 -DENABLE_CRYPTO_REFRESH=ON builds clean on Botan 3.12 (native-API errors gone).
  • Full rnp_tests suite passes: 280 tests (v6 SKESK/sig/import, ECDSA keygen, Ed25519/Ed448, ECDH, CLI examples).
  • All four migrated files contain zero Botan:: references.

Notes / follow-ups

  • ed25519_ed448.cpp still calls the deprecated botan_privkey_ed25519_get_privkey / botan_*_ed448_get_* getters (consistent with the existing eddsa.cpp on main). These are the same -Wdeprecated-declarations class that PR fix: silence -Wnonnull-compare and Botan FFI deprecation warnings #2446 migrates to botan_*_view_raw; that migration should be extended to these three new call sites (tracked in the roadmap doc).
  • Phase 2 (the remaining native-API files) is intentionally out of scope for this PR.

The crypto-refresh paths used Botan's unstable native C++ API, which broke
the ENABLE_CRYPTO_REFRESH build against Botan 3.12 (tightened include
hygiene made EC_Group/EC_AffinePoint/BigInt incomplete, and public_point()
and the Ed25519_PrivateKey(span) ctor are deprecated). Move these three
files onto the stable FFI, matching the rest of rnp:

- ec.cpp: ec_generate_generic_native now mirrors the existing FFI keygen in
  Key::generate (botan_privkey_create + get_field). Drops the native
  bigint.h/ecdh.h includes.
- ed25519_ed448.cpp: Ed25519/Ed448 keygen, sign, verify, and validate moved
  to FFI (botan_privkey_create/load_*, botan_pk_op_sign_*/verify_*,
  botan_*_check_key).
- exdsa_ecdhkem.cpp/.h: replaced the Botan key-object helpers with FFI key
  handles; ECDH KEM via botan_pk_op_key_agreement_*, ECDSA sign/verify,
  check_key, and SEC1 loaders (botan_pubkey_load_ecdh_sec1/_ecdsa_sec1).
  The private key_ member moves from Botan::secure_vector to
  rnp::secure_bytes and all botan includes are removed from the header.

A gap audit found no missing FFI entry points (documented in
TODO.refactor/botan-ffi-migration.md, along with the Phase 2 roadmap for the
remaining native-API files).

Validated: ENABLE_CRYPTO_REFRESH=ON builds clean on Botan 3.12; full rnp_tests
suite passes (280 tests).
@codecov

codecov Bot commented Jul 29, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.38%. Comparing base (975b67c) to head (a77b894).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2447   +/-   ##
=======================================
  Coverage   85.37%   85.38%           
=======================================
  Files         126      126           
  Lines       22790    22787    -3     
=======================================
- Hits        19458    19456    -2     
+ Misses       3332     3331    -1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ronaldtse

Copy link
Copy Markdown
Contributor Author

Consolidating this into #2448, which rebases the same FFI migration on top of kaie's pqc-openssl (PR #2392). That's the better base because #2392 restructures exdsa_ecdhkem.{cpp,h} (backend split), which conflicts with the exdsa change carried here.

#2448 completes the full migration (all crypto-refresh/PQC Botan-side files, not just Phase 1), fixes the Botan 3.12 build on kaie's branch too, and passes 296/296 tests including all PQC tests. Closing this in favor of #2448.

@ronaldtse ronaldtse closed this Jul 30, 2026
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