rzo1 commented on code in PR #2126:
URL: https://github.com/apache/stormcrawler/pull/2126#discussion_r3976828892
##########
core/src/main/java/org/apache/stormcrawler/protocol/okhttp/HttpProtocol.java:
##########
@@ -245,15 +332,32 @@ public void configure(Config conf) {
builder.addNetworkInterceptor(new HTTPHeadersInterceptor());
}
- if (ConfUtils.getBoolean(conf, "http.trust.everything", true)) {
- builder.sslSocketFactory(trustAllSslSocketFactory,
(X509TrustManager) trustAllCerts[0]);
- builder.hostnameVerifier(
- new HostnameVerifier() {
- @Override
- public boolean verify(String hostname, SSLSession
session) {
- return true;
+ // getProtocolOutput only filters the initial request, the redirect
follower copies
+ // the headers onto the next hop without re-checking.
Proxy-Authorization is left
+ // alone, it authenticates against the proxy rather than the crawled
server.
+ builder.addNetworkInterceptor(
Review Comment:
Done in 18c696c6: the credential interceptor is now registered before
`HTTPHeadersInterceptor`, so the request headers stored for a hop (and in the
WARC request record) are the ones actually sent. Pinned by
`storedRequestHeadersMatchTheStrippedHop`, which fails with the old order.
##########
core/src/main/java/org/apache/stormcrawler/protocol/okhttp/HttpProtocol.java:
##########
@@ -108,6 +109,36 @@ public class HttpProtocol extends AbstractHttpProtocol {
// makes sure that a missing cookie origin is reported once and not for
every url
private final AtomicBoolean missingCookieOriginLogged = new
AtomicBoolean();
+ // makes sure that withheld cookies are reported once and not for every url
+ private final AtomicBoolean withheldCookiesLogged = new AtomicBoolean();
+
+ // makes sure that withheld request headers are reported once and not for
every url
+ private final AtomicBoolean withheldRequestHeadersLogged = new
AtomicBoolean();
+
+ /** Default for http.credentials.headers: header names (lower case)
carrying credentials. */
+ private static final Set<String> DEFAULT_CREDENTIAL_HEADERS =
+ Set.of(
+ HttpHeaders.AUTHORIZATION.toLowerCase(Locale.ROOT),
+ HttpHeaders.PROXY_AUTHORIZATION.toLowerCase(Locale.ROOT),
Review Comment:
Yes, fixed in 18c696c6. `Proxy-Authorization` is now only kept on a hop when
the proxy authenticator set it (the request is tagged) and the hop is
cleartext, i.e. when the proxy is the one reading it. A static one from
`http.custom.headers` is stripped like any other credential, matching the yaml.
Pinned by `staticProxyAuthorizationIsStrippedOnHttpsToHttpRedirect`, which
fails with the old exemption.
##########
core/src/main/resources/crawler-default.yaml:
##########
@@ -149,6 +149,42 @@ config:
# Follow redirect HTTP responses:
http.allow.redirects: false
+ # Accept any TLS certificate, including self-signed, expired or otherwise
+ # invalid ones (okhttp protocol only)? The certificate chains are accepted
+ # without validation, i.e. the servers are not authenticated: anyone able to
+ # answer for the host name receives everything sent to them. Needed for
+ # crawling hosts with unvalidatable certificates, e.g. self-signed ones, but
+ # also disables protection against man-in-the-middle attacks. Credentials
+ # (basic auth, credential headers, cookies) are withheld from cleartext
+ # http:// requests and from https:// requests on such connections unless
+ # http.credentials.allow.insecure is set to true.
+ http.trust.everything: false
+
+ # Check that the certificate presented by the server matches the host name
+ # contacted (okhttp protocol only)? Disabling this is independent of
+ # http.trust.everything: a valid certificate for a different name is then
+ # accepted. Hostname verification does not need to be disabled for hosts
+ # with self-signed certificates when http.trust.everything is enabled.
+ http.verify.hostnames: true
+
+ # Send credentials (basic auth, credential headers, cookies) even on
+ # connections which do not authenticate the server (okhttp protocol only)?
+ # Without it, credentials are withheld from cleartext http:// requests and
+ # from https:// requests whose certificate was not validated
+ # (http.trust.everything), so that they are not disclosed to servers which
Review Comment:
Added in 18c696c6: the `http.verify.hostnames` and
`http.credentials.allow.insecure` comments in crawler-default.yaml now say it,
and so do configuration.adoc and extending.adoc.
--
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]
