[
https://issues.apache.org/jira/browse/KAFKA-5117?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16687038#comment-16687038
]
ASF GitHub Bot commented on KAFKA-5117:
---------------------------------------
qiao-meng-zefr closed pull request #4269: KAFKA-5117: Add password masking for
kafka connect REST endpoint
URL: https://github.com/apache/kafka/pull/4269
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git
a/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/AbstractHerder.java
b/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/AbstractHerder.java
index fbe0ae2afb2..9eb2def0c64 100644
---
a/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/AbstractHerder.java
+++
b/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/AbstractHerder.java
@@ -184,6 +184,22 @@ public void resumeConnector(String connector) {
configBackingStore.putTargetState(connector, TargetState.STARTED);
}
+ @Override
+ public Map<String, String> maskCredentials(String connName, Map<String,
String> config) {
+ Map<String, String> newConfig = new LinkedHashMap<>();
+ for (Map.Entry<String, String> entry : config.entrySet()) {
+ // Password.toString() will return the hidden value
+ String value = null;
+ if (entry.getValue() != null) {
+ value = entry.getValue().toString();
+ }
+
+ newConfig.put(entry.getKey(), value);
+ }
+
+ return newConfig;
+ }
+
@Override
public Plugins plugins() {
return worker.getPlugins();
diff --git
a/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/Herder.java
b/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/Herder.java
index 5dfb808f764..774dbaf8ca1 100644
--- a/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/Herder.java
+++ b/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/Herder.java
@@ -169,6 +169,14 @@
*/
void resumeConnector(String connector);
+ /**
+ * Goes through config parameters and replace password field value with
"[hidden"]
+ * @param connName name of the connector
+ * @param config configuration of the connector
+ * @return new map of the configurations, with password omitted from
clear-text
+ */
+ Map<String, String> maskCredentials(String connName, Map<String, String>
config);
+
/**
* Returns a handle to the plugin factory used by this herder and its
worker.
*
diff --git
a/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/rest/resources/ConnectorsResource.java
b/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/rest/resources/ConnectorsResource.java
index 2c031245c06..6e19cc729a8 100644
---
a/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/rest/resources/ConnectorsResource.java
+++
b/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/rest/resources/ConnectorsResource.java
@@ -50,6 +50,7 @@
import java.util.Collection;
import java.util.List;
import java.util.Map;
+import java.util.ArrayList;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.TimeoutException;
@@ -109,7 +110,8 @@ public ConnectorInfo getConnector(final
@PathParam("connector") String connector
final @QueryParam("forward") Boolean
forward) throws Throwable {
FutureCallback<ConnectorInfo> cb = new FutureCallback<>();
herder.connectorInfo(connector, cb);
- return completeOrForwardRequest(cb, "/connectors/" + connector, "GET",
null, forward);
+ ConnectorInfo connectorInfo = completeOrForwardRequest(cb,
"/connectors/" + connector, "GET", null, forward);
+ return new ConnectorInfo(connectorInfo.name(),
herder.maskCredentials(connector, connectorInfo.config()),
connectorInfo.tasks(), connectorInfo.type());
}
@GET
@@ -118,7 +120,8 @@ public ConnectorInfo getConnector(final
@PathParam("connector") String connector
final @QueryParam("forward")
Boolean forward) throws Throwable {
FutureCallback<Map<String, String>> cb = new FutureCallback<>();
herder.connectorConfig(connector, cb);
- return completeOrForwardRequest(cb, "/connectors/" + connector +
"/config", "GET", null, forward);
+ Map<String, String> config = completeOrForwardRequest(cb,
"/connectors/" + connector + "/config", "GET", null, forward);
+ return herder.maskCredentials(connector, config);
}
@GET
@@ -177,8 +180,14 @@ public Response resumeConnector(@PathParam("connector")
String connector) {
final @QueryParam("forward") Boolean
forward) throws Throwable {
FutureCallback<List<TaskInfo>> cb = new FutureCallback<>();
herder.taskConfigs(connector, cb);
- return completeOrForwardRequest(cb, "/connectors/" + connector +
"/tasks", "GET", null, new TypeReference<List<TaskInfo>>() {
- }, forward);
+ List<TaskInfo> taskInfoList = completeOrForwardRequest(cb,
"/connectors/" + connector + "/tasks", "GET", null,
+ new TypeReference<List<TaskInfo>>() {
+ }, forward);
+ List<TaskInfo> maskedTaskInfoList = new ArrayList<>();
+ for (TaskInfo taskInfo : taskInfoList) {
+ maskedTaskInfoList.add(new TaskInfo(taskInfo.id(),
herder.maskCredentials(connector, taskInfo.config())));
+ }
+ return maskedTaskInfoList;
}
@POST
diff --git
a/connect/runtime/src/test/java/org/apache/kafka/connect/runtime/rest/resources/ConnectorsResourceTest.java
b/connect/runtime/src/test/java/org/apache/kafka/connect/runtime/rest/resources/ConnectorsResourceTest.java
index 89a221835ef..614eff9d585 100644
---
a/connect/runtime/src/test/java/org/apache/kafka/connect/runtime/rest/resources/ConnectorsResourceTest.java
+++
b/connect/runtime/src/test/java/org/apache/kafka/connect/runtime/rest/resources/ConnectorsResourceTest.java
@@ -268,7 +268,10 @@ public void testDeleteConnectorNotFound() throws Throwable
{
@Test
public void testGetConnector() throws Throwable {
+ EasyMock.expect(herder.maskCredentials(CONNECTOR_NAME,
CONNECTOR_CONFIG)).andReturn(CONNECTOR_CONFIG);
+
final Capture<Callback<ConnectorInfo>> cb = Capture.newInstance();
+
herder.connectorInfo(EasyMock.eq(CONNECTOR_NAME),
EasyMock.capture(cb));
expectAndCallbackResult(cb, new ConnectorInfo(CONNECTOR_NAME,
CONNECTOR_CONFIG, CONNECTOR_TASK_NAMES,
ConnectorType.SOURCE));
@@ -276,6 +279,7 @@ public void testGetConnector() throws Throwable {
PowerMock.replayAll();
ConnectorInfo connInfo =
connectorsResource.getConnector(CONNECTOR_NAME, FORWARD);
+
assertEquals(new ConnectorInfo(CONNECTOR_NAME, CONNECTOR_CONFIG,
CONNECTOR_TASK_NAMES, ConnectorType.SOURCE),
connInfo);
@@ -284,13 +288,15 @@ public void testGetConnector() throws Throwable {
@Test
public void testGetConnectorConfig() throws Throwable {
+ EasyMock.expect(herder.maskCredentials(CONNECTOR_NAME,
CONNECTOR_CONFIG)).andReturn(CONNECTOR_CONFIG);
+
final Capture<Callback<Map<String, String>>> cb =
Capture.newInstance();
herder.connectorConfig(EasyMock.eq(CONNECTOR_NAME),
EasyMock.capture(cb));
expectAndCallbackResult(cb, CONNECTOR_CONFIG);
PowerMock.replayAll();
-
Map<String, String> connConfig =
connectorsResource.getConnectorConfig(CONNECTOR_NAME, FORWARD);
+
assertEquals(CONNECTOR_CONFIG, connConfig);
PowerMock.verifyAll();
@@ -298,6 +304,8 @@ public void testGetConnectorConfig() throws Throwable {
@Test(expected = NotFoundException.class)
public void testGetConnectorConfigConnectorNotFound() throws Throwable {
+ EasyMock.expect(herder.maskCredentials(CONNECTOR_NAME,
CONNECTOR_CONFIG)).andReturn(CONNECTOR_CONFIG);
+
final Capture<Callback<Map<String, String>>> cb =
Capture.newInstance();
herder.connectorConfig(EasyMock.eq(CONNECTOR_NAME),
EasyMock.capture(cb));
expectAndCallbackException(cb, new NotFoundException("not found"));
@@ -375,6 +383,10 @@ public void testCreateConnectorConfigNameMismatch() throws
Throwable {
@Test
public void testGetConnectorTaskConfigs() throws Throwable {
+ for (int i = 0; i < TASK_CONFIGS.size(); i++) {
+ EasyMock.expect(herder.maskCredentials(CONNECTOR_NAME,
TASK_CONFIGS.get(i))).andReturn(TASK_CONFIGS.get(i));
+ }
+
final Capture<Callback<List<TaskInfo>>> cb = Capture.newInstance();
herder.taskConfigs(EasyMock.eq(CONNECTOR_NAME), EasyMock.capture(cb));
expectAndCallbackResult(cb, TASK_INFOS);
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Kafka Connect REST endpoints reveal Password typed values
> ---------------------------------------------------------
>
> Key: KAFKA-5117
> URL: https://issues.apache.org/jira/browse/KAFKA-5117
> Project: Kafka
> Issue Type: Bug
> Components: KafkaConnect
> Affects Versions: 0.10.2.0
> Reporter: Thomas Holmes
> Assignee: Ewen Cheslack-Postava
> Priority: Major
> Labels: needs-kip
> Fix For: 2.0.0
>
>
> A Kafka Connect connector can specify ConfigDef keys as type of Password.
> This type was added to prevent logging the values (instead "[hidden]" is
> logged).
> This change does not apply to the values returned by executing a GET on
> {{connectors/\{connector-name\}}} and
> {{connectors/\{connector-name\}/config}}. This creates an easily accessible
> way for an attacker who has infiltrated your network to gain access to
> potential secrets that should not be available.
> I have started on a code change that addresses this issue by parsing the
> config values through the ConfigDef for the connector and returning their
> output instead (which leads to the masking of Password typed configs as
> [hidden]).
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
