abhinav-phi commented on PR #2126: URL: https://github.com/apache/stormcrawler/pull/2126#issuecomment-5574036067
Thanks for the review — all four points are addressed in a5d7ba15: **1. Cleartext http://** — done as suggested. The decision is now made per request: credentials are only sent when the connection is `https://` *and* the certificate chain is validated, i.e. `credentialsAllowed(url) = http.credentials.allow.insecure || (url.isHttps() && !http.trust.everything)`. Basic auth, credential custom headers, per-request credential headers and cookies all go through it; cleartext http:// requests no longer receive any of them. Three new tests cover the cleartext path (`basicAuthIsWithheldOverCleartextHttp`, `basicAuthIsSentOverCleartextHttpWhenExplicitlyAllowed`) and the per-request split. `HttpProtocolCookieTest` and `HttpRobotRulesParserRedirectTest` verify header/cookie mechanics rather than the security policy, so they now opt in with `http.credentials.allow.insecure: true`. **2. Credential header denylist** — went with option B: `http.credentials.headers` configures the list, defaulting to `authorization`, `proxy-authorization`, `cookie`, `x-api-key`. When set it replaces the default (documented in crawler-default.yaml, configuration.adoc and the test `credentialHeaderNamesCanBeConfigured`, which shows a site-specific `x-auth-token` being withheld while `x-api-key` — no longer in the list — is sent). **3. Cookie warning** — moved below the emptiness check, so it only fires when a cookie was actually going to be sent. The per-request header warning is likewise logged once per protocol instance instead of per header. **4. Proxy credentials** — comment added where the `proxyAuthenticator` is installed: the proxy is not the crawled server, its credentials authenticate against the proxy itself and are sent regardless of whether the target server was authenticated. **5. Keystore regeneration** — the two `keytool` commands are now in a comment in `OkHttpTrustEverythingTest`. **Combined upgrade note** — happy to carry it here (or wherever you prefer, e.g. the 4.0.0 announcement). A draft covering the batch, to adjust as the other PRs settle: > ### Upgrading to StormCrawler 4.0.0 > Several defaults that used to favour coverage now favour security; crawls relying on the old behaviour need explicit configuration: > - **`protocols` no longer contains `file`** — crawling `file://` urls requires adding the scheme back and confining reads with `file.protocol.root` (#2124). > - **The fetcher refuses private, loopback and linklocal addresses by default** (`http.filter.ipaddress.exclude`) — intranet crawls must allow their address ranges explicitly (#2127). > - **`http.trust.everything` defaults to `false`** — hosts with self-signed or otherwise unvalidatable certificates need it set to `true`, and credentials (basic auth, credential headers, cookies) are now withheld from unauthenticated connections — cleartext `http://` included — unless `http.credentials.allow.insecure` is set. Site-specific credential header names go into `http.credentials.headers` (#2126). > - **SiteMapParserBolt parses strictly** — lenient parsing and content-type sniffing are opt-in (#2125). > - **Spouts only emit stored urls with http/https schemes** (#2123). > - **IPFilterRules fails on unparseable rules** instead of silently ignoring them (#2119). Say the word and I'll add it to this PR (a short "Upgrading" section in the docs, or wherever you'd rather keep it). -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
