Sanitize logs in JBossLoggingEventListenerProvider

Closes #25078

Signed-off-by: rmartinc <rmartinc@redhat.com>

docs/documentation/upgrading/topics/keycloak/changes-23_0_5.adoc

@@ -0,0 +1,9 @@
+= Changes in jboss-logging event messages
+
+Because of issue https://github.com/keycloak/keycloak/issues/25078[#25078], the `jboss-logging` message values are now quoted (character `"` by default) and sanitized to prevent any line break. There are two new options in the provider (`spi-events-listener-jboss-logging-sanitize` and `spi-events-listener-jboss-logging-quotes`) that allow you to customize the new behavior. For example, to avoid both sanitization and quoting, the server can be started in this manner:
+
+```
+./kc.sh start --spi-events-listener-jboss-logging-sanitize=false --spi-events-listener-jboss-logging-quotes=none ...
+```
+
+More information about the options in the https://www.keycloak.org/server/all-provider-config#_jboss_logging[all provider configuration guide].

docs/documentation/upgrading/topics/keycloak/changes.adoc

@@ -4,6 +4,10 @@
 
 include::changes-24_0_0.adoc[leveloffset=3]
 
+=== Migrating to 23.0.5
+
+include::changes-23_0_5.adoc[leveloffset=3]
+
 === Migrating to 23.0.4
 
 include::changes-23_0_4.adoc[leveloffset=3]

server-spi/src/main/java/org/keycloak/utils/StringUtil.java

@@ -56,4 +56,38 @@ public class StringUtil {
         return options.toString();
     }
 
+    /**
+     * Utility method that substitutes any isWhitespace char to common space ' ' or character 20.
+     * The idea is removing any weird space character in the string like \t, \n, \r.
+     * If quotes character is passed the quotes char is escaped to mark is not the end
+     * of the value (for example escaped \" if quotes char " is found in the string).
+     *
+     * @param str The string to normalize
+     * @param quotes The quotes to escape (for example " or '). It can be null.
+     * @return The string without weird whitespaces and quotes escaped
+     */
+    public static String sanitizeSpacesAndQuotes(String str, Character quotes) {
+        // idea taken from commons-lang StringUtils.normalizeSpace
+        if (str == null || str.isEmpty()) {
+            return str;
+        }
+        StringBuilder sb = null;
+        for (int i = 0; i < str.length(); i++) {
+            final char actualChar = str.charAt(i);
+            if ((Character.isWhitespace(actualChar) && actualChar != ' ') || actualChar == 160) {
+                if (sb == null) {
+                    sb = new StringBuilder(str.length() + 10).append(str.substring(0, i));
+                }
+                sb.append(' ');
+            } else if (quotes != null && actualChar == quotes) {
+                if (sb == null) {
+                    sb = new StringBuilder(str.length() + 10).append(str.substring(0, i));
+                }
+                sb.append('\\').append(actualChar);
+            } else if (sb != null) {
+                sb.append(actualChar);
+            }
+        }
+        return sb == null? str : sb.toString();
+    }
 }

server-spi/src/test/java/org/keycloak/utils/StringUtilTest.java

@@ -0,0 +1,38 @@
+/*
+ * Copyright 2023 Red Hat, Inc. and/or its affiliates
+ * and other contributors as indicated by the @author tags.
+ *
+ * Licensed 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.keycloak.utils;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+/**
+ *
+ * @author rmartinc
+ */
+public class StringUtilTest {
+
+    @Test
+    public void testSanitize() {
+        Assert.assertEquals("test1 test2 test3", StringUtil.sanitizeSpacesAndQuotes("test1 test2 test3", null));
+        Assert.assertEquals("test1 test2 test3", StringUtil.sanitizeSpacesAndQuotes("test1\ntest2\ttest3", null));
+        Assert.assertEquals("test1 test2 test3 \"test4\"", StringUtil.sanitizeSpacesAndQuotes("test1\ntest2\ttest3\r\"test4\"", null));
+        Assert.assertEquals("teswith\\\"quotes", StringUtil.sanitizeSpacesAndQuotes("teswith\"quotes", '"'));
+        Assert.assertEquals("test1 test2 test3 \\\"test4\\\"", StringUtil.sanitizeSpacesAndQuotes("test1\ntest2\ttest3\r\"test4\"", '"'));
+        Assert.assertEquals(" \\\"test", StringUtil.sanitizeSpacesAndQuotes("\n\"test", '"'));
+        Assert.assertEquals("\\\" test", StringUtil.sanitizeSpacesAndQuotes("\"\rtest", '"'));
+    }
+}

services/src/main/java/org/keycloak/events/log/JBossLoggingEventListenerProvider.java

@@ -17,8 +17,8 @@
 
 package org.keycloak.events.log;
 
-import org.keycloak.common.util.StackUtil;
 import org.jboss.logging.Logger;
+import org.keycloak.common.util.StackUtil;
 import org.keycloak.events.Event;
 import org.keycloak.events.EventListenerProvider;
 import org.keycloak.events.EventListenerTransaction;
@@ -26,6 +26,7 @@ import org.keycloak.events.admin.AdminEvent;
 import org.keycloak.models.KeycloakContext;
 import org.keycloak.models.KeycloakSession;
 import org.keycloak.sessions.AuthenticationSessionModel;
+import org.keycloak.utils.StringUtil;
 
 import jakarta.ws.rs.core.Cookie;
 import jakarta.ws.rs.core.HttpHeaders;
@@ -41,14 +42,18 @@ public class JBossLoggingEventListenerProvider implements EventListenerProvider
     private final Logger logger;
     private final Logger.Level successLevel;
     private final Logger.Level errorLevel;
+    private final boolean sanitize;
+    private final Character quotes;
     private final EventListenerTransaction tx = new EventListenerTransaction(this::logAdminEvent, this::logEvent);
 
-    public JBossLoggingEventListenerProvider(KeycloakSession session, Logger logger, Logger.Level successLevel, Logger.Level errorLevel) {
+    public JBossLoggingEventListenerProvider(KeycloakSession session, Logger logger,
+            Logger.Level successLevel, Logger.Level errorLevel, Character quotes, boolean sanitize) {
         this.session = session;
         this.logger = logger;
         this.successLevel = successLevel;
         this.errorLevel = errorLevel;
-
+        this.sanitize = sanitize;
+        this.quotes = quotes;
         this.session.getTransactionManager().enlistAfterCompletion(tx);
     }
 
@@ -62,6 +67,19 @@ public class JBossLoggingEventListenerProvider implements EventListenerProvider
         tx.addAdminEvent(adminEvent, includeRepresentation);
     }
 
+    private void sanitize(StringBuilder sb, String str) {
+        if (quotes != null) {
+            sb.append(quotes);
+        }
+        if (sanitize) {
+            str = StringUtil.sanitizeSpacesAndQuotes(str, quotes);
+        }
+        sb.append(str);
+        if (quotes != null) {
+            sb.append(quotes);
+        }
+    }
+
     private void logEvent(Event event) {
         Logger.Level level = event.getError() != null ? errorLevel : successLevel;
 
@@ -69,42 +87,36 @@ public class JBossLoggingEventListenerProvider implements EventListenerProvider
             StringBuilder sb = new StringBuilder();
 
             sb.append("type=");
-            sb.append(event.getType());
+            sanitize(sb, event.getType().toString());
             sb.append(", realmId=");
-            sb.append(event.getRealmId());
+            sanitize(sb, event.getRealmId());
             sb.append(", clientId=");
-            sb.append(event.getClientId());
+            sanitize(sb, event.getClientId());
             sb.append(", userId=");
-            sb.append(event.getUserId());
+            sanitize(sb, event.getUserId());
             sb.append(", ipAddress=");
-            sb.append(event.getIpAddress());
+            sanitize(sb, event.getIpAddress());
 
             if (event.getError() != null) {
                 sb.append(", error=");
-                sb.append(event.getError());
+                sanitize(sb, event.getError());
             }
 
             if (event.getDetails() != null) {
                 for (Map.Entry<String, String> e : event.getDetails().entrySet()) {
                     sb.append(", ");
-                    sb.append(e.getKey());
+                    sb.append(StringUtil.sanitizeSpacesAndQuotes(e.getKey(), null));
-                    if (e.getValue() == null || e.getValue().indexOf(' ') == -1) {
+                    sb.append("=");
-                        sb.append("=");
+                    sanitize(sb, e.getValue());
-                        sb.append(e.getValue());
-                    } else {
-                        sb.append("='");
-                        sb.append(e.getValue());
-                        sb.append("'");
-                    }
                 }
             }
 
             AuthenticationSessionModel authSession = session.getContext().getAuthenticationSession();
             if(authSession!=null) {
                 sb.append(", authSessionParentId=");
-                sb.append(authSession.getParentSession().getId());
+                sanitize(sb, authSession.getParentSession().getId());
                 sb.append(", authSessionTabId=");
-                sb.append(authSession.getTabId());
+                sanitize(sb, authSession.getTabId());
             }
 
             if(logger.isTraceEnabled()) {
@@ -126,23 +138,23 @@ public class JBossLoggingEventListenerProvider implements EventListenerProvider
             StringBuilder sb = new StringBuilder();
 
             sb.append("operationType=");
-            sb.append(adminEvent.getOperationType());
+            sanitize(sb, adminEvent.getOperationType().toString());
             sb.append(", realmId=");
-            sb.append(adminEvent.getAuthDetails().getRealmId());
+            sanitize(sb, adminEvent.getAuthDetails().getRealmId());
             sb.append(", clientId=");
-            sb.append(adminEvent.getAuthDetails().getClientId());
+            sanitize(sb, adminEvent.getAuthDetails().getClientId());
             sb.append(", userId=");
-            sb.append(adminEvent.getAuthDetails().getUserId());
+            sanitize(sb, adminEvent.getAuthDetails().getUserId());
             sb.append(", ipAddress=");
-            sb.append(adminEvent.getAuthDetails().getIpAddress());
+            sanitize(sb, adminEvent.getAuthDetails().getIpAddress());
             sb.append(", resourceType=");
-            sb.append(adminEvent.getResourceTypeAsString());
+            sanitize(sb, adminEvent.getResourceTypeAsString());
             sb.append(", resourcePath=");
-            sb.append(adminEvent.getResourcePath());
+            sanitize(sb, adminEvent.getResourcePath());
 
             if (adminEvent.getError() != null) {
                 sb.append(", error=");
-                sb.append(adminEvent.getError());
+                sanitize(sb, adminEvent.getError());
             }
 
             if(logger.isTraceEnabled()) {
@@ -163,7 +175,7 @@ public class JBossLoggingEventListenerProvider implements EventListenerProvider
         HttpHeaders headers = context.getRequestHeaders();
         if (uriInfo != null) {
             sb.append(", requestUri=");
-            sb.append(uriInfo.getRequestUri().toString());
+            sanitize(sb, uriInfo.getRequestUri().toString());
         }
 
         if (headers != null) {
@@ -175,7 +187,7 @@ public class JBossLoggingEventListenerProvider implements EventListenerProvider
                 } else {
                     sb.append(", ");
                 }
-                sb.append(e.getValue().toString());
+                sb.append(StringUtil.sanitizeSpacesAndQuotes(e.getValue().toString(), null));
             }
             sb.append("]");
         }

services/src/main/java/org/keycloak/events/log/JBossLoggingEventListenerProviderFactory.java

@@ -40,16 +40,25 @@ public class JBossLoggingEventListenerProviderFactory implements EventListenerPr
 
     private Logger.Level successLevel;
     private Logger.Level errorLevel;
+    private boolean sanitize;
+    private Character quotes;
 
     @Override
     public EventListenerProvider create(KeycloakSession session) {
-        return new JBossLoggingEventListenerProvider(session, logger, successLevel, errorLevel);
+        return new JBossLoggingEventListenerProvider(session, logger, successLevel, errorLevel, quotes, sanitize);
     }
 
     @Override
     public void init(Config.Scope config) {
         successLevel = Logger.Level.valueOf(config.get("success-level", "debug").toUpperCase());
         errorLevel = Logger.Level.valueOf(config.get("error-level", "warn").toUpperCase());
+        sanitize = config.getBoolean("sanitize", true);
+        String quotesString = config.get("quotes", "\"");
+        if (!quotesString.equals("none") && quotesString.length() > 1) {
+            logger.warn("Invalid quotes configuration, it should be none or one character to use as quotes. Using default \" quotes");
+            quotesString = "\"";
+        }
+        quotes = quotesString.equals("none")? null : quotesString.charAt(0);
     }
 
     @Override
@@ -88,6 +97,18 @@ public class JBossLoggingEventListenerProviderFactory implements EventListenerPr
                 .options(logLevels)
                 .defaultValue("warn")
                 .add()
+                .property()
+                .name("sanitize")
+                .type("boolean")
+                .helpText("If true the log messages are sanitized to avoid line breaks. If false messages are not sanitized.")
+                .defaultValue("true")
+                .add()
+                .property()
+                .name("quotes")
+                .type("string")
+                .helpText("The quotes to use for values, it should be one character like \" or '. Use \"none\" if quotes are not needed.")
+                .defaultValue("\"")
+                .add()
                 .build();
     }
 }