rzo1 opened a new issue, #2102:
URL: https://github.com/apache/stormcrawler/issues/2102
## What happens
`SCProxy.toString()` reassembles the connection string it was parsed from,
and puts the username and password back in when both are set. The okhttp
protocol passes that string to `LOG.debug` once per proxied fetch. With DEBUG
enabled on the protocol classes, the proxy credentials are written to the
worker log on every request that uses a proxy.
## Where
`core/src/main/java/org/apache/stormcrawler/proxy/SCProxy.java:138-140`,
inside `toString()` at `:133`:
```java
if (this.username != null && this.password != null) {
proxyString = this.username + ":" + this.password + "@" +
proxyString;
}
```
Call site,
`core/src/main/java/org/apache/stormcrawler/protocol/okhttp/HttpProtocol.java:364`:
```java
LOG.debug("fetching with proxy {} - {} ", url,
prox.toString());
```
## Why it matters
Worker logs are usually shipped to central aggregation, and the people who
can read them are not the same set as the people entrusted with the proxy
account. Turning on DEBUG to look at fetch behaviour is a normal thing to do,
not a misconfiguration, and it writes the credential once per request. The
proxy is often a paid third party service, so the credential has value on its
own.
Nothing depends on `toString()` functionally. Proxies are used through
`getAddress`, `getPort`, `getUsername` and `getPassword`; `equals` and
`hashCode` are implemented on the fields, not on the rendered string. The only
caller that expects the full string is `SCProxyTest.testToString`.
## Reproduction
Save as
`core/src/test/java/org/apache/stormcrawler/proxy/SCProxyRedactionTest.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.proxy;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
class SCProxyRedactionTest {
@Test
void toStringDoesNotContainCredentials() {
SCProxy proxy = new SCProxy("http://user1:[email protected]:8080";);
String rendered = proxy.toString();
Assertions.assertFalse(
rendered.contains("pass1"),
"SCProxy.toString() exposes the proxy password: " +
rendered);
Assertions.assertTrue(
rendered.contains("example.com:8080"),
"SCProxy.toString() should still identify the proxy
endpoint: " + rendered);
}
}
```
Run it:
```
mvn -pl core test -Dtest=SCProxyRedactionTest
```
It asserts the wanted behaviour and fails on main.
```
org.apache.stormcrawler.proxy.SCProxyRedactionTest.toStringDoesNotContainCredentials
-- Time elapsed: 0.071 s <<< FAILURE!
org.opentest4j.AssertionFailedError: SCProxy.toString() exposes the proxy
password: http://user1:[email protected]:8080 ==> expected: <false> but was:
<true>
Tests run: 1, Failures: 1, Errors: 0, Skipped: 0
```
## Suggested fix
Change `SCProxy.toString()` to render `protocol://user:***@address:port`,
keeping the username if that is useful for support and dropping the password.
Change the `LOG.debug` call to log `getAddress()` and `getPort()` only.
`SCProxyTest.testToString` asserts the current round trip and has to be updated
in the same commit. If some downstream code really needs the full connection
string, add an explicit method for it rather than leaving it on `toString()`;
nothing in this tree needs one today, so the round trip is the only
compatibility impact.
--
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]
