Repository navigation
fix(ssl): retain loaded certificate authorities - #3897
Conversation
adriadam10
left a comment
There was a problem hiding this comment.
Thanks for this. Not mergeable as is:
- The branch is based on an old
mainand conflicts indocs/dev/TechnicalDebt.md. Main has since split that page: entries about one library now live indocs/dev/debt/<library>.mdunder the same sections. The diff rewrites the old single file and would revert that split. Please rebase on currentmainand put thelibSceSsllines in the matchingdebt/file, leavingTechnicalDebt.mdas on main. I can't push the rebase myself because "Allow edits from maintainers" is off on the PR. - This adds
3rdparty/mbedtlsas a new third-party dependency (new submodule, new CMake wiring). Please mention in the PR body why an existing dependency isn't enough and its license/size impact, so the maintainers can OK it.
After the rebase, please rerun guest_ssl_load and the other focused tests on the rebased head.
Adapt PR boykopovar#3897 at 1d533d2. Build Mbed TLS 3.6.7 from its pinned submodule. Retain context-owned DER/PEM authorities and matching client credentials, provide owned CA snapshots, enforce pool budgets and preserve existing credentials on import failure. The functional regression fails on the original LoadCert stub. Expanded Windows tests cover DER round trips, malformed input, snapshot ownership and guards, atomic rollback, pool exhaustion/reuse and concurrent shared-context imports. Six SSL/HTTP/memory-pool tests pass. Co-authored-by: eliasbruno124-dev <eliasbruno124@gmail.com>
|
Rebased in f76d8c6 onto 0aa4cb1. The SSL and shared pool-stat debt entries are in the matching library files; TechnicalDebt.md is unchanged from main, and newer throwing exports are preserved. The full Windows Release build, libs and all six focused tests pass on the rebased head. The certificate-load regression fails with main's Export.cpp and passes after restoring the implementation; the final rebase changes neither side. Conventions, including the updated PR body, and diff checks pass. Linux was not tested locally. The description now explains why a certificate parser is needed, the Mbed TLS/p256-m license options and the measured size impact: 47,433,926 tracked source bytes including the framework, and +640,288 bytes for the complete unstripped Windows SSL binary. The actual link excludes Everest and the TLS transport library. TLOU's current startup observation is in #5468; it is not yet rendering. #5226 separately changes the SSL context registry for connection operations. Do you prefer that to land first? I can rebase this certificate work over it; it is not included in this branch. Maintainer edits remain disabled at the owner's request; I will handle any further corrections here. Prepared with AI assistance. |
|
I recommend integrating #5226 before this certificate change, then rebasing this PR onto it. There is no behavioral dependency for |
f76d8c6 to
8bd213c
Compare
|
Integrated #5226 first, preserving its authors and connection/socket behavior, then rebased the certificate change onto main The full Windows runtime/relinker build and all seven focused tests pass. With the parent-only SSL implementation, the connection test passes but the certificate regression fails at the original missing operation; restoring the candidate passes again. The final rebase preserves that implementation byte-for-byte. Conventions and diff checks pass; Linux remains untested locally. The same-input TLOU comparison on Prepared with AI assistance. |
|
@eliasbruno124-dev #5226 goes in first, and this branch carries older copies of its two commits, so I rebased your certificate commit onto #5226's final version: oneandonlydean/AnyPS5@b737308 (branch |
Met: rebased onto main, debt lines moved to debt/, dependency and license explained, focused tests rerun.
8bd213c to
b737308
Compare
|
Adopted your exact The complete Windows runtime, relinker and relevant test executables build. All seven focused SSL/HTTP/Net tests pass with the 30-second limit; conventions and diff checks pass. Linux was not run locally. The description keeps the earlier TLOU comparison tied to its measured commit rather than claiming a new title run. Prepared with AI assistance. |
|
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.
|
build.yml runs git submodule update --init --recursive, but release.yml checked out only the top-level submodules. A submodule with its own submodules, such as Mbed TLS's framework in #3897, would leave the release configure step without them.
3rdparty/mbedtls (#3897) has its own framework submodule. The PR build fetched it in its merge step, but push builds on main skipped that step and failed at configure. Co-Authored-By: Claude <noreply@anthropic.com>
What
The Last of Us Part II Remastered (PPSA15508/01.002.000) stops at
sceSslLoadCertduring startup. This change parses and retains the supplied CA chain and optional client credentials per SSL context, exposes owned DER snapshots, and releases allocations on unload/free/term. Invalid imports leave existing credentials intact. Loading credentials does not establish a TLS connection or authenticate a peer; unsupported operations still throw.Adopts the maintainer's exact rebased commit
b737308e, directly on the final #5226 commit8e688966, now merged into main. The shared registry and certificate/connection lifetime regression are retained.sceSslConnect,sceSslReadandsceSslWriteremain explicitNotImplemented_nid_no_patchstubs; this PR adds no TLS transport. SSL and pool-stat debt stays in the matching library files;TechnicalDebt.mdis unchanged from the parent.Dependency
Mbed TLS 3.6.7 is pinned as a source submodule for X.509 DER/PEM parsing and client certificate/private-key matching. Main has no existing certificate parser; its media dependencies do not provide these operations. The link uses
mbedx509,mbedcryptoandp256m, without Everest or the Mbed TLS transport library.The linked Mbed TLS and p256-m sources offer Apache-2.0 OR GPL-2.0-or-later; the Apache-2.0 option is available here, and upstream license files are retained. The pinned Mbed TLS/framework revisions contain 47,433,926 tracked source bytes, excluding Git history, generated files and build outputs. In the earlier October 14 Windows Release comparison, the patched, unstripped SSL DLL grows from 79,106 bytes with #5226 alone to 720,093 bytes with certificate support (+640,987). This historical measurement covers the whole certificate contribution; it is not a fresh size comparison of the maintainer's rebase.
Tested
Windows 11, WinLibs GCC 15.2.0, Release, exact
b737308e:guest_ssl_load,guest_ssl,guest_http,guest_http2,guest_http_response_header,guest_memory_pool_statsandguest_sce_net, with a 30-second per-test limit.sceSslLoadCert not implemented; restoring the certificate implementation passes all seven. That control predates this rebase. The certificate regression and its fixtures are unchanged; all seven tests were rerun onb737308e.git diff --checkpass. Linux was not tested locally.The same-input TLOU comparison used main
0b6d033b8plus #5226, changing only the SSL DLL. Baseline aborts atsceSslLoadCert. Candidate3c96928e4returns 0 for the same original 1,408-byte CA; the merged #3925 then completes HTTP2 send/wait with the offline result0x80436063. The next stop is a null read at original VA0x1b60f2, before VideoOutOpen or any submitted frame. The certificate code and its fixtures remain unchanged by the maintainer's latest rebase; its SSL changes preserve the final parent's explicit transport stubs. The runtime and focused tests were rebuilt onb737308e, but this title observation remains attributed to3c96928e4; it was not rerun onb737308e.The original title files were preserved, using recorded plain-SELF staging and same-name resource mappings outside the dump. This is not a container-identical direct-dump launch or a menu, gameplay or performance result. Bring-up: #5468.
Console error precedence, repeated-import behavior, snapshot lifetime and exact allocator overhead remain unmeasured and are documented as technical debt.
Checklist
main; no other open PR implements the same certificate operations