HADOOP-17311. ABFS: Logs should redact SAS signature (#2422)

Contributed by bilaharith.
This commit is contained in:
bilaharith 2020-11-25 19:52:10 +05:30 committed by GitHub
parent 08b2e285db
commit 3193d8c793
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 187 additions and 19 deletions

View File

@ -87,7 +87,7 @@ private static String formatMessage(final AbfsHttpOperation abfsHttpOperation) {
"Operation failed: \"%1$s\", %2$s, HEAD, %3$s", "Operation failed: \"%1$s\", %2$s, HEAD, %3$s",
abfsHttpOperation.getStatusDescription(), abfsHttpOperation.getStatusDescription(),
abfsHttpOperation.getStatusCode(), abfsHttpOperation.getStatusCode(),
abfsHttpOperation.getUrl().toString()); abfsHttpOperation.getSignatureMaskedUrl());
} }
return String.format( return String.format(
@ -95,7 +95,7 @@ private static String formatMessage(final AbfsHttpOperation abfsHttpOperation) {
abfsHttpOperation.getStatusDescription(), abfsHttpOperation.getStatusDescription(),
abfsHttpOperation.getStatusCode(), abfsHttpOperation.getStatusCode(),
abfsHttpOperation.getMethod(), abfsHttpOperation.getMethod(),
abfsHttpOperation.getUrl().toString(), abfsHttpOperation.getSignatureMaskedUrl(),
abfsHttpOperation.getStorageErrorCode(), abfsHttpOperation.getStorageErrorCode(),
// Remove break line to ensure the request id and timestamp can be shown in console. // Remove break line to ensure the request id and timestamp can be shown in console.
abfsHttpOperation.getStorageErrorMessage().replaceAll("\\n", " ")); abfsHttpOperation.getStorageErrorMessage().replaceAll("\\n", " "));

View File

@ -51,6 +51,8 @@
public class AbfsHttpOperation implements AbfsPerfLoggable { public class AbfsHttpOperation implements AbfsPerfLoggable {
private static final Logger LOG = LoggerFactory.getLogger(AbfsHttpOperation.class); private static final Logger LOG = LoggerFactory.getLogger(AbfsHttpOperation.class);
public static final String SIGNATURE_QUERY_PARAM_KEY = "sig=";
private static final int CONNECT_TIMEOUT = 30 * 1000; private static final int CONNECT_TIMEOUT = 30 * 1000;
private static final int READ_TIMEOUT = 30 * 1000; private static final int READ_TIMEOUT = 30 * 1000;
@ -61,6 +63,8 @@ public class AbfsHttpOperation implements AbfsPerfLoggable {
private final String method; private final String method;
private final URL url; private final URL url;
private String maskedUrl;
private String maskedEncodedUrl;
private HttpURLConnection connection; private HttpURLConnection connection;
private int statusCode; private int statusCode;
@ -103,8 +107,8 @@ public String getMethod() {
return method; return method;
} }
public URL getUrl() { public String getHost() {
return url; return url.getHost();
} }
public int getStatusCode() { public int getStatusCode() {
@ -154,7 +158,6 @@ public String getResponseHeader(String httpHeader) {
// Returns a trace message for the request // Returns a trace message for the request
@Override @Override
public String toString() { public String toString() {
final String urlStr = url.toString();
final StringBuilder sb = new StringBuilder(); final StringBuilder sb = new StringBuilder();
sb.append(statusCode); sb.append(statusCode);
sb.append(","); sb.append(",");
@ -180,19 +183,12 @@ public String toString() {
sb.append(","); sb.append(",");
sb.append(method); sb.append(method);
sb.append(","); sb.append(",");
sb.append(urlStr); sb.append(getSignatureMaskedUrl());
return sb.toString(); return sb.toString();
} }
// Returns a trace message for the ABFS API logging service to consume // Returns a trace message for the ABFS API logging service to consume
public String getLogString() { public String getLogString() {
String urlStr = null;
try {
urlStr = URLEncoder.encode(url.toString(), "UTF-8");
} catch(UnsupportedEncodingException e) {
urlStr = "https%3A%2F%2Ffailed%2Fto%2Fencode%2Furl";
}
final StringBuilder sb = new StringBuilder(); final StringBuilder sb = new StringBuilder();
sb.append("s=") sb.append("s=")
@ -220,7 +216,7 @@ public String getLogString() {
.append(" m=") .append(" m=")
.append(method) .append(method)
.append(" u=") .append(" u=")
.append(urlStr); .append(getSignatureMaskedEncodedUrl());
return sb.toString(); return sb.toString();
} }
@ -513,4 +509,42 @@ private void parseListFilesResponse(final InputStream stream) throws IOException
private boolean isNullInputStream(InputStream stream) { private boolean isNullInputStream(InputStream stream) {
return stream == null ? true : false; return stream == null ? true : false;
} }
public static String getSignatureMaskedUrl(String url) {
int qpStrIdx = url.indexOf('?' + SIGNATURE_QUERY_PARAM_KEY);
if (qpStrIdx == -1) {
qpStrIdx = url.indexOf('&' + SIGNATURE_QUERY_PARAM_KEY);
}
if (qpStrIdx == -1) {
return url;
}
final int sigStartIdx = qpStrIdx + SIGNATURE_QUERY_PARAM_KEY.length() + 1;
final int ampIdx = url.indexOf("&", sigStartIdx);
final int sigEndIndex = (ampIdx != -1) ? ampIdx : url.length();
String signature = url.substring(sigStartIdx, sigEndIndex);
return url.replace(signature, "XXXX");
}
public static String encodedUrlStr(String url) {
try {
return URLEncoder.encode(url, "UTF-8");
} catch (UnsupportedEncodingException e) {
return "https%3A%2F%2Ffailed%2Fto%2Fencode%2Furl";
}
}
public String getSignatureMaskedUrl() {
if (this.maskedUrl == null) {
this.maskedUrl = getSignatureMaskedUrl(this.url.toString());
}
return this.maskedUrl;
}
public String getSignatureMaskedEncodedUrl() {
if (this.maskedEncodedUrl == null) {
this.maskedEncodedUrl = encodedUrlStr(getSignatureMaskedUrl());
}
return this.maskedEncodedUrl;
}
} }

View File

@ -58,6 +58,9 @@ public static void dumpHeadersToDebugLog(final String origin,
if (key.contains("Cookie")) { if (key.contains("Cookie")) {
values = "*cookie info*"; values = "*cookie info*";
} }
if (key.equals("sig")) {
values = "XXXX";
}
LOG.debug(" {}={}", LOG.debug(" {}={}",
key, key,
values); values);

View File

@ -254,11 +254,18 @@ private boolean executeHttpOperation(final int retryCount) throws AzureBlobFileS
incrementCounter(AbfsStatistic.BYTES_RECEIVED, incrementCounter(AbfsStatistic.BYTES_RECEIVED,
httpOperation.getBytesReceived()); httpOperation.getBytesReceived());
} }
} catch (IOException ex) { } catch (UnknownHostException ex) {
if (ex instanceof UnknownHostException) { String hostname = null;
LOG.warn(String.format("Unknown host name: %s. Retrying to resolve the host name...", httpOperation.getUrl().getHost())); if (httpOperation != null) {
hostname = httpOperation.getHost();
} }
LOG.warn("Unknown host name: %s. Retrying to resolve the host name...",
hostname);
if (!client.getRetryPolicy().shouldRetry(retryCount, -1)) {
throw new InvalidAbfsRestOperationException(ex);
}
return false;
} catch (IOException ex) {
if (LOG.isDebugEnabled()) { if (LOG.isDebugEnabled()) {
if (httpOperation != null) { if (httpOperation != null) {
LOG.debug("HttpRequestFailure: " + httpOperation.toString(), ex); LOG.debug("HttpRequestFailure: " + httpOperation.toString(), ex);

View File

@ -25,7 +25,7 @@
import java.util.List; import java.util.List;
import java.util.UUID; import java.util.UUID;
import org.junit.Assert; import org.assertj.core.api.Assertions;
import org.junit.Assume; import org.junit.Assume;
import org.junit.Test; import org.junit.Test;
import org.slf4j.Logger; import org.slf4j.Logger;
@ -39,6 +39,8 @@
import org.apache.hadoop.fs.azurebfs.constants.AbfsHttpConstants; import org.apache.hadoop.fs.azurebfs.constants.AbfsHttpConstants;
import org.apache.hadoop.fs.azurebfs.constants.TestConfigurationKeys; import org.apache.hadoop.fs.azurebfs.constants.TestConfigurationKeys;
import org.apache.hadoop.fs.azurebfs.extensions.MockDelegationSASTokenProvider; import org.apache.hadoop.fs.azurebfs.extensions.MockDelegationSASTokenProvider;
import org.apache.hadoop.fs.azurebfs.services.AbfsHttpOperation;
import org.apache.hadoop.fs.azurebfs.services.AbfsRestOperation;
import org.apache.hadoop.fs.azurebfs.services.AuthType; import org.apache.hadoop.fs.azurebfs.services.AuthType;
import org.apache.hadoop.fs.permission.AclEntry; import org.apache.hadoop.fs.permission.AclEntry;
import org.apache.hadoop.fs.permission.AclEntryScope; import org.apache.hadoop.fs.permission.AclEntryScope;
@ -53,6 +55,7 @@
import static org.apache.hadoop.fs.permission.AclEntryScope.DEFAULT; import static org.apache.hadoop.fs.permission.AclEntryScope.DEFAULT;
import static org.apache.hadoop.fs.permission.AclEntryType.GROUP; import static org.apache.hadoop.fs.permission.AclEntryType.GROUP;
import static org.apache.hadoop.fs.permission.AclEntryType.USER; import static org.apache.hadoop.fs.permission.AclEntryType.USER;
import static org.apache.hadoop.test.LambdaTestUtils.intercept;
/** /**
* Test Perform Authorization Check operation * Test Perform Authorization Check operation
@ -381,4 +384,30 @@ public void testProperties() throws Exception {
assertArrayEquals(propertyValue, fs.getXAttr(reqPath, propertyName)); assertArrayEquals(propertyValue, fs.getXAttr(reqPath, propertyName));
} }
@Test
public void testSignatureMask() throws Exception {
final AzureBlobFileSystem fs = getFileSystem();
String src = "/testABC/test.xt";
fs.create(new Path(src));
AbfsRestOperation abfsHttpRestOperation = fs.getAbfsClient()
.renamePath(src, "/testABC" + "/abc.txt", null);
AbfsHttpOperation result = abfsHttpRestOperation.getResult();
String url = result.getSignatureMaskedUrl();
String encodedUrl = result.getSignatureMaskedEncodedUrl();
Assertions.assertThat(url.substring(url.indexOf("sig=")))
.describedAs("Signature query param should be masked")
.startsWith("sig=XXXX");
Assertions.assertThat(encodedUrl.substring(encodedUrl.indexOf("sig%3D")))
.describedAs("Signature query param should be masked")
.startsWith("sig%3DXXXX");
}
@Test
public void testSignatureMaskOnExceptionMessage() throws Exception {
intercept(IOException.class, "sig=XXXX",
() -> getFileSystem().getAbfsClient()
.renamePath("testABC/test.xt", "testABC/abc.txt", null));
}
} }

View File

@ -0,0 +1,95 @@
/**
* 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
* <p>
* http://www.apache.org/licenses/LICENSE-2.0
* <p>
* 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.hadoop.fs.azurebfs.services;
import java.io.UnsupportedEncodingException;
import java.net.MalformedURLException;
import java.net.URLEncoder;
import org.assertj.core.api.Assertions;
import org.junit.Test;
public class TestAbfsHttpOperation {
@Test
public void testMaskingAndEncoding()
throws MalformedURLException, UnsupportedEncodingException {
testIfMaskAndEncodeSuccessful("Where sig is the only query param",
"http://www.testurl.net?sig=abcd", "http://www.testurl.net?sig=XXXX");
testIfMaskAndEncodeSuccessful("Where sig is the first query param",
"http://www.testurl.net?sig=abcd&abc=xyz",
"http://www.testurl.net?sig=XXXX&abc=xyz");
testIfMaskAndEncodeSuccessful(
"Where sig is neither first nor last query param",
"http://www.testurl.net?lmn=abc&sig=abcd&abc=xyz",
"http://www.testurl.net?lmn=abc&sig=XXXX&abc=xyz");
testIfMaskAndEncodeSuccessful("Where sig is the last query param",
"http://www.testurl.net?abc=xyz&sig=abcd",
"http://www.testurl.net?abc=xyz&sig=XXXX");
testIfMaskAndEncodeSuccessful("Where sig query param is not present",
"http://www.testurl.net?abc=xyz", "http://www.testurl.net?abc=xyz");
testIfMaskAndEncodeSuccessful(
"Where sig query param is not present but mysig",
"http://www.testurl.net?abc=xyz&mysig=qwerty",
"http://www.testurl.net?abc=xyz&mysig=qwerty");
testIfMaskAndEncodeSuccessful(
"Where sig query param is not present but sigmy",
"http://www.testurl.net?abc=xyz&sigmy=qwerty",
"http://www.testurl.net?abc=xyz&sigmy=qwerty");
testIfMaskAndEncodeSuccessful(
"Where sig query param is not present but a " + "value sig",
"http://www.testurl.net?abc=xyz&mnop=sig",
"http://www.testurl.net?abc=xyz&mnop=sig");
testIfMaskAndEncodeSuccessful(
"Where sig query param is not present but a " + "value ends with sig",
"http://www.testurl.net?abc=xyz&mnop=abcsig",
"http://www.testurl.net?abc=xyz&mnop=abcsig");
testIfMaskAndEncodeSuccessful(
"Where sig query param is not present but a " + "value starts with sig",
"http://www.testurl.net?abc=xyz&mnop=sigabc",
"http://www.testurl.net?abc=xyz&mnop=sigabc");
}
private void testIfMaskAndEncodeSuccessful(final String scenario,
final String url, final String expectedMaskedUrl)
throws UnsupportedEncodingException {
Assertions.assertThat(AbfsHttpOperation.getSignatureMaskedUrl(url))
.describedAs(url + " (" + scenario + ") after masking should be: "
+ expectedMaskedUrl).isEqualTo(expectedMaskedUrl);
final String expectedMaskedEncodedUrl = URLEncoder
.encode(expectedMaskedUrl, "UTF-8");
Assertions.assertThat(AbfsHttpOperation.encodedUrlStr(expectedMaskedUrl))
.describedAs(
url + " (" + scenario + ") after masking and encoding should "
+ "be: " + expectedMaskedEncodedUrl)
.isEqualTo(expectedMaskedEncodedUrl);
}
}