rzo1 opened a new issue, #2091:
URL: https://github.com/apache/stormcrawler/issues/2091
## What happens
`HttpProtocol.getProtocolOutput()` copies every response header into the
protocol metadata. When a header name equals `_request.headers_` or
`_response.headers_`, the value is base64 decoded first and then stored under
exactly the key the WARC writer treats as the crawler's own capture record.
`_response.ip_` and `_request.time_` are copied in the same loop without any
name check. The interceptor that legitimately sets these keys is only installed
when `http.store.headers` is true, which is not the default, so with the
default configuration nothing overwrites a value that came from the wire.
## Where
`core/src/main/java/org/apache/stormcrawler/protocol/okhttp/HttpProtocol.java:434-446`
on main; consumers in
`external/warc/src/main/java/org/apache/stormcrawler/warc/WARCRecordFormat.java:349`
and `:372-379` and `WARCRequestRecordFormat.java:54`. Config key:
`http.store.headers`.
```java
if (key.equals(ProtocolResponse.REQUEST_HEADERS_KEY)
|| key.equals(ProtocolResponse.RESPONSE_HEADERS_KEY)) {
value = new String(Base64.getDecoder().decode(value),
StandardCharsets.ISO_8859_1);
}
responsemetadata.addValue(key.toLowerCase(Locale.ROOT), value);
```
## Why it matters
`WARCRecordFormat.format` promotes a record from `resource` to `response`
whenever `_response.headers_` is non-blank and embeds the decoded block
verbatim, and `WARCRequestRecordFormat` writes the `_request.headers_` block as
what the crawler supposedly sent. A fetched server can therefore choose the
contents of records that are read as the crawler's own capture, including raw
CRLF inside them, and can set `WARC-IP-Address` and `WARC-Date`. The archive
consumer, not the crawler, carries the consequence, and
`external/warc/README.md:150` tells WARC users to set `http.store.headers:
true`, which makes the interceptor overwrite the forged values. Deployments
that leave it false are the affected ones. Separately, the decode is unguarded,
so a value that is not valid base64 throws `IllegalArgumentException` out of
`getProtocolOutput` and fails that fetch.
## Reproduction
Save as
`core/src/test/java/org/apache/stormcrawler/protocol/OkHttpReservedHeaderKeysTest.java`:
```java
/*
* Licensed to the Apache Software Foundation (ASF) under one or more
* contributor license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright ownership.
* The ASF licenses this file to you under the Apache License, Version 2.0
* (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.apache.stormcrawler.protocol;
import jakarta.servlet.http.HttpServletResponse;
import java.io.IOException;
import java.io.OutputStream;
import java.nio.charset.StandardCharsets;
import java.util.Base64;
import org.apache.storm.Config;
import org.apache.stormcrawler.Metadata;
import org.eclipse.jetty.server.Handler;
import org.eclipse.jetty.server.Request;
import org.eclipse.jetty.server.handler.AbstractHandler;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
/** Checks that reserved protocol metadata keys cannot be set by a fetched
server. */
class OkHttpReservedHeaderKeysTest extends AbstractProtocolTest {
private static String b64(String s) {
return
Base64.getEncoder().encodeToString(s.getBytes(StandardCharsets.ISO_8859_1));
}
@Override
protected Handler[] getHandlers() {
return new Handler[] {
new AbstractHandler() {
@Override
public void handle(
String target,
Request baseRequest,
jakarta.servlet.http.HttpServletRequest request,
HttpServletResponse response)
throws IOException {
baseRequest.setHandled(true);
response.setStatus(HttpServletResponse.SC_OK);
response.setContentType("text/html");
response.setHeader(
ProtocolResponse.RESPONSE_HEADERS_KEY,
b64("HTTP/1.1 200 OK\r\nContent-Type:
text/plain\r\n\r\n"));
response.setHeader(
ProtocolResponse.REQUEST_HEADERS_KEY,
b64("GET /elsewhere HTTP/1.1\r\nHost:
somewhere.example\r\n\r\n"));
response.setHeader(ProtocolResponse.RESPONSE_IP_KEY,
"10.0.0.5");
response.setHeader(ProtocolResponse.REQUEST_TIME_KEY,
"1");
final byte[] content =
"hello".getBytes(StandardCharsets.UTF_8);
response.setContentLength(content.length);
try (OutputStream out = response.getOutputStream()) {
out.write(content);
}
}
}
};
}
@Test
void responseHeadersCannotSetReservedKeys() throws Exception {
Config conf = new Config();
conf.put("http.agent.name", "this_is_only_a_test");
org.apache.stormcrawler.protocol.okhttp.HttpProtocol protocol =
new org.apache.stormcrawler.protocol.okhttp.HttpProtocol();
protocol.configure(conf);
ProtocolResponse response =
protocol.getProtocolOutput("http://localhost:"; + HTTP_PORT +
"/", new Metadata());
Metadata md = response.getMetadata();
Assertions.assertNull(
md.getFirstValue(ProtocolResponse.RESPONSE_HEADERS_KEY),
"a response header must not populate the verbatim response
record");
Assertions.assertNull(
md.getFirstValue(ProtocolResponse.REQUEST_HEADERS_KEY),
"a response header must not populate the verbatim request
record");
Assertions.assertNull(
md.getFirstValue(ProtocolResponse.RESPONSE_IP_KEY),
"a response header must not populate the recorded IP
address");
protocol.cleanup();
}
}
```
Run it:
```
mvn -pl core test -Dtest=OkHttpReservedHeaderKeysTest
```
A local Jetty server returns the reserved header names; the test asserts the
intended behaviour and fails on main.
```
[ERROR] OkHttpReservedHeaderKeysTest.responseHeadersCannotSetReservedKeys:82
a response header must not populate the verbatim response record ==> expected:
<null> but was: <HTTP/1.1 200 OK
Content-Type: text/plain
>
```
## Suggested fix
In `getProtocolOutput`, drop incoming response headers whose lowercased name
matches one of the reserved keys (`REQUEST_HEADERS_KEY`,
`RESPONSE_HEADERS_KEY`, `RESPONSE_IP_KEY`, `REQUEST_TIME_KEY`,
`PROTOCOL_VERSIONS_KEY`, `TRIMMED_RESPONSE_KEY`,
`TRIMMED_RESPONSE_REASON_KEY`), or store them under a quarantined prefix.
Remove the base64 decoding of server-supplied values entirely, since only
`HTTPHeadersInterceptor` should ever write these keys and it runs after this
loop. Nothing legitimate sends header names of that shape, so no compatibility
impact is expected.
--
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]
