oscerd commented on code in PR #26803:
URL: https://github.com/apache/camel/pull/26803#discussion_r4091011895
##########
core/camel-util/src/main/java/org/apache/camel/util/URISupport.java:
##########
@@ -51,20 +53,18 @@ public final class URISupport {
public static final char[] RAW_TOKEN_START = { '(', '{' };
public static final char[] RAW_TOKEN_END = { ')', '}' };
- @SuppressWarnings("RegExpUnnecessaryNonCapturingGroup")
- private static final String PRE_SECRETS_FORMAT =
"([?&][^=]*(?:%s)[^=]*)=(RAW(([{][^}]*[}])|([(][^)]*[)]))|[^&]*)";
+ // Match the key of a query parameter as first capture group (the value
starts after the = sign)
+ private static final Pattern QUERY_PARAMETER_KEY =
Pattern.compile("[?&]([^?&=]*)=");
- // Match any key-value pair in the URI query string whose key contains
- // "passphrase" or "password" or secret key (case-insensitive).
- // First capture group is the key, second is the value.
- private static final Pattern ALL_SECRETS
- =
Pattern.compile(PRE_SECRETS_FORMAT.formatted(SensitiveUtils.getSensitivePattern()),
- Pattern.CASE_INSENSITIVE);
+ // Match any of the sensitive keywords (such as passphrase, password or
secret key) in a query parameter key
+ private static final Pattern SENSITIVE_KEYWORDS
+ = Pattern.compile(SensitiveUtils.getSensitivePattern(),
Pattern.CASE_INSENSITIVE);
// Match the user password in the URI as second capture group
// (applies to URI with authority component and userinfo token in the form
- // "user:password").
- private static final Pattern USERINFO_PASSWORD =
Pattern.compile("(.*://.*?:)(.*)(@)");
+ // "user:password"). The authority ends at the first / or ? and the
userinfo
+ // ends at the last @ in the authority, which is how normalizeUri reads it.
+ private static final Pattern USERINFO_PASSWORD =
Pattern.compile("(://[^/?:]*:)([^/?]*)(@)");
Review Comment:
Thanks, confirmed: 4.22.1 masks `ftp://joe:pa/ss@host/dir` and
`ftp://joe:pa?ss@host/dir`, this branch did not, and the comment about
`normalizeUri` was wrong (it returns such a URI unchanged).
Fixed in f477989959ba with the two-stage match you suggested, now in one
helper in `SensitiveUtils` (see the review reply). The userinfo ends at the
last `@` before the path or query. When there is no `@` there, the password may
contain `/` or `?` and ends at the last `@` before a query parameter (`?key=`
or `&key=`), the next URI or, in free text, a whitespace or a quote. That keeps
`smtp://host:[email protected]` intact.
The one shape that stays unmasked is a password that itself contains
`?key=value`, since it cannot be told apart from `host:port?key=value@...`. It
is pinned in `testSanitizeUriWithUserInfoPasswordWithSlashOrQuestionMark` and
described in the javadoc. I also ran 68 generated inputs (passwords with `@`,
`/`, `?`, `:`, `&`, `=`, `#`, spaces and quotes, in plain URIs, URIs with
queries, labels and route strings) against the old pattern: that is the only
shape where the new rule masks less.
_Claude Code on behalf of oscerd_
##########
core/camel-util/src/main/java/org/apache/camel/util/URISupport.java:
##########
@@ -88,16 +89,19 @@ private URISupport() {
* @param keywords keywords separated by comma
*/
public static synchronized void addSanitizeKeywords(String keywords) {
Review Comment:
Kept the accumulation: it matches the javadoc (a keyword cannot be removed)
and only ever masks more.
In f477989959ba the comment in
`DefaultCamelContextExtension.setAdditionalSensitiveKeywords` now says the
keywords apply to the whole JVM and cannot be removed, and the 4.23 upgrade
guide has a note (`camel-core - masking of sensitive values in endpoint URIs`)
covering the keyword change and the other changes to the masked output.
_Claude Code on behalf of oscerd_
##########
core/camel-util/src/main/java/org/apache/camel/util/URISupport.java:
##########
@@ -106,21 +110,101 @@ public static synchronized void
addSanitizeKeywords(String keywords) {
* @param uri The uri to sanitize.
* @return Returns null if the uri is null, otherwise the URI with the
passphrase, password or secretKey
* sanitized.
- * @see #ALL_SECRETS and #USERINFO_PASSWORD for the matched pattern
+ * @see #USERINFO_PASSWORD for the matched pattern
*/
public static String sanitizeUri(String uri) {
// use xxxxx as replacement as that works well with JMX also
String sanitized = uri;
if (uri != null) {
- sanitized = ALL_SECRETS.matcher(sanitized).replaceAll("$1=xxxxxx");
- if (EXTRA_SECRETS != null) {
- sanitized =
EXTRA_SECRETS.matcher(sanitized).replaceFirst("$1=xxxxxx");
- }
- sanitized =
USERINFO_PASSWORD.matcher(sanitized).replaceFirst("$1xxxxxx$3");
+ sanitized = sanitizeQueryParameters(sanitized);
+ sanitized =
USERINFO_PASSWORD.matcher(sanitized).replaceAll("$1xxxxxx$3");
}
return sanitized;
}
+ /**
+ * Returns a copy of the parameters where the values of sensitive
parameters (such as passwords) are masked, using
+ * the same rules as {@link #sanitizeUri(String)}.
+ *
+ * @param parameters the parameters
+ * @return a copy of the parameters with the sensitive values
masked
+ */
+ public static Map<String, Object> sanitizeParameters(Map<String, Object>
parameters) {
+ Map<String, Object> answer = new LinkedHashMap<>(parameters.size());
Review Comment:
`sanitizeParameters(null)` now returns `null`, like `sanitizeUri(null)`,
with a test (f477989959ba).
_Claude Code on behalf of oscerd_
--
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]
