Repository navigation
feat(libSceLibreSsl3): implement EVP_CIPHER_nid, BN_is_negative and RSA_bits - #14008
Appelschorle187 wants to merge 11 commits into
Conversation
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013rhFWQL7oZRJSiGdkL5Gkn
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013rhFWQL7oZRJSiGdkL5Gkn
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013rhFWQL7oZRJSiGdkL5Gkn
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013rhFWQL7oZRJSiGdkL5Gkn
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013rhFWQL7oZRJSiGdkL5Gkn
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013rhFWQL7oZRJSiGdkL5Gkn
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013rhFWQL7oZRJSiGdkL5Gkn
…d moduli Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013rhFWQL7oZRJSiGdkL5Gkn
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013rhFWQL7oZRJSiGdkL5Gkn
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013rhFWQL7oZRJSiGdkL5Gkn
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013rhFWQL7oZRJSiGdkL5Gkn
MarcZX10
left a comment
There was a problem hiding this comment.
Tests run and reviewed — approval withheld pending PS5 RSA layout evidence
The implementations match the pinned LibreSSL source model, but the PR leaves the RSA.n layout unverified for the console module and RSA_bits dereferences that field.
What I ran
Reviewed head c6ee4cf150339432043d141947c859c3fd034099 against base 474030e58424fefdb65e5df9f3820810fbb4553c; the exact head has a clean individual merge preview. Environment: Debian Linux 6.1 x86_64, Python 3.11.2, GCC 12.2.0. No project build or C++ test run.
python3 tools/pr_precheck.py --repo boykopovar/AnyPS5 --pr 14008
git diff --check 474030e58424fefdb65e5df9f3820810fbb4553c...c6ee4cf150339432043d141947c859c3fd034099
python3 tools/nid_names.py --name EVP_CIPHER_nid --name BN_is_negative --name RSA_bits
git worktree add --detach /tmp/anyps5-review-14008 c6ee4cf150339432043d141947c859c3fd034099
python3 tools/pr_test_quality.py --root /tmp/anyps5-review-14008 --json core/libs/prx/libSceLibreSsl3/tests/Bignum.cpp core/libs/prx/libSceLibreSsl3/tests/Cipher.cpp core/libs/prx/libSceLibreSsl3/tests/Rsa.cpp
git worktree remove /tmp/anyps5-review-14008
python3 tools/pr_abi_audit.py --repo . --base 474030e58424fefdb65e5df9f3820810fbb4553c --head c6ee4cf150339432043d141947c859c3fd034099 --json
curl -fsSL https://raw.githubusercontent.com/libressl/openbsd/ce488b78e0bd994780569df718596b9776ff681a/src/lib/libcrypto/rsa/rsa_eay.c | grep -n -A3 -B2 'RSA_bits'
curl -fsSL https://raw.githubusercontent.com/libressl/openbsd/ce488b78e0bd994780569df718596b9776ff681a/src/lib/libcrypto/rsa/rsa_lib.c | grep -n -A3 -B2 'RSA_bits'
gh pr checks 14008 --repo boykopovar/AnyPS5I also inspected the exact CMake test registration and the pinned LibreSSL source files cited by the PR.
Results
| Check | Result |
|---|---|
| Diff check | Passed; no output. |
| Conventions and PR template | Passed on the exact head. |
| Name/NID mapping | All three names match tools/nid_names.py. |
| Export/declaration audit | The three new exports each have one header declaration and one definition. Warnings are the former Unknown_* placeholders removed as the same NIDs are renamed by this PR. |
| Test registration | The heuristic flagged all three test paths as possibly unregistered. Manual inspection confirms they are registered by the library's CMake file; the heuristic does not resolve ${CMAKE_CURRENT_SOURCE_DIR} paths. |
| Precheck helper availability | The exact PR tree predates pr_abi_audit.py and pr_test_quality.py, so the precheck could not invoke those helpers from that checkout. I ran the current local helpers against the exact PR commits instead. |
| Hosted CI | Conventions, intake, and labels passed. No build or C++ test job was present in the check snapshot. |
| Author-reported tests | The PR body reports 9/9 focused Linux tests passed, including the new cipher, bignum, and RSA tests. I did not independently run them. |
| Local build/tests | Not run. |
Unedited relevant output
NID mapping:
NAME NID
EVP_CIPHER_nid aLdgrI2jAF0
BN_is_negative AiNud8LR8B4
RSA_bits 3KZ-L4y1gak
ABI audit, filtered to the three new symbols and its findings:
{
"status": "passed_with_warnings",
"symbols": [
{
"name": "BN_is_negative_nid_postfix",
"computed_nid": "AiNud8LR8B4",
"declarations": [
"core/libs/prx/libSceLibreSsl3/include/Api.hpp"
],
"definitions": [
"core/libs/prx/libSceLibreSsl3/src/Bignum.cpp"
]
},
{
"name": "EVP_CIPHER_nid_nid_postfix",
"computed_nid": "aLdgrI2jAF0",
"declarations": [
"core/libs/prx/libSceLibreSsl3/include/Api.hpp"
],
"definitions": [
"core/libs/prx/libSceLibreSsl3/src/Cipher.cpp"
]
},
{
"name": "RSA_bits_nid_postfix",
"computed_nid": "3KZ-L4y1gak",
"declarations": [
"core/libs/prx/libSceLibreSsl3/include/Api.hpp"
],
"definitions": [
"core/libs/prx/libSceLibreSsl3/src/Rsa.cpp"
]
}
],
"findings": [
{
"symbol": "Unknown_3KZ_minus_L4y1gak_nid_no_patch",
"kind": "export-removed",
"message": "This APS5_VABI declaration, definition, or explicit alias appeared in the changed base files but not in the corresponding head files; verify that removing it is intentional."
},
{
"symbol": "Unknown_AiNud8LR8B4_nid_no_patch",
"kind": "export-removed",
"message": "This APS5_VABI declaration, definition, or explicit alias appeared in the changed base files but not in the corresponding head files; verify that removing it is intentional."
},
{
"symbol": "Unknown_aLdgrI2jAF0_nid_no_patch",
"kind": "export-removed",
"message": "This APS5_VABI declaration, definition, or explicit alias appeared in the changed base files but not in the corresponding head files; verify that removing it is intentional."
}
]
}Test-quality heuristic:
{
"findings": [
{"path":"core/libs/prx/libSceLibreSsl3/tests/Bignum.cpp","line":1,"kind":"possible-unregistered-cpp-test","message":"Changed C++ test source path was not found literally in CMake files; inspect registration, generated source lists, and workflow coverage."},
{"path":"core/libs/prx/libSceLibreSsl3/tests/Cipher.cpp","line":1,"kind":"possible-unregistered-cpp-test","message":"Changed C++ test source path was not found literally in CMake files; inspect registration, generated source lists, and workflow coverage."},
{"path":"core/libs/prx/libSceLibreSsl3/tests/Rsa.cpp","line":1,"kind":"possible-unregistered-cpp-test","message":"Changed C++ test source path was not found literally in CMake files; inspect registration, generated source lists, and workflow coverage."}
],
"negative_control_candidates": [],
"limits": [
"Heuristics can miss weak assertions and can flag valid tests; inspect every finding.",
"A test that passes against base is not by itself proof that the PR test is inadequate.",
"Negative controls are only candidates until the test actually runs against base and fails by assertion rather than import/setup error."
]
}LibreSSL source lookup:
rsa_eay.c
852-RSA_bits(const RSA *r)
853-{
854- return BN_num_bits(r->n);
855-}
rsa_lib.c
223-RSA_security_bits(const RSA *rsa)
224-{
225- return BN_security_bits(RSA_bits(rsa), -1);
Negative control
| Assertion | Change to code under test | Did the test fail? |
|---|---|---|
| New tests check the reported cipher NID, bignum sign, and RSA modulus bit count. | Restore the prior throwing placeholder exports. | Not exercised; the exact-head C++ tests require a build, which I did not run. |
Findings
Blocking
None confirmed from the inspected source and checks.
Non-blocking
RSA_bitsreadsrsa->nat offset 24. The cited LibreSSL 4.0.0rsa_local.hhas that layout, but the PR's debt entry explicitly says the PS5 console layout is unverified; the other AnyPS5LibreSslRsamodel placesnat offset 32 because it includes anenginefield. This is a material ABI uncertainty, so I am withholding approval rather than treating the upstream layout as console evidence. Please provide evidence that the exactlibSceLibreSsl3RSA object uses offset 24, or keepRSA_bitsthrowing until that layout is established.- The debt entry links the
RSA_bitsbehavior torsa_lib.c, but the function body is in the pinned LibreSSLrsa_eay.c;rsa_lib.conly callsRSA_bitsfromRSA_security_bits. Please correct the source link.
Sources
- The PR's note that the console RSA layout is unverified
- The other AnyPS5 RSA layout, with
nafterengine - The
libSceLibreSsl3model placesnat offset 24 - Pinned LibreSSL 4.0.0 RSA structure
- Pinned LibreSSL
RSA_bitsimplementation - Pinned LibreSSL
BN_is_negativeimplementation - Pinned LibreSSL
EVP_CIPHER_nidimplementation - CMake registrations for the three tests
Not verified
I did not build the PR or run its C++ tests, Windows tests, a title, or console behavior. The 9/9 test result is author-reported. In particular, I could not verify that the console's libSceLibreSsl3 RSA object matches the upstream structure at offset 24.
To get this to a good state
Add evidence for the console RSA layout at offset 24 (or leave that export throwing), and correct the RSA_bits source link. I will reassess the same checks on the updated head.
MarcZX10
left a comment
There was a problem hiding this comment.
Tests run and reviewed — approval withheld pending PS5 RSA layout evidence
The implementations match the pinned LibreSSL source model, but the PR leaves the RSA.n layout unverified for the console module and RSA_bits dereferences that field.
What I ran
Reviewed head c6ee4cf150339432043d141947c859c3fd034099 against base 474030e58424fefdb65e5df9f3820810fbb4553c; the exact head has a clean individual merge preview. Environment: Debian Linux 6.1 x86_64, Python 3.11.2, GCC 12.2.0. No project build or C++ test run.
python3 tools/pr_precheck.py --repo boykopovar/AnyPS5 --pr 14008
git diff --check 474030e58424fefdb65e5df9f3820810fbb4553c...c6ee4cf150339432043d141947c859c3fd034099
python3 tools/nid_names.py --name EVP_CIPHER_nid --name BN_is_negative --name RSA_bits
git worktree add --detach /tmp/anyps5-review-14008 c6ee4cf150339432043d141947c859c3fd034099
python3 tools/pr_test_quality.py --root /tmp/anyps5-review-14008 --json core/libs/prx/libSceLibreSsl3/tests/Bignum.cpp core/libs/prx/libSceLibreSsl3/tests/Cipher.cpp core/libs/prx/libSceLibreSsl3/tests/Rsa.cpp
git worktree remove /tmp/anyps5-review-14008
python3 tools/pr_abi_audit.py --repo . --base 474030e58424fefdb65e5df9f3820810fbb4553c --head c6ee4cf150339432043d141947c859c3fd034099 --json
curl -fsSL https://raw.githubusercontent.com/libressl/openbsd/ce488b78e0bd994780569df718596b9776ff681a/src/lib/libcrypto/rsa/rsa_eay.c | grep -n -A3 -B2 'RSA_bits'
curl -fsSL https://raw.githubusercontent.com/libressl/openbsd/ce488b78e0bd994780569df718596b9776ff681a/src/lib/libcrypto/rsa/rsa_lib.c | grep -n -A3 -B2 'RSA_bits'
gh pr checks 14008 --repo boykopovar/AnyPS5I also inspected the exact CMake test registration and the pinned LibreSSL source files cited by the PR.
Results
| Check | Result |
|---|---|
| Diff check | Passed; no output. |
| Conventions and PR template | Passed on the exact head. |
| Name/NID mapping | All three names match tools/nid_names.py. |
| Export/declaration audit | The three new exports each have one header declaration and one definition. Warnings are the former Unknown_* placeholders removed as the same NIDs are renamed by this PR. |
| Test registration | The heuristic flagged all three test paths as possibly unregistered. Manual inspection confirms they are registered by the library's CMake file; the heuristic does not resolve ${CMAKE_CURRENT_SOURCE_DIR} paths. |
| Precheck helper availability | The exact PR tree predates pr_abi_audit.py and pr_test_quality.py, so the precheck could not invoke those helpers from that checkout. I ran the current local helpers against the exact PR commits instead. |
| Hosted CI | Conventions, intake, and labels passed. No build or C++ test job was present in the check snapshot. |
| Author-reported tests | The PR body reports 9/9 focused Linux tests passed, including the new cipher, bignum, and RSA tests. I did not independently run them. |
| Local build/tests | Not run. |
Unedited relevant output
NID mapping:
NAME NID
EVP_CIPHER_nid aLdgrI2jAF0
BN_is_negative AiNud8LR8B4
RSA_bits 3KZ-L4y1gak
ABI audit, filtered to the three new symbols and its findings:
{
"status": "passed_with_warnings",
"symbols": [
{
"name": "BN_is_negative_nid_postfix",
"computed_nid": "AiNud8LR8B4",
"declarations": [
"core/libs/prx/libSceLibreSsl3/include/Api.hpp"
],
"definitions": [
"core/libs/prx/libSceLibreSsl3/src/Bignum.cpp"
]
},
{
"name": "EVP_CIPHER_nid_nid_postfix",
"computed_nid": "aLdgrI2jAF0",
"declarations": [
"core/libs/prx/libSceLibreSsl3/include/Api.hpp"
],
"definitions": [
"core/libs/prx/libSceLibreSsl3/src/Cipher.cpp"
]
},
{
"name": "RSA_bits_nid_postfix",
"computed_nid": "3KZ-L4y1gak",
"declarations": [
"core/libs/prx/libSceLibreSsl3/include/Api.hpp"
],
"definitions": [
"core/libs/prx/libSceLibreSsl3/src/Rsa.cpp"
]
}
],
"findings": [
{
"symbol": "Unknown_3KZ_minus_L4y1gak_nid_no_patch",
"kind": "export-removed",
"message": "This APS5_VABI declaration, definition, or explicit alias appeared in the changed base files but not in the corresponding head files; verify that removing it is intentional."
},
{
"symbol": "Unknown_AiNud8LR8B4_nid_no_patch",
"kind": "export-removed",
"message": "This APS5_VABI declaration, definition, or explicit alias appeared in the changed base files but not in the corresponding head files; verify that removing it is intentional."
},
{
"symbol": "Unknown_aLdgrI2jAF0_nid_no_patch",
"kind": "export-removed",
"message": "This APS5_VABI declaration, definition, or explicit alias appeared in the changed base files but not in the corresponding head files; verify that removing it is intentional."
}
]
}Test-quality heuristic:
{
"findings": [
{"path":"core/libs/prx/libSceLibreSsl3/tests/Bignum.cpp","line":1,"kind":"possible-unregistered-cpp-test","message":"Changed C++ test source path was not found literally in CMake files; inspect registration, generated source lists, and workflow coverage."},
{"path":"core/libs/prx/libSceLibreSsl3/tests/Cipher.cpp","line":1,"kind":"possible-unregistered-cpp-test","message":"Changed C++ test source path was not found literally in CMake files; inspect registration, generated source lists, and workflow coverage."},
{"path":"core/libs/prx/libSceLibreSsl3/tests/Rsa.cpp","line":1,"kind":"possible-unregistered-cpp-test","message":"Changed C++ test source path was not found literally in CMake files; inspect registration, generated source lists, and workflow coverage."}
],
"negative_control_candidates": [],
"limits": [
"Heuristics can miss weak assertions and can flag valid tests; inspect every finding.",
"A test that passes against base is not by itself proof that the PR test is inadequate.",
"Negative controls are only candidates until the test actually runs against base and fails by assertion rather than import/setup error."
]
}LibreSSL source lookup:
rsa_eay.c
852-RSA_bits(const RSA *r)
853-{
854- return BN_num_bits(r->n);
855-}
rsa_lib.c
223-RSA_security_bits(const RSA *rsa)
224-{
225- return BN_security_bits(RSA_bits(rsa), -1);
Negative control
| Assertion | Change to code under test | Did the test fail? |
|---|---|---|
| New tests check the reported cipher NID, bignum sign, and RSA modulus bit count. | Restore the prior throwing placeholder exports. | Not exercised; the exact-head C++ tests require a build, which I did not run. |
Findings
Blocking
None confirmed from the inspected source and checks.
Non-blocking
RSA_bitsreadsrsa->nat offset 24. The cited LibreSSL 4.0.0rsa_local.hhas that layout, but the PR's debt entry explicitly says the PS5 console layout is unverified; the other AnyPS5LibreSslRsamodel placesnat offset 32 because it includes anenginefield. This is a material ABI uncertainty, so I am withholding approval rather than treating the upstream layout as console evidence. Please provide evidence that the exactlibSceLibreSsl3RSA object uses offset 24, or keepRSA_bitsthrowing until that layout is established.- The debt entry links the
RSA_bitsbehavior torsa_lib.c, but the function body is in the pinned LibreSSLrsa_eay.c;rsa_lib.conly callsRSA_bitsfromRSA_security_bits. Please correct the source link.
Sources
- The PR's note that the console RSA layout is unverified
- The other AnyPS5 RSA layout, with
nafterengine - The
libSceLibreSsl3model placesnat offset 24 - Pinned LibreSSL 4.0.0 RSA structure
- Pinned LibreSSL
RSA_bitsimplementation - Pinned LibreSSL
BN_is_negativeimplementation - Pinned LibreSSL
EVP_CIPHER_nidimplementation - CMake registrations for the three tests
Not verified
I did not build the PR or run its C++ tests, Windows tests, a title, or console behavior. The 9/9 test result is author-reported. In particular, I could not verify that the console's libSceLibreSsl3 RSA object matches the upstream structure at offset 24.
To get this to a good state
Add evidence for the console RSA layout at offset 24 (or leave that export throwing), and correct the RSA_bits source link. I will reassess the same checks on the updated head.
|
@Appelschorle187 Please fix Please replace the forged |
|
Other open pull requests touch the same code. Merging one will break the other, or both implement the same thing. If this one builds on them, list them in Depends on.
Some comparisons could not be completed because a pull request conflicts with the base branch. Review these shared files manually before concluding that there is no overlap.
|
…ments unnamed BN_is_negative, EVP_CIPHER_nid and RSA_bits keep their NID aliases. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
What
Names and implements three libSceLibreSsl3 exports that were placeholders known only by NID. Each name was matched with
compute_nidfromtools/nid_names.py. The behaviour follows LibreSSL 4.0.0:EVP_CIPHER_nid(aLdgrI2jAF0) returnsnidat offset 0 of the 80-byteEVP_CIPHER.BN_is_negative(AiNud8LR8B4) returnsneg != 0from the 24-byteBIGNUM. The newinclude/Bignum.hppdefinesLibreSsl3Bignum.RSA_bits(3KZ-L4y1gak) returnsBN_num_bits(n), readingnat offset 24 of the module's existing 152-byteLibreSsl3Rsalayout.A null argument throws, as do malformed bignums: negative
top, nulld, or a bit count aboveINT_MAX. The additional-export inventory goes from 177 entries (126 known only by NID) to 174 (123). Debt entries record that none of this is checked on the console. They also record that the console's RSA layout (offset 24 here, 32 in libSceLibreSsl'sLibreSslRsa) is unverified.Tested
Linux x86-64 (GCC). Built the libSceLibreSsl3 test targets and ran
ctest -R "guest_libressl3|guest_crypto_memcmp": 9 of 9 passed, including the newguest_libressl3_bignumandguest_libressl3_rsa, andguest_libressl3_cipher, which now coversEVP_CIPHER_nid.tools/check_conventions.py --base origin/mainandtest_public_headers.py --audit-onlypass.nm -Don the builtlibSceLibreSsl3.prxshowsEVP_CIPHER_nid_nid_postfix,BN_is_negative_nid_postfixandRSA_bits_nid_postfix. No title, Windows or console testing.The
INT_MAXcase intests/Rsa.cppcomputes adpointer throughuintptr_tso thatd[top - 1]lands on one stack word instead of allocating 256 MiB. This is implementation-defined but holds on flat x86-64 address spaces.tools/pr_overlap.pylists #10461, #10557, #10631, #10960, #12754, #12928, #12945 and #13662. They share files (Api.hpp,CMakeLists.txt,Key.hpp,AdditionalExports.cpp, the debt file) but none implements these exports.Checklist
main; no other open PR implements the same functionsNotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file.mdfiles or images added to the repository (attach them to this PR)🤖 Generated with Claude Code
https://claude.ai/code/session_013rhFWQL7oZRJSiGdkL5Gkn