Avoid reflection and services lookup for JWT processing

.mvn/modernizer/violations.xml

@@ -172,4 +172,16 @@
         <version>1.8</version>
         <comment>Use io.airlift.configuration.ConfigurationAwareModule.combine</comment>
     </violation>
+
+    <violation>
+        <name>io/jsonwebtoken/Jwts.builder:()Lio/jsonwebtoken/JwtBuilder;</name>
+        <version>1.8</version>
+        <comment>Use io.trino.server.security.jwt.JwtsUtil or equivalent</comment>
+    </violation>
+
+    <violation>
+        <name>io/jsonwebtoken/Jwts.parserBuilder:()Lio/jsonwebtoken/JwtParserBuilder;</name>
+        <version>1.8</version>
+        <comment>Use io.trino.server.security.jwt.JwtsUtil or equivalent</comment>
+    </violation>
 </modernizer>

client/trino-jdbc/src/test/java/io/trino/jdbc/TestTrinoDriverAuth.java

@@ -16,7 +16,6 @@
 import com.google.common.collect.ImmutableMap;
 import io.airlift.log.Logging;
 import io.airlift.security.pem.PemReader;
-import io.jsonwebtoken.Jwts;
 import io.jsonwebtoken.security.Keys;
 import io.trino.plugin.tpch.TpchPlugin;
 import io.trino.server.testing.TestingTrinoServer;
@@ -43,6 +42,7 @@
 import static io.jsonwebtoken.JwsHeader.KEY_ID;
 import static io.jsonwebtoken.SignatureAlgorithm.HS512;
 import static io.jsonwebtoken.security.Keys.hmacShaKeyFor;
+import static io.trino.server.security.jwt.JwtUtil.newJwtBuilder;
 import static java.lang.String.format;
 import static java.nio.charset.StandardCharsets.US_ASCII;
 import static java.util.Base64.getMimeDecoder;
@@ -99,7 +99,7 @@ public void teardown()
     public void testSuccessDefaultKey()
             throws Exception
     {
-        String accessToken = Jwts.builder()
+        String accessToken = newJwtBuilder()
                 .setSubject("test")
                 .signWith(defaultKey)
                 .compact();
@@ -119,7 +119,7 @@ public void testSuccessDefaultKey()
     public void testSuccessHmac()
             throws Exception
     {
-        String accessToken = Jwts.builder()
+        String accessToken = newJwtBuilder()
                 .setSubject("test")
                 .setHeaderParam(KEY_ID, "222")
                 .signWith(hmac222)
@@ -140,7 +140,7 @@ public void testSuccessHmac()
     public void testSuccessPublicKey()
             throws Exception
     {
-        String accessToken = Jwts.builder()
+        String accessToken = newJwtBuilder()
                 .setSubject("test")
                 .setHeaderParam(KEY_ID, "33")
                 .signWith(privateKey33)
@@ -172,7 +172,7 @@ public void testFailedNoToken()
     public void testFailedUnsigned()
             throws Exception
     {
-        String accessToken = Jwts.builder()
+        String accessToken = newJwtBuilder()
                 .setSubject("test")
                 .compact();
 
@@ -188,7 +188,7 @@ public void testFailedBadHmacSignature()
             throws Exception
     {
         Key badKey = Keys.secretKeyFor(HS512);
-        String accessToken = Jwts.builder()
+        String accessToken = newJwtBuilder()
                 .setSubject("test")
                 .signWith(badKey)
                 .compact();
@@ -204,7 +204,7 @@ public void testFailedBadHmacSignature()
     public void testFailedWrongPublicKey()
             throws Exception
     {
-        String accessToken = Jwts.builder()
+        String accessToken = newJwtBuilder()
                 .setSubject("test")
                 .setHeaderParam(KEY_ID, "42")
                 .signWith(privateKey33)
@@ -221,7 +221,7 @@ public void testFailedWrongPublicKey()
     public void testFailedUnknownPublicKey()
             throws Exception
     {
-        String accessToken = Jwts.builder()
+        String accessToken = newJwtBuilder()
                 .setSubject("test")
                 .setHeaderParam(KEY_ID, "unknown")
                 .signWith(privateKey33)
@@ -238,7 +238,7 @@ public void testFailedUnknownPublicKey()
     public void testSuccessFullSslVerification()
             throws Exception
     {
-        String accessToken = Jwts.builder()
+        String accessToken = newJwtBuilder()
                 .setSubject("test")
                 .setHeaderParam(KEY_ID, "33")
                 .signWith(privateKey33)
@@ -259,7 +259,7 @@ public void testSuccessFullSslVerification()
     public void testSuccessCaSslVerification()
             throws Exception
     {
-        String accessToken = Jwts.builder()
+        String accessToken = newJwtBuilder()
                 .setSubject("test")
                 .setHeaderParam(KEY_ID, "33")
                 .signWith(privateKey33)

core/trino-main/pom.xml

@@ -258,6 +258,11 @@
             <artifactId>jjwt-impl</artifactId>
         </dependency>
 
+        <dependency>
+            <groupId>io.jsonwebtoken</groupId>
+            <artifactId>jjwt-jackson</artifactId>
+        </dependency>
+
         <dependency>
             <groupId>it.unimi.dsi</groupId>
             <artifactId>fastutil</artifactId>
@@ -352,12 +357,6 @@
             <scope>runtime</scope>
         </dependency>
 
-        <dependency>
-            <groupId>io.jsonwebtoken</groupId>
-            <artifactId>jjwt-jackson</artifactId>
-            <scope>runtime</scope>
-        </dependency>
-
         <dependency>
             <groupId>org.assertj</groupId>
             <artifactId>assertj-core</artifactId>

core/trino-main/src/main/java/io/trino/server/InternalAuthenticationManager.java

@@ -19,7 +19,7 @@
 import io.airlift.log.Logger;
 import io.airlift.node.NodeInfo;
 import io.jsonwebtoken.JwtException;
-import io.jsonwebtoken.Jwts;
+import io.jsonwebtoken.JwtParser;
 import io.trino.server.security.InternalPrincipal;
 import io.trino.spi.security.Identity;
 
@@ -34,6 +34,8 @@
 import static io.airlift.http.client.Request.Builder.fromRequest;
 import static io.jsonwebtoken.security.Keys.hmacShaKeyFor;
 import static io.trino.server.ServletSecurityUtils.setAuthenticatedIdentity;
+import static io.trino.server.security.jwt.JwtUtil.newJwtBuilder;
+import static io.trino.server.security.jwt.JwtUtil.newJwtParserBuilder;
 import static java.nio.charset.StandardCharsets.UTF_8;
 import static java.util.Objects.requireNonNull;
 import static javax.ws.rs.core.MediaType.TEXT_PLAIN_TYPE;
@@ -48,6 +50,7 @@ public class InternalAuthenticationManager
 
     private final Key hmac;
     private final String nodeId;
+    private final JwtParser jwtParser;
 
     @Inject
     public InternalAuthenticationManager(InternalCommunicationConfig internalCommunicationConfig, NodeInfo nodeInfo)
@@ -75,6 +78,7 @@ public InternalAuthenticationManager(String sharedSecret, String nodeId)
         requireNonNull(nodeId, "nodeId is null");
         this.hmac = hmacShaKeyFor(Hashing.sha256().hashString(sharedSecret, UTF_8).asBytes());
         this.nodeId = nodeId;
+        this.jwtParser = newJwtParserBuilder().setSigningKey(hmac).build();
     }
 
     public static boolean isInternalRequest(ContainerRequestContext request)
@@ -115,7 +119,7 @@ public Request filterRequest(Request request)
 
     private String generateJwt()
     {
-        return Jwts.builder()
+        return newJwtBuilder()
                 .signWith(hmac)
                 .setSubject(nodeId)
                 .setExpiration(Date.from(ZonedDateTime.now().plusMinutes(5).toInstant()))
@@ -124,9 +128,7 @@ private String generateJwt()
 
     private String parseJwt(String jwt)
     {
-        return Jwts.parserBuilder()
-                .setSigningKey(hmac)
-                .build()
+        return jwtParser
                 .parseClaimsJws(jwt)
                 .getBody()
                 .getSubject();

core/trino-main/src/main/java/io/trino/server/security/jwt/JwtAuthenticator.java

@@ -15,7 +15,6 @@
 
 import io.jsonwebtoken.JwtParser;
 import io.jsonwebtoken.JwtParserBuilder;
-import io.jsonwebtoken.Jwts;
 import io.jsonwebtoken.SigningKeyResolver;
 import io.trino.server.security.AbstractBearerAuthenticator;
 import io.trino.server.security.AuthenticationException;
@@ -30,6 +29,7 @@
 import java.util.Optional;
 
 import static io.trino.server.security.UserMapping.createUserMapping;
+import static io.trino.server.security.jwt.JwtUtil.newJwtParserBuilder;
 
 public class JwtAuthenticator
         extends AbstractBearerAuthenticator
@@ -43,7 +43,7 @@ public JwtAuthenticator(JwtAuthenticatorConfig config, @ForJwt SigningKeyResolve
     {
         principalField = config.getPrincipalField();
 
-        JwtParserBuilder jwtParser = Jwts.parserBuilder()
+        JwtParserBuilder jwtParser = newJwtParserBuilder()
                 .setSigningKeyResolver(signingKeyResolver);
 
         if (config.getRequiredIssuer() != null) {

core/trino-main/src/main/java/io/trino/server/security/jwt/JwtUtil.java

@@ -0,0 +1,46 @@
+/*
+ * 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 io.trino.server.security.jwt;
+
+import io.jsonwebtoken.JwtBuilder;
+import io.jsonwebtoken.JwtParserBuilder;
+import io.jsonwebtoken.impl.DefaultJwtBuilder;
+import io.jsonwebtoken.impl.DefaultJwtParserBuilder;
+import io.jsonwebtoken.io.Deserializer;
+import io.jsonwebtoken.io.Serializer;
+import io.jsonwebtoken.jackson.io.JacksonDeserializer;
+import io.jsonwebtoken.jackson.io.JacksonSerializer;
+
+import java.util.Map;
+
+// avoid reflection and services lookup
+public final class JwtUtil
+{
+    private static final Serializer<Map<String, ?>> JWT_SERIALIZER = new JacksonSerializer<>();
+    private static final Deserializer<Map<String, ?>> JWT_DESERIALIZER = new JacksonDeserializer<>();
+
+    private JwtUtil() {}
+
+    public static JwtBuilder newJwtBuilder()
+    {
+        return new DefaultJwtBuilder()
+                .serializeToJsonWith(JWT_SERIALIZER);
+    }
+
+    public static JwtParserBuilder newJwtParserBuilder()
+    {
+        return new DefaultJwtParserBuilder()
+                .deserializeJsonWith(JWT_DESERIALIZER);
+    }
+}

core/trino-main/src/main/java/io/trino/server/security/oauth2/OAuth2Service.java

@@ -23,7 +23,7 @@
 import io.airlift.http.client.Request;
 import io.airlift.log.Logger;
 import io.jsonwebtoken.Claims;
-import io.jsonwebtoken.Jwts;
+import io.jsonwebtoken.JwtParser;
 import io.jsonwebtoken.SigningKeyResolver;
 import io.jsonwebtoken.impl.DefaultClaims;
 import io.trino.server.security.oauth2.OAuth2Client.OAuth2Response;
@@ -57,6 +57,8 @@
 import static io.airlift.json.JsonCodec.mapJsonCodec;
 import static io.jsonwebtoken.Claims.AUDIENCE;
 import static io.jsonwebtoken.security.Keys.hmacShaKeyFor;
+import static io.trino.server.security.jwt.JwtUtil.newJwtBuilder;
+import static io.trino.server.security.jwt.JwtUtil.newJwtParserBuilder;
 import static io.trino.server.security.oauth2.OAuth2CallbackResource.CALLBACK_ENDPOINT;
 import static io.trino.server.ui.FormWebUiAuthenticationFilter.UI_LOCATION;
 import static java.lang.String.format;
@@ -90,6 +92,7 @@ public class OAuth2Service
     private final Set<String> scopes;
     private final TemporalAmount challengeTimeout;
     private final Key stateHmac;
+    private final JwtParser jwtParser;
 
     private final HttpClient httpClient;
     private final String issuer;
@@ -119,6 +122,10 @@ public OAuth2Service(OAuth2Client client, @ForOAuth2 SigningKeyResolver signingK
         this.stateHmac = hmacShaKeyFor(oauth2Config.getStateKey()
                 .map(key -> sha256().hashString(key, UTF_8).asBytes())
                 .orElseGet(() -> secureRandomBytes(32)));
+        this.jwtParser = newJwtParserBuilder()
+                .setSigningKey(stateHmac)
+                .requireAudience(STATE_AUDIENCE_UI)
+                .build();
 
         this.httpClient = requireNonNull(httpClient, "httpClient is null");
         this.issuer = oauth2Config.getIssuer();
@@ -149,15 +156,10 @@ public Response startOAuth2Challenge(UriInfo uriInfo, String handlerState)
                 Optional.of(handlerState));
     }
 
-    public Response startOAuth2Challenge(URI callbackUri, String handlerState)
-    {
-        return startOAuth2Challenge(callbackUri, Optional.of(handlerState));
-    }
-
     private Response startOAuth2Challenge(URI callbackUri, Optional<String> handlerState)
     {
         Instant challengeExpiration = now().plus(challengeTimeout);
-        String state = Jwts.builder()
+        String state = newJwtBuilder()
                 .signWith(stateHmac)
                 .setAudience(STATE_AUDIENCE_UI)
                 .claim(HANDLER_STATE_CLAIM, handlerState.orElse(null))
@@ -279,10 +281,7 @@ private Claims parseState(String state)
             throws ChallengeFailedException
     {
         try {
-            return Jwts.parserBuilder()
-                    .setSigningKey(stateHmac)
-                    .requireAudience(STATE_AUDIENCE_UI)
-                    .build()
+            return jwtParser
                     .parseClaimsJws(state)
                     .getBody();
         }
@@ -304,7 +303,7 @@ private void validateIdTokenAndNonce(OAuth2Response oauth2Response, Optional<Str
     {
         if (nonce.isPresent() && oauth2Response.getIdToken().isPresent()) {
             // validate ID token including nonce
-            Claims claims = Jwts.parserBuilder()
+            Claims claims = newJwtParserBuilder()
                     .setSigningKeyResolver(signingKeyResolver)
                     .requireIssuer(issuer)
                     .require(NONCE, hashNonce(nonce.get()))
@@ -350,7 +349,7 @@ private Optional<Claims> internalConvertTokenToClaims(String accessToken)
         }
 
         // validate access token is trusted by this server
-        Claims claims = Jwts.parserBuilder()
+        Claims claims = newJwtParserBuilder()
                 .setSigningKeyResolver(signingKeyResolver)
                 .requireIssuer(accessTokenIssuer)
                 .build()