You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@knox.apache.org by kr...@apache.org on 2019/02/19 16:39:22 UTC

[knox] branch master updated: KNOX-1162 - Logging stacktrace for FATAL messages and displaying a meaningful error message in case of missing/non-parsable JAAS configuration (#55)

This is an automated email from the ASF dual-hosted git repository.

krisden pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/knox.git


The following commit(s) were added to refs/heads/master by this push:
     new dac2f2e  KNOX-1162 - Logging stacktrace for FATAL messages and displaying a meaningful error message in case of missing/non-parsable JAAS configuration (#55)
dac2f2e is described below

commit dac2f2e289ea4ca99afd37dadcfc623fdac19fb1
Author: Sandor Molnar <sm...@apache.org>
AuthorDate: Tue Feb 19 17:39:17 2019 +0100

    KNOX-1162 - Logging stacktrace for FATAL messages and displaying a meaningful error message in case of missing/non-parsable JAAS configuration (#55)
---
 .../org/apache/knox/gateway/GatewayMessages.java   |  6 +-
 gateway-service-remoteconfig/pom.xml               |  4 ++
 .../config/remote/zk/CuratorClientService.java     |  7 ++-
 .../zk/RemoteConfigurationRegistryJAASConfig.java  | 22 +++++++-
 .../RemoteConfigurationRegistryJAASConfigTest.java | 65 ++++++++++++++++++++++
 .../knox/gateway/util/urltemplate/MatcherTest.java |  2 +-
 6 files changed, 98 insertions(+), 8 deletions(-)

diff --git a/gateway-server/src/main/java/org/apache/knox/gateway/GatewayMessages.java b/gateway-server/src/main/java/org/apache/knox/gateway/GatewayMessages.java
index e1e27e5..6f0ade5 100644
--- a/gateway-server/src/main/java/org/apache/knox/gateway/GatewayMessages.java
+++ b/gateway-server/src/main/java/org/apache/knox/gateway/GatewayMessages.java
@@ -34,13 +34,13 @@ import java.util.Set;
 public interface GatewayMessages {
 
   @Message( level = MessageLevel.FATAL, text = "Failed to parse command line: {0}" )
-  void failedToParseCommandLine( @StackTrace( level = MessageLevel.DEBUG ) ParseException e );
+  void failedToParseCommandLine( @StackTrace( level = MessageLevel.FATAL ) ParseException e );
 
   @Message( level = MessageLevel.INFO, text = "Starting gateway..." )
   void startingGateway();
 
   @Message( level = MessageLevel.FATAL, text = "Failed to start gateway: {0}" )
-  void failedToStartGateway( @StackTrace( level = MessageLevel.DEBUG ) Exception e );
+  void failedToStartGateway( @StackTrace( level = MessageLevel.FATAL ) Exception e );
 
   @Message( level = MessageLevel.INFO, text = "Started gateway on port {0}." )
   void startedGateway( int port );
@@ -227,7 +227,7 @@ public interface GatewayMessages {
   void failedToReloadTopologies( @StackTrace( level = MessageLevel.DEBUG ) Exception e );
 
   @Message( level = MessageLevel.FATAL, text = "Unsupported encoding: {0}" )
-  void unsupportedEncoding( @StackTrace( level = MessageLevel.DEBUG ) Exception e );
+  void unsupportedEncoding( @StackTrace( level = MessageLevel.FATAL ) Exception e );
 
   @Message( level = MessageLevel.ERROR, text = "Failed to persist master secret: {0}" )
   void failedToPersistMasterSecret( @StackTrace( level = MessageLevel.DEBUG ) Exception e );
diff --git a/gateway-service-remoteconfig/pom.xml b/gateway-service-remoteconfig/pom.xml
index d5aa25b..fa32755 100644
--- a/gateway-service-remoteconfig/pom.xml
+++ b/gateway-service-remoteconfig/pom.xml
@@ -38,6 +38,10 @@
             <groupId>org.apache.knox</groupId>
             <artifactId>gateway-spi</artifactId>
         </dependency>
+        <dependency>
+            <groupId>org.apache.knox</groupId>
+            <artifactId>gateway-util-configinjector</artifactId>
+        </dependency>
 
         <dependency>
             <groupId>org.apache.zookeeper</groupId>
diff --git a/gateway-service-remoteconfig/src/main/java/org/apache/knox/gateway/service/config/remote/zk/CuratorClientService.java b/gateway-service-remoteconfig/src/main/java/org/apache/knox/gateway/service/config/remote/zk/CuratorClientService.java
index b93dc33..445b326 100644
--- a/gateway-service-remoteconfig/src/main/java/org/apache/knox/gateway/service/config/remote/zk/CuratorClientService.java
+++ b/gateway-service-remoteconfig/src/main/java/org/apache/knox/gateway/service/config/remote/zk/CuratorClientService.java
@@ -27,6 +27,7 @@ import org.apache.curator.framework.recipes.cache.PathChildrenCache;
 import org.apache.curator.framework.recipes.cache.PathChildrenCacheEvent;
 import org.apache.curator.framework.recipes.cache.PathChildrenCacheListener;
 import org.apache.curator.retry.ExponentialBackoffRetry;
+import org.apache.knox.gateway.config.ConfigurationException;
 import org.apache.knox.gateway.config.GatewayConfig;
 import org.apache.knox.gateway.i18n.messages.MessagesFactory;
 import org.apache.knox.gateway.service.config.remote.RemoteConfigurationMessages;
@@ -73,8 +74,12 @@ class CuratorClientService implements ZooKeeperClientService {
       // Load the remote registry configurations
       List<RemoteConfigurationRegistryConfig> registryConfigs = new ArrayList<>(RemoteConfigurationRegistriesAccessor.getRemoteRegistryConfigurations(config));
 
-        // Configure registry authentication
+      // Configure registry authentication
+      try {
         RemoteConfigurationRegistryJAASConfig.configure(registryConfigs, aliasService);
+      } catch (ConfigurationException e) {
+        throw new ServiceLifecycleException("Error while configuring registry authentication", e);
+      }
 
         if (registryConfigs.size() > 1) {
             // Warn about current limit on number of supported client configurations
diff --git a/gateway-service-remoteconfig/src/main/java/org/apache/knox/gateway/service/config/remote/zk/RemoteConfigurationRegistryJAASConfig.java b/gateway-service-remoteconfig/src/main/java/org/apache/knox/gateway/service/config/remote/zk/RemoteConfigurationRegistryJAASConfig.java
index 0c0d8c0..d702d17 100644
--- a/gateway-service-remoteconfig/src/main/java/org/apache/knox/gateway/service/config/remote/zk/RemoteConfigurationRegistryJAASConfig.java
+++ b/gateway-service-remoteconfig/src/main/java/org/apache/knox/gateway/service/config/remote/zk/RemoteConfigurationRegistryJAASConfig.java
@@ -16,6 +16,8 @@
  */
 package org.apache.knox.gateway.service.config.remote.zk;
 
+import org.apache.knox.gateway.config.ConfigurationException;
+import org.apache.knox.gateway.config.GatewayConfig;
 import org.apache.knox.gateway.i18n.messages.MessagesFactory;
 import org.apache.knox.gateway.service.config.remote.RemoteConfigurationMessages;
 import org.apache.knox.gateway.service.config.remote.RemoteConfigurationRegistryConfig;
@@ -34,6 +36,8 @@ import java.util.Map;
  */
 class RemoteConfigurationRegistryJAASConfig extends Configuration {
 
+   static final String JAAS_CONFIG_ERRROR_PREFIX = "Error while getting secure configuration. This error usually indicates an issue within the supplied JAAS configuration";
+
     // Underlying SASL mechanisms supported
     enum SASLMechanism {
         Unsupported,
@@ -49,17 +53,25 @@ class RemoteConfigurationRegistryJAASConfig extends Configuration {
     private static final RemoteConfigurationMessages log = MessagesFactory.get(RemoteConfigurationMessages.class);
 
     // Cache the current JAAS configuration
-    private Configuration delegate = Configuration.getConfiguration();
+    private final Configuration delegate;
 
-    private AliasService aliasService;
+    private final AliasService aliasService;
 
-    private Map<String, AppConfigurationEntry[]> contextEntries =  new HashMap<>();
+    private final Map<String, AppConfigurationEntry[]> contextEntries =  new HashMap<>();
 
     static RemoteConfigurationRegistryJAASConfig configure(List<RemoteConfigurationRegistryConfig> configs, AliasService aliasService) {
         return new RemoteConfigurationRegistryJAASConfig(configs, aliasService);
     }
 
     private RemoteConfigurationRegistryJAASConfig(List<RemoteConfigurationRegistryConfig> configs, AliasService aliasService) {
+        try {
+          delegate = Configuration.getConfiguration();
+        } catch(Exception e) {
+          //populate the original error with a meaningful message; logging will happen later in the call hierarchy
+          final String message = String.format(Locale.ROOT, "%s: %s", JAAS_CONFIG_ERRROR_PREFIX, System.getProperty(GatewayConfig.KRB5_LOGIN_CONFIG, "Undefined"));
+          throw new ConfigurationException(message, e);
+        }
+
         this.aliasService = aliasService;
 
         // Populate context entries
@@ -135,6 +147,8 @@ class RemoteConfigurationRegistryJAASConfig extends Configuration {
                 opts.put("isUseKeyTab", String.valueOf(config.isUseKeyTab()));
                 opts.put("keyTab", config.getKeytab());
                 opts.put("principal", config.getPrincipal());
+            default:
+                break;
         }
 
         if (!opts.isEmpty()) {
@@ -159,6 +173,8 @@ class RemoteConfigurationRegistryJAASConfig extends Configuration {
                 break;
             case Digest:
                 loginModuleName = digestLoginModules.get(registryType.toUpperCase(Locale.ROOT));
+            default:
+              break;
         }
         return loginModuleName;
     }
diff --git a/gateway-service-remoteconfig/src/test/java/org/apache/knox/gateway/service/config/remote/zk/RemoteConfigurationRegistryJAASConfigTest.java b/gateway-service-remoteconfig/src/test/java/org/apache/knox/gateway/service/config/remote/zk/RemoteConfigurationRegistryJAASConfigTest.java
index 752ed34..e80c823 100644
--- a/gateway-service-remoteconfig/src/test/java/org/apache/knox/gateway/service/config/remote/zk/RemoteConfigurationRegistryJAASConfigTest.java
+++ b/gateway-service-remoteconfig/src/test/java/org/apache/knox/gateway/service/config/remote/zk/RemoteConfigurationRegistryJAASConfigTest.java
@@ -16,14 +16,25 @@
  */
 package org.apache.knox.gateway.service.config.remote.zk;
 
+import org.apache.commons.io.FileUtils;
+import org.apache.knox.gateway.config.ConfigurationException;
+import org.apache.knox.gateway.config.GatewayConfig;
 import org.apache.knox.gateway.service.config.remote.RemoteConfigurationRegistryConfig;
 import org.apache.knox.gateway.services.security.AliasService;
 import org.easymock.EasyMock;
+import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.ExpectedException;
+import org.junit.rules.TemporaryFolder;
 
 import javax.security.auth.login.AppConfigurationEntry;
 import javax.security.auth.login.Configuration;
+
+import static org.hamcrest.CoreMatchers.startsWith;
+
 import java.io.File;
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
@@ -36,6 +47,12 @@ import static org.junit.Assert.fail;
 
 public class RemoteConfigurationRegistryJAASConfigTest {
 
+    @Rule
+    public final TemporaryFolder testFolder = new TemporaryFolder();
+
+    @Rule
+    public final ExpectedException expectedException = ExpectedException.none();
+
     @Test
     public void testZooKeeperDigestContextEntry() throws Exception {
         List<RemoteConfigurationRegistryConfig> registryConfigs = new ArrayList<>();
@@ -166,6 +183,54 @@ public class RemoteConfigurationRegistryJAASConfigTest {
         }
     }
 
+    @Test
+    public void shouldRaiseAnErrorWithMeaningfulErrorMessageIfAuthLoginConfigCannotBeRead() throws Exception {
+      final List<RemoteConfigurationRegistryConfig> registryConfigs = new ArrayList<>();
+      System.setProperty(GatewayConfig.KRB5_LOGIN_CONFIG, "nonExistingFilePath");
+
+      expectedException.expect(ConfigurationException.class);
+      expectedException.expectMessage(startsWith(RemoteConfigurationRegistryJAASConfig.JAAS_CONFIG_ERRROR_PREFIX));
+
+      try {
+        RemoteConfigurationRegistryJAASConfig.configure(registryConfigs, null);
+      } finally {
+        System.clearProperty(GatewayConfig.KRB5_LOGIN_CONFIG);
+      }
+    }
+
+    @Test
+    public void shouldRaiseAnErrorWithMeaningfulErrorMessageIfAuthLoginConfigCannotBeParsed() throws Exception {
+      final List<RemoteConfigurationRegistryConfig> registryConfigs = new ArrayList<>();
+      final String jaasConfigFilePath = writeInvalidJaasConf();
+      System.setProperty(GatewayConfig.KRB5_LOGIN_CONFIG, jaasConfigFilePath);
+
+      expectedException.expect(ConfigurationException.class);
+      expectedException.expectMessage(startsWith(RemoteConfigurationRegistryJAASConfig.JAAS_CONFIG_ERRROR_PREFIX));
+
+      try {
+        RemoteConfigurationRegistryJAASConfig.configure(registryConfigs, null);
+      } finally {
+        System.clearProperty(GatewayConfig.KRB5_LOGIN_CONFIG);
+      }
+    }
+
+    private String writeInvalidJaasConf() throws IOException {
+      final File jaasConfigFile = testFolder.newFile("jaas.conf");
+      final String jaasConfig = "com.sun.security.jgss.initiate {" +
+        "com.sun.security.auth.module.Krb5LoginModule required" +
+        "renewTGT=false" +
+        "doNotPrompt=true" +
+        "useKeyTab=true" +
+        "keyTab=/etc/security/keytabs/knox.service.keytab" + //note the missing quotes; it should be keyTab="/etc/security/keytabs/knox.service.keytab"
+        "principal=\"knox/myHost@myRealm\"" +
+        "storeKey=true" +
+        "useTicketCache=false; " +
+        "};";
+
+      FileUtils.writeStringToFile(jaasConfigFile, jaasConfig, StandardCharsets.UTF_8);
+      return jaasConfigFile.getAbsolutePath();
+    }
+
     private static RemoteConfigurationRegistryConfig createDigestConfig(String entryName,
                                                                         String principal,
                                                                         String credentialAlias) {
diff --git a/gateway-util-urltemplate/src/test/java/org/apache/knox/gateway/util/urltemplate/MatcherTest.java b/gateway-util-urltemplate/src/test/java/org/apache/knox/gateway/util/urltemplate/MatcherTest.java
index d34646e..03fc713 100644
--- a/gateway-util-urltemplate/src/test/java/org/apache/knox/gateway/util/urltemplate/MatcherTest.java
+++ b/gateway-util-urltemplate/src/test/java/org/apache/knox/gateway/util/urltemplate/MatcherTest.java
@@ -796,7 +796,7 @@ public class MatcherTest {
     Template template;
     Template input;
     Matcher<String> stringMatcher;
-    Matcher<?>.Match match;
+    Matcher<String>.Match match;
 
 //    template = Parser.parse( "*://*:*/**/webhdfs/v1/**?**" );
 //    input = Parser.parse( "http://localhost:53221/gateway/cluster/webhdfs/v1/tmp/GatewayWebHdfsFuncTest/testBasicHdfsUseCase/dir?user.name=hdfs&op=MKDIRS" );