Repository navigation
feat(libressl): implement X509 extension getters - #10078
atharvjairath wants to merge 1 commit into
Conversation
Progress report (edf6804 → f0e1f6b)📈 System libraries: 45.93% (+0.02%, +3 implemented functions) ✅ 3 implemented
Changed libraries (implemented exports)
|
|
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.
|
65d7a66 to
0b3a1e6
Compare
0b3a1e6 to
9c11a09
Compare
9c11a09 to
abc9919
Compare
MarcZX10
left a comment
There was a problem hiding this comment.
Reviewed exact head abc991925908a7b7fb695f21f89f0eee609d9977. The X509 getter implementation follows the cited LP64 layout using byte copies, preserves borrowed pointers, and the focused 1,100-case fixture exercises signed critical values, pointer preservation and non-mutation. The description reports exact-head Linux and Windows 6/6 runs and records that console validation is outstanding. Hosted eligibility, conventions, labels and report checks pass; hosted product jobs are path-skipped. I have not independently run the local test suite.\n\nOne contribution-rule item needs resolving before merge: a direct merge preview of this head with your open #10071 reports a content conflict in docs/dev/debt/libSceLibreSsl.md (both edit the same pending-export count and add a debt entry). Please coordinate which lands first, declare that order with Depends on, and make sure the second branch keeps both entries and the corrected count. This is a keep-both sequencing request; neither PR needs to be closed.
|
Formatting correction to my review above: its paragraph separator rendered as literal The direct merge preview confirms a content conflict in |
abc9919 to
0058290
Compare
|
@atharvjairath on MarcZX10's sequencing point for this PR and #10071: #10071 lands first. Once it has merged, please merge main into this branch, keep both debt entries in docs/dev/debt/libSceLibreSsl.md, and set the pending count from Export.cpp (it should come to 367 if nothing else in that library lands in between). Please also add "Depends on: #10071" to this PR's checklist. #10071 needs no change for this. |
0058290 to
b0c5bea
Compare
MarcZX10
left a comment
There was a problem hiding this comment.
Reviewed exact head b0c5bea, comparing the feature commit against its declared prerequisite #10071. The three getters follow the cited LibreSSL 2.5.4 layout and signed critical rule, preserve borrowed object/data pointers, and leave the extension bytes untouched; the focused test covers null inputs, signed boundaries, pointer preservation, and adjacent-byte canaries. The refreshed branch merges cleanly with #10071 (git merge-tree); the checklist now declares Depends on: #10071, matching the recorded landing order. Author-reported Linux and Windows focused runs pass, current hosted conventions/eligibility/labels/report pass, product jobs are path-skipped. I did not run the tests locally.
5aebe79 to
cb67340
Compare
|
Native Windows validation for head Windows 11 Pro 10.0.26300, Ryzen 7 5700X3D, WinLibs GCC 15.2.0 UCRT POSIX SEH r7, CMake/CTest 4.3.1, Ninja Release, Separate build directories were used. The baseline reused a verified compatible build whose relevant libc, libkernel, LibreSSL, CMake and submodule inputs were unchanged; the candidate build directory was new. Built the real libraries and the runtime, ASN.1, cipher and extension test executables. These fixtures use unpatched libraries.
No skips or timeouts. In particular, the two tests that needed Negative controls used the unchanged candidate executables and other runtime binaries, replacing only The public-header declaration/umbrella audit passes (368 libraries, 12,188 exports). The LibreSSL public umbrella compiles standalone, included twice, with MinGW C++20 and Commands: Both PR heads were rechecked unchanged after testing. Main advanced during the campaign; results apply to the pinned integration above. Logs, JUnit and binary hashes are retained locally. No title, GPU, full-suite, patched-library or console-layout/equivalence validation is claimed; the documented console ABI/layout uncertainty remains. The landing order remains #10071 then #10078. AI-assisted: yes; these builds, tests and controls ran locally on native Windows. |
cb67340 to
968b3d8
Compare
|
The layout matches firmware 13.42's
|
968b3d8 to
5c03235
Compare
5c03235 to
0cf7067
Compare
0cf7067 to
afcbeb3
Compare
afcbeb3 to
698a270
Compare
698a270 to
eb4af42
Compare
|
@atharvjairath the getters are unchanged and still read right. One thing for the merge: main's |
eb4af42 to
29181a5
Compare
29181a5 to
7321dc0
Compare
7321dc0 to
f0e1f6b
Compare
What
Implement
X509_EXTENSION_get_critical,X509_EXTENSION_get_dataandX509_EXTENSION_get_objectin libSceLibreSsl. Callers can read an extension's critical flag and borrowed ASN.1 object/data pointers instead of hitting unsupported-export exceptions. Null extensions return zero/null; the critical flag is true only for a positive signed 32-bit value, so the upstream default-1remains false. Pointer values are returned unchanged without decoding, copying or taking ownership.The contract follows LibreSSL 2.5.4 x509_v3.c and the 24-byte LP64 layout in x509.h. The implementation reads only the requested field, using fixed-width critical storage and byte copies to avoid aliasing assumptions. Published firmware 13.40 measurements identify this module as LibreSSL 2.5.4. These getters and this layout have not been independently measured on a console; that limitation is recorded in the existing debt file.
The progress scanner counts three additional completed functions on the same main baseline (main: 90/401; this PR: 93/401), corresponding exactly to these three exported getters. Its denominator also includes main's new
CompareObjectsandLoadFilehelper callbacks, so 401 is not an export inventory or a compatibility measure. The actual unsupported-export block contains 296 exports, recounted from source against main's 299. The scanner also marks ten APIs outside that block incomplete:ASN1_INTEGER_get,SSL_select_next_proto,ERR_remove_thread_state,i2t_ASN1_OBJECT,X509_STORE_add_lookup,X509_STORE_free,X509_STORE_set_flags,X509_VERIFY_PARAM_set1_host,X509_VERIFY_PARAM_set1_ip_ascandX509_VERIFY_PARAM_set_hostflags. Main's implementations and partial implementations are not attributed to this PR.Landing status: This single extension-getter follow-up is based on main
2960e9a6e7e1fc99c4ed180c71495b5eb83464c7, with no open prerequisites. The refresh resolves a debt conflict after #11189'sX509_VERIFY_PARAM_set1_hostmerged. It retains main's entire implementation, canonical declaration, expanded host-name/allocation tests and debt entry. Every own production/header/CMake/fixture patch addition and deletion is unchanged from the prior follow-up. The actual unsupported-export count is main 299, this branch 296. The earlier pending-count review remains addressed. All earlier store/lookup/verification-parameter, ExData, store-context, ObjectText/ObjectTable, PKEY, protocol, ASN.1 and LibreSSL3 source/test/debt unions remain intact. Main's improvements are not attributed to these three getters.The public-header review is addressed by replacing the three extension getter placeholders in
include/Api.hppwith their documented signatures. The fixture uses the public umbrella header without duplicate local declarations. Main now provides the canonical cipher declarations. Getter bodies and regression assertions are unchanged.Tested
Linux passed all 33 tests; Windows PE through Wine/QEMU passed 28 of 33. UI and the normal X509 store, verification-parameter and lookup tests time out after
0xc000001d/liblibc.dll+0xc75a4. ExData times out withSt12system_error/guest_libressl_ex_data_tests.exe+0x9758and missing unwind data. A freshly rebuilt complete LibreSSL DLL from exact main base2960e9a6e7e1fc99c4ed180c71495b5eb83464c7, including protocol selection and the merged host-name support but excluding our getters, reproduces all five failures with the same rebuilt libc/mbedcrypto and unchanged test executables. The four primary libc fault offsets and ExData exception/offset match; Wine loader traces confirm every control loaded the separate main DLL. These remain validation limitations. No new runtime switch, assertion change or production workaround was added. Failure/control logs and artifact hashes are retained in/private/tmp/anyps5-monitor-20261029-1200.Exact tested head:
f0e1f6b3818cc2542ff7fc3f08b69fab5608cda0; parent and main base:2960e9a6e7e1fc99c4ed180c71495b5eb83464c7; tree:1a10fc6fc4dabc09452278a72e23f250b4cf7ee8. Recovery ref:backup/pr-10078-before-verify-host-refresh-20261029-1200preserves prior remote head7321dc0124cc087a3e8b38e6ea9fdcc0daea895f. Author and committer:atharvjairath <54663702+atharvjairath@users.noreply.github.com>.-Werror. Implementation and fixtures compile against the canonical declarations on both tested platforms.APS5_NO_WRITE_WATCH=1only forguest_libressl_runtimeandguest_libressl_asn1. The separate ASN.1 allocator test and all other cases run without it. Assertions and production checks remain enabled; no new workaround was introduced.The earlier refresh on main
6bc113969895900890ae9bb4c473beb840ba12dbinitially passed 11/13 tests with default Wine write watching; runtime and ASN.1 conversion timed out after0xc000001datGuestHeap::allocate,liblibc.dll+0xb8624. Its exact-main control excluded our getters, rebuilt all LibreSSL sources with the same libc/mbedcrypto and unchanged upstream fixtures, reproduced both faults, then passed withAPS5_NO_WRITE_WATCH=1. Those failure/control logs are retained. This refresh rebuilds the complete real libraries and libc and retains that setting only for those same two tests; UI, ExData, store-context, store/verification-parameter/lookup normal and allocator cases, PKEY, protocol, ASN.1 decoder, key-reference, object-text/allocation and other tests use defaults. Historical negative controls and the earlier runtime/ASN.1 default-Wine controls were not rerun on the new base; the fresh exact-main controls above reproduce all five current failures, including matching primary libc offsets and ExData exception/offset. Default Windows page write watching remains unvalidated here; no source assertion or production check was disabled.Merge/source audit: All three active owned branches merge cleanly with main
2960e9a6e7e1fc99c4ed180c71495b5eb83464c7. The extension runtime-tested head is directly based on that revision. The complete real libc was rebuilt for this focused matrix, including the shared source changes inspected in the intervening source audits. Since the last audit, main's math fixtures only add source-location failure diagnostics without changing assertions. The new host-name setter and expanded store tests are byte-identical to main. The external harness retains main's quoted HostStandardStreams include path; its MinGW host-only helper convention does not apply to these guest-linked targets. The unchanged CES and HTTP branches were not rebuilt during this refresh. This focused matrix and scoped source/merge review do not establish whole-project runtime coverage.Later-main audit: Main subsequently advanced to
edf6804cf52c62eacafc592dd6142cb4efe5bd05. All three clean local merge previews retain every own patch payload. New CES null-input assertions and macOS interface-address support were inspected and preserved; other changed paths received scoped path review. This does not extend the exact runtime coverage beyond the tested base above.Separate firmware report: BunnyFl reports testing firmware 13.42's library in an emulator. Offset-marker inputs produced object/data reads at 0/16, a signed critical field at 8 with
critical > 0, and null results for null extensions in two runs. This agrees with this implementation. It is attributed emulated-library evidence, not independent hardware validation here or a review approval.Separate contributor evidence: benjaminmathias reports native Windows validation of previous extension head
cb6734063f5fd3f4d7612eea44fff5749fbe4454, including cipher0390178ba06a360f4c4799725bf0699e3a3c4916, integrated with main16ff208dc32cf3927ba1bae492eee363a5ae7ae8. On Windows 11 Pro with WinLibs GCC 15.2 UCRT, the runtime, ASN.1 conversion, ASN.1 allocator, cipher and extension tests each passed three repetitions withAPS5_NO_WRITE_WATCHunset. The report also records failing main-library controls for the two getter fixtures and passing results after restoring the candidate library. This is attributed evidence for those test paths on that pinned integration, not independent reproduction here, approval, or native validation of the refreshed head above. Logs and hashes are retained by the contributor.The refreshed tree has not been run on native Windows here. Whole-project, title and PS5 console validation were not performed; console layout and ABI limitations remain. Full upstream product CI requires maintainer dispatch.
Focused runtime build recipe
Use this CMake project outside the checkout, with repository-pinned ZArchive, zstd and mbedTLS checkouts under its
3rdparty/directory (mbedTLS068ff080b369adfac81509f9b57b2afabaf82dc5, including its pinned framework submodule):Configure with
-DANYPS5_SOURCE_ROOT=<checkout> -DCMAKE_BUILD_TYPE=Release, build, then runctest --test-dir <build> --output-on-failure. Linux in this environment setsCMAKE_CROSSCOMPILING_EMULATORto the native ARM QEMU 11.0.3 x86-64 binary; omit this on native x86-64 Linux. Windows builds use WinLibs GCC 15.2.0 UCRT r7 and compile serially under Wine. The actual Windows executables run through QEMU/Wine withWINELOADERNOEXEC=1,WINEPREFIX=/build/wine, andWINEPATHcontaining the build directory, both LibreSSL DLL directories and WinLibs bin.For the reported Wine run, the external Windows CTest manifest additionally sets
ENVIRONMENT "APS5_NO_WRITE_WATCH=1"only onguest_libressl_runtimeandguest_libressl_asn1, because Wine page write watching faults in this environment. The separate contributor report above covers those default native Windows paths on its pinned earlier integration; the refreshed head has not been tested natively on Windows here.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)