#2137: fix fix-vpn-tls-problem not detecting TLS certificate issues behind HTTP redirects#2138
Open
maybeec wants to merge 2 commits into
Open
#2137: fix fix-vpn-tls-problem not detecting TLS certificate issues behind HTTP redirects#2138maybeec wants to merge 2 commits into
maybeec wants to merge 2 commits into
Conversation
e9071fe to
872d900
Compare
Collaborator
Coverage Report for CI Build 29010809949Coverage increased (+0.3%) to 72.484%Details
Uncovered ChangesNo uncovered changes found. Coverage Regressions45 previously-covered lines in 5 files lost coverage.
Coverage Stats💛 - Coveralls |
f88c7dc to
38a7a26
Compare
…ehind HTTP redirects Reuse the central redirect-following HttpClient (new HttpClientFactory) for the truststore reachability check and capture the certificate from the effective endpoint after following redirects (e.g. https://aka.ms/...), instead of a raw SSLSocket handshake against the requested host.
38a7a26 to
4022123
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR fixes #2137
ide fix-vpn-tls-problem <url>reported success and did nothing when the given URL redirects (HTTP 30x) to another host whose certificate is the one actually being rejected (e.g.https://aka.ms/vs/17/release/vs_BuildTools.exeused by themsvc/rustinstallers). The commandlet probed reachability with a hand-rolled rawSSLSockethandshake against the requested host, so the publicly-trusted redirector (aka.ms) passed while the intercepted certificate on the redirect target was never seen. This did not reuse the central redirect-followingHttpClientthat the actual download uses.Implemented changes:
HttpClientFactory(com.devonfw.tools.ide.io) as the single place that builds the redirect-following (Redirect.ALWAYS)HttpClient, and reused it inHttpDownloader,IdeaPluginDownloaderandAbstractUrlUpdater(removing the duplicated builders).TruststoreUtilto probe reachability viaisUrlReachable(url, truststore)using that client (following redirects, honoring an optional custom truststore) instead of a rawSSLSocket, and addedresolveEffectiveEndpoint(url)which follows redirects to determine the host that actually serves the content.TruststoreCommandletnow probes the URL and captures the certificate from the effective (post-redirect) endpoint. Removed the now-dead raw-socket helpers (isReachable,verifyConnectionWithTruststore).NetworkStatusImplnow detects the (usually wrapped)SSLHandshakeExceptionacross the whole cause chain and logs a clear hint naming the failing URL, warning to verify the endpoint, and giving the ready-to-runide fix-vpn-tls-problem <url>command. Previously that hint only fired for a rawIOException, so it was bypassed for theIllegalStateExceptionthe downloader throws — the user just saw an "unexpected error" and a stack trace (exactly the reported symptom).Testing instructions
mvn clean test— the new tests pass:HttpClientFactoryTest— the central client always follows redirects.TruststoreUtilTest#testResolveEffectiveEndpointFollowsRedirectToFinalHost— a request to server A is resolved to the redirected server B's port (two WireMock HTTPS servers on distinct ports).TruststoreUtilTest#testIsUrlReachableFalseForUntrustedCertificateAndTrueWithCapturedCertificate— an untrusted self-signed endpoint is unreachable by default and reachable once its captured certificate is in the custom truststore.TruststoreCommandletTest#testRunCapturesUntrustedCertificateFollowingRedirectAndConfiguresTruststore— the commandlet follows a redirect over HTTPS, captures the certificate, writes the truststore and configuresIDE_OPTIONS.NetworkStatusImplTest— a TLS trust error wrapped in aRuntimeExceptionis still detected and reported with the failing URL and theide fix-vpn-tls-problem <url>hint.CN=localhost(SANlocalhost/127.0.0.1) keystore (cli/src/test/resources/truststore/localhost.p12) so the JDKHttpClienthostname verification passes against WireMock.Checklist for this PR
mvn clean testlocally all tests pass and build is successful#«issue-id»: «brief summary»(e.g.#921: fixed setup.bat). If no issue ID exists, title only.In Progressand assigned to you or there is no issue (might happen for very small PRs) — issue is assigned; board status "In Progress" still to be set manuallyinternalChecklist for tool commandlets
Not applicable — this PR does not introduce a new tool commandlet; it fixes the existing
fix-vpn-tls-problemcommandlet.