Repository navigation
fix(http): trust the system CA list for HTTPS requests - #11134
p3n1cillin wants to merge 18 commits into
Conversation
MarcZX10
left a comment
There was a problem hiding this comment.
I found one blocking input-bound issue in the chunked response reader; details are attached to the terminal-chunk loop below. I did not complete a full review of the 2,194-line diff, so this request is limited to that finding.
| if (const int parseResult = ParseChunkSize(line, request.chunkRemaining); parseResult != 0) return parseResult; | ||
| if (request.chunkRemaining == 0) { | ||
| for (;;) { | ||
| if (ReadLine(*request.stream, request.pending, request.pendingOffset, line, kMaximumChunkLine) != 0) return ERROR_NETWORK; |
There was a problem hiding this comment.
Please cap the aggregate trailer section and reclaim consumed input as the parser advances. ReadLine limits each line to 8192 bytes but appends each received block to pending while only advancing pendingOffset; this loop accepts an unlimited number of nonempty trailer lines before the blank terminator, with no aggregate byte/count limit. A peer can therefore keep the synchronous request in this loop and grow the retained buffer without bound. The same retained-prefix issue can accumulate chunk framing on long streamed responses. Please enforce a total trailer limit (for example, based on the configured response-header maximum), compact or otherwise bound consumed pending data, and add regression coverage for an over-limit trailer block and sustained chunked input.
There was a problem hiding this comment.
Thanks. The trailer loop at Request.cpp:720 is in #9834's transport commit (feat(libSceHttp): add synchronous HTTP request transport). This PR's own commits don't touch ReadLine, pending or the chunk reader, and the loop is unchanged at #9834's current head f658abbf1e. So I've asked for it on #9834, so that it's fixed where the code lives, and I'll restack this PR on whatever lands there.
#9834 has also been rebased since I opened this, so I'm moving these commits onto its new head. That applies cleanly, and I'll post the test results once the build finishes.
|
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.
|
|
A related console observation for the scope question in this PR's debt entries, from PS5SX2 (PPSA99203). Its HTTPS client header,
The author marks the client "AI-assisted; needs proper testing", so this is weaker evidence than a measurement. Read together with this PR's two titles:
I've kept the change to libSceHttp, where the titles show it. Making bare libSceSsl contexts trust the system roots, and having AI-assisted: yes (Claude Code) |
|
A precision on the ProsperoStore evidence in the description. I re-read its transport selection and found a second homebrew note on this.
So the evidence supports default trust for a sandboxed process, which is what a retail title is. It doesn't apply to an elevated homebrew process: there the console's system TLS stack fails with unknown CA, and these homebrew titles fall back to their own TLS. AnyPS5 reports a root identity (libkernel debt: AI-assisted: yes (Claude Code) |
cc3a1f0 to
db2335e
Compare
|
One more sandboxed title for the default trust. DOOM Native Port (PPSA99666) creates its libSceHttp template with no certificate, callback or option ( AI-assisted: yes (Claude Code) |
db2335e to
0e548ae
Compare
0e548ae to
17b2aef
Compare
|
Restacked on #9834's new head |
17b2aef to
aa5fd7b
Compare
|
@MarcZX10 I restacked this PR at head
|
aa5fd7b to
a9841e8
Compare
|
@jotitadev Thanks, confirmed on Windows: What it changes:
Rerun on that exact head (Windows 11 Pro 26300, Ryzen 5 7600X, WinLibs GCC 15.2.0, Release, full build). This covers the Windows side you hadn't checked, with
The duplicates come from #9834's files, so I've also added the same commit to the fix branch I offered on #9834. |
jotitadev
left a comment
There was a problem hiding this comment.
Re-checked f9f85640fd on Linux: public_headers passes, so both sides are covered now.
Ubuntu 26.04, i7-8700, GCC 15.2, Release, BUILD_TESTING=ON. Head merged (--no-commit) onto current main 16345cad6a; clean merge, full build OK.
public_headersandguest_http_transport*: pass 3/3.- HTTP/SSL/net tests,
public_headers*andguest_offline_imports: 160/160. - Control: reverting only
f9f85640fdon the merge brings back the same 43 duplicate declarations andpublic_headersfails.
Written with AI assistance (Claude Code). I ran the builds and tests above on my machine.
The trailer lines after the last chunk now count against the response header maximum, and a trailer block over it fails the read with ERROR_PARSE_HTTP_INVALID_RESPONSE as the last error. Consumed bytes of the pending buffer are dropped once it is empty or the consumed prefix reaches the I/O block size, so a long chunked body no longer keeps all earlier input in memory. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
public_headers requires each export to be declared once, in its library's public header. Request.cpp and the two transport tests redeclared the sceHttp, sceNet and sceSsl exports they call in their own extern "C" blocks. Include Http.hpp, Net.hpp and Ssl.hpp instead. The allocation test now uses the header's HttpMemoryPoolStats instead of its own copy of the same 24-byte layout. Reported by jotitadev on boykopovar#11134. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…d check d4e20e7 brought back the NpEntitlementAccess abort/delete loop that expects both calls to throw. main removed that loop when 2456bef made them idempotent, so guest_offline_imports aborted at it. Drop the loop again; only the sceHttpSendRequest expectation differs from main. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
libSceHttp's TLS connections trusted only certificates the title had loaded, so an HTTPS request without a verify callback always failed with an unknown CA. PokeMMO Prospero (PPSA98001) makes its Google sign-in that way, and its README reports it working on firmware 12.40. libSceHttp now calls SslTrustSystemRoots_nid_no_patch, added by boykopovar#11145, for every HTTPS connection, so the system CA list served by libkernel is trusted after the context's own authorities. Bare libSceSsl connections are unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
f9f8564 to
35c09d3
Compare
|
Restacked onto #9834's new head #9834's new test commit also brought back the NpEntitlementAccess abort/delete loop that Against the previous head
|
What
Depends on #9834, which adds libSceHttp's request transport. The libSceSsl helper this PR calls came from #11145, now merged. The stack, from the bottom:
974dfd5ade, unchanged;claude/9834-fixes-1031):mainremoved;The libSceSsl code is the one merged with #11145, as MarcZX10 asked on #11145.
With #9834, every HTTPS request that doesn't install a verify callback fails with
0x80431063, last error0x8095F00C(unknown CA). The cause is that the libSceSsl context the title passes tosceHttpInitstarts with no trust anchors unless the title loads some itself. Titles that rely on the console's built-in trust can't reach any public server.sceHttps*function, loads no certificate and installs no verify callback. Its README reports the Google Drive backup working on firmware 12.40.sceHttpsEnableOptionbefore fetching its public catalog.On the console, both requests pass the known-CA check against public roots, so the system must trust them by default. That is inferred from the titles, not measured.
The change (the libSceSsl parts came with #11145):
/<sandbox word>/common/cert/CA_LIST.cer, once per process. libkernel already serves that file from the host trust store (feat(libkernel): serve the sandbox CA bundle from host TLS roots #5280). The roots are parsed into a separate allocation pool, so they aren't charged to the title'ssceSslInitpool; PokeMMO's is 512 KiB.SslTrustSystemRoots_nid_no_patch(connection), makes one connection trust those roots after the context's own authorities. The session links them into the chain instead of copying them, and unlinks them before freeing.sceSslCreateConnection, and whatsceSslGetCaListandsceSslGetCaCertsreport. Whether a direct libSceSsl connection on the console trusts the system roots is not measured.Tested
Windows 11 Pro (26300), Ryzen 5 7600X, WinLibs GCC 15.2.0, Release, full build of head
35c09d332f(merges cleanly intomaine49d43d625). 164 of 164 pass: the 98 tests that libSceHttp, libSceHttp2, libSceSsl and libSceNet register (including the two trailer cases inguest_http_transportand #11145's five system CA list tests),public_headerswith its 64 compile tests, andguest_offline_imports. The libSceHttp code of this PR is unchanged from the earlier stack; the title runs below were made on that stack:guest_http_transportwith a new no-callback case.cloud: POST send failed (0x80431063). With it:sign-in code shown (… at https://www.google.com/device));client update: download done, 92 MB in 2 s,RESULT client.update PASS);sceAppInstUtilInitializewithout it, from a cleared/data/prosperostore, 60 s each:The new
guest_http_transportcase makes an HTTPS request with no callback to the fixture TLS server:SSL_CERT_FILEto the fixture root, so the request succeeds. With thelibSceHttpcall removed, the test fails. The transport and libSceSsl tests pass.0x8095F00C.tools/check_conventions.py --base origin/mainpasses.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