Enable internal JWT by default

trinodb · Feb 2, 2020 · 79639e5 · 79639e5
1 parent 189018f
commit 79639e5
Show file tree

Hide file tree

Showing 4 changed files with 28 additions and 8 deletions.
diff --git a/presto-docs/src/main/sphinx/security/internal-communication.rst b/presto-docs/src/main/sphinx/security/internal-communication.rst
@@ -8,8 +8,8 @@ between Presto nodes can be secured with SSL/TLS.
 Internal Authentication
 -----------------------
 
-Requests between Presto nodes are authenticated using a shared secret.  The shared
-secret must be configured on all nodes in the cluster:
+Requests between Presto nodes are authenticated using a shared secret. For secure
+internal communication, the shared secret must be configured on all nodes in the cluster:
 
     .. code-block:: none
 

diff --git a/presto-main/src/main/java/io/prestosql/server/InternalAuthenticationManager.java b/presto-main/src/main/java/io/prestosql/server/InternalAuthenticationManager.java
@@ -29,7 +29,6 @@
 import java.security.Principal;
 import java.time.ZonedDateTime;
 import java.util.Date;
-import java.util.Optional;
 
 import static io.airlift.http.client.Request.Builder.fromRequest;
 import static java.nio.charset.StandardCharsets.UTF_8;
@@ -49,16 +48,21 @@ public class InternalAuthenticationManager
     @Inject
     public InternalAuthenticationManager(InternalCommunicationConfig internalCommunicationConfig, NodeInfo nodeInfo)
     {
-        this(requireNonNull(internalCommunicationConfig, "internalCommunicationConfig is null").getSharedSecret(), nodeInfo.getNodeId());
+        this(
+                requireNonNull(internalCommunicationConfig, "internalCommunicationConfig is null")
+                        .getSharedSecret()
+                        .orElse(requireNonNull(nodeInfo, "nodeInfo is null").getEnvironment()),
+                nodeInfo.getNodeId(),
+                internalCommunicationConfig.isInternalJwtEnabled());
     }
 
-    public InternalAuthenticationManager(Optional<String> sharedSecret, String nodeId)
+    public InternalAuthenticationManager(String sharedSecret, String nodeId, boolean internalJwtEnabled)
     {
         requireNonNull(sharedSecret, "sharedSecret is null");
         requireNonNull(nodeId, "nodeId is null");
-        this.internalJwtEnabled = sharedSecret.isPresent();
+        this.internalJwtEnabled = internalJwtEnabled;
         if (internalJwtEnabled) {
-            this.hmac = Hashing.sha256().hashString(sharedSecret.get(), UTF_8).asBytes();
+            this.hmac = Hashing.sha256().hashString(sharedSecret, UTF_8).asBytes();
         }
         else {
             this.hmac = null;
@@ -74,7 +78,7 @@ public boolean isInternalRequest(HttpServletRequest request)
     public Principal authenticateInternalRequest(HttpServletRequest request)
     {
         if (!internalJwtEnabled) {
-            log.error("Internal authentication in not configured");
+            log.error("Internal authentication is not enabled");
             return null;
         }
 

diff --git a/presto-main/src/main/java/io/prestosql/server/InternalCommunicationConfig.java b/presto-main/src/main/java/io/prestosql/server/InternalCommunicationConfig.java
@@ -25,6 +25,7 @@ public class InternalCommunicationConfig
     public static final String INTERNAL_COMMUNICATION_KERBEROS_ENABLED = "internal-communication.kerberos.enabled";
 
     private String sharedSecret;
+    private boolean internalJwtEnabled = true;
     private boolean httpsRequired;
     private String keyStorePath;
     private String keyStorePassword;
@@ -47,6 +48,18 @@ public InternalCommunicationConfig setSharedSecret(String sharedSecret)
         return this;
     }
 
+    public boolean isInternalJwtEnabled()
+    {
+        return internalJwtEnabled;
+    }
+
+    @Config("internal-communication.jwt.enabled")
+    public InternalCommunicationConfig setInternalJwtEnabled(boolean internalJwtEnabled)
+    {
+        this.internalJwtEnabled = internalJwtEnabled;
+        return this;
+    }
+
     public boolean isHttpsRequired()
     {
         return httpsRequired;

diff --git a/presto-main/src/test/java/io/prestosql/server/TestInternalCommunicationConfig.java b/presto-main/src/test/java/io/prestosql/server/TestInternalCommunicationConfig.java
@@ -29,6 +29,7 @@ public void testDefaults()
     {
         assertRecordedDefaults(recordDefaults(InternalCommunicationConfig.class)
                 .setSharedSecret(null)
+                .setInternalJwtEnabled(true)
                 .setHttpsRequired(false)
                 .setKeyStorePath(null)
                 .setKeyStorePassword(null)
@@ -43,6 +44,7 @@ public void testExplicitPropertyMappings()
     {
         Map<String, String> properties = new ImmutableMap.Builder<String, String>()
                 .put("internal-communication.shared-secret", "secret")
+                .put("internal-communication.jwt.enabled", "false")
                 .put("internal-communication.https.required", "true")
                 .put("internal-communication.https.keystore.path", "key-path")
                 .put("internal-communication.https.keystore.key", "key-key")
@@ -54,6 +56,7 @@ public void testExplicitPropertyMappings()
 
         InternalCommunicationConfig expected = new InternalCommunicationConfig()
                 .setSharedSecret("secret")
+                .setInternalJwtEnabled(false)
                 .setHttpsRequired(true)
                 .setKeyStorePath("key-path")
                 .setKeyStorePassword("key-key")