xiangfu0 commented on code in PR #19775:
URL: https://github.com/apache/pinot/pull/19775#discussion_r4290440281
##########
pinot-spi/src/main/java/org/apache/pinot/spi/stream/StreamConfig.java:
##########
@@ -49,6 +52,13 @@ public class StreamConfig {
public static final double CONSUMPTION_RATE_LIMIT_NOT_SPECIFIED = -1;
+ // Matches "username=" or "password=" key-value pairs embedded inside a
single config value, e.g. a Kafka
+ // sasl.jaas.config login-module string: `... required username="foo"
password="bar";`. Obfuscator (used below)
+ // only masks values of fields whose *key* looks sensitive, so it cannot
redact a secret that is embedded inside
+ // the value of an otherwise innocuous-looking key such as
"sasl.jaas.config".
+ private static final Pattern EMBEDDED_CREDENTIAL_PATTERN =
+ Pattern.compile("((?:username|password)\\s*=\\s*\")[^\"]*(\")",
Pattern.CASE_INSENSITIVE);
Review Comment:
Nice targeted heuristic for the JAAS case. One small gap: this only matches
double-quoted values, so a login-module string written with single quotes or
unquoted values (both of which JAAS accepts) would slip through. Not a blocker,
but worth a short comment noting the double-quote assumption so a future reader
knows the limit.
##########
pinot-spi/src/main/java/org/apache/pinot/spi/stream/StreamConfig.java:
##########
@@ -443,6 +453,22 @@ public Map<String, String> getStreamConfigsMap() {
return _streamConfigMap;
}
+ /// Returns a copy of the given config map with sensitive values masked,
suitable for logging.
+ ///
+ /// Keys that look sensitive (password, secret, token, apiKey, keytab
suffixes, see [Obfuscator]) have their
+ /// whole value masked. In addition, any `username="..."` / `password="..."`
pair embedded inside a value (as
+ /// found in a Kafka `sasl.jaas.config` login-module string) is redacted,
since such a secret is not the value
+ /// of a sensitive-looking key but is nested inside the value of an
unrelated one.
+ private static Map<String, String> redactSensitiveValues(Map<String, String>
configMap) {
+ JsonNode maskedByKey = Obfuscator.DEFAULT.toJson(configMap);
+ Map<String, String> redacted = new HashMap<>();
+ for (String key : configMap.keySet()) {
+ String value = maskedByKey.get(key).asText();
+ redacted.put(key,
EMBEDDED_CREDENTIAL_PATTERN.matcher(value).replaceAll("$1*****$2"));
Review Comment:
Worth keeping in mind that key-based masking only covers the suffixes
Obfuscator knows about (password, secret, token, etc.). A key like a plain
'username' or an AWS 'secret.access.key' value sitting in its own config entry
would still print in the clear here. That is an existing Obfuscator limitation
rather than something this PR introduces, so I would not block on it, but it is
good to be aware the redaction is not exhaustive.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
