YARN-10814. Fallback to RandomSecretProvider if the secret file is empty (#3206)

The rest endpoint would be unusable with an empty secret file
(throwing IllegalArgumentExceptions).

Any IO error would have resulted in the same fallback path.

Co-authored-by: Tamas Domok <tdomok@cloudera.com>

hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/server/AuthenticationFilter.java

@@ -237,8 +237,8 @@ public static SignerSecretProvider constructSecretProvider(
         provider.init(config, ctx, validity);
       } catch (Exception e) {
         if (!disallowFallbackToRandomSecretProvider) {
-          LOG.info("Unable to initialize FileSignerSecretProvider, " +
+          LOG.warn("Unable to initialize FileSignerSecretProvider, " +
-                       "falling back to use random secrets.");
+              "falling back to use random secrets. Reason: " + e.getMessage());
           provider = new RandomSignerSecretProvider();
           provider.init(config, ctx, validity);
         } else {

hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/util/FileSignerSecretProvider.java

@@ -13,15 +13,15 @@
  */
 package org.apache.hadoop.security.authentication.util;
 
-import org.apache.hadoop.thirdparty.com.google.common.base.Charsets;
 import org.apache.hadoop.classification.InterfaceAudience;
 import org.apache.hadoop.classification.InterfaceStability;
 import org.apache.hadoop.security.authentication.server.AuthenticationFilter;
-import org.apache.hadoop.security.authentication.util.SignerSecretProvider;
 
 import javax.servlet.ServletContext;
 import java.io.*;
-import java.nio.charset.Charset;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.Paths;
 import java.util.Properties;
 
 /**
@@ -43,29 +43,24 @@ public void init(Properties config, ServletContext servletContext,
     String signatureSecretFile = config.getProperty(
         AuthenticationFilter.SIGNATURE_SECRET_FILE, null);
 
-    Reader reader = null;
     if (signatureSecretFile != null) {
-      try {
+      try (Reader reader = new InputStreamReader(Files.newInputStream(
+              Paths.get(signatureSecretFile)), StandardCharsets.UTF_8)) {
         StringBuilder sb = new StringBuilder();
-        reader = new InputStreamReader(
-            new FileInputStream(signatureSecretFile), Charsets.UTF_8);
         int c = reader.read();
         while (c > -1) {
           sb.append((char) c);
           c = reader.read();
         }
-        secret = sb.toString().getBytes(Charset.forName("UTF-8"));
+
+        secret = sb.toString().getBytes(StandardCharsets.UTF_8);
+        if (secret.length == 0) {
+          throw new RuntimeException("No secret in signature secret file: "
+             + signatureSecretFile);
+        }
       } catch (IOException ex) {
         throw new RuntimeException("Could not read signature secret file: " +
             signatureSecretFile);
-      } finally {
-        if (reader != null) {
-          try {
-            reader.close();
-          } catch (IOException e) {
-            // nothing to do
-          }
-        }
       }
     }
 

hadoop-common-project/hadoop-auth/src/test/java/org/apache/hadoop/security/authentication/server/TestAuthenticationFilter.java

@@ -305,6 +305,34 @@ public byte[][] getAllSecrets() {
       filter.destroy();
     }
   }
+
+  @Test
+  public void testEmptySecretFileFallbacksToRandomSecret() throws Exception {
+    AuthenticationFilter filter = new AuthenticationFilter();
+    try {
+      FilterConfig config = Mockito.mock(FilterConfig.class);
+      Mockito.when(config.getInitParameter(
+              AuthenticationFilter.AUTH_TYPE)).thenReturn("simple");
+      File secretFile = File.createTempFile("test_empty_secret", ".txt");
+      secretFile.deleteOnExit();
+      Assert.assertTrue(secretFile.exists());
+      Mockito.when(config.getInitParameter(
+              AuthenticationFilter.SIGNATURE_SECRET_FILE))
+              .thenReturn(secretFile.getAbsolutePath());
+      Mockito.when(config.getInitParameterNames()).thenReturn(
+              new Vector<>(Arrays.asList(AuthenticationFilter.AUTH_TYPE,
+                      AuthenticationFilter.SIGNATURE_SECRET_FILE)).elements());
+      ServletContext context = Mockito.mock(ServletContext.class);
+      Mockito.when(context.getAttribute(
+              AuthenticationFilter.SIGNER_SECRET_PROVIDER_ATTRIBUTE))
+              .thenReturn(null);
+      Mockito.when(config.getServletContext()).thenReturn(context);
+      filter.init(config);
+      Assert.assertTrue(filter.isRandomSecret());
+    } finally {
+      filter.destroy();
+    }
+  }
   
   @Test
   public void testInitCaseSensitivity() throws Exception {

hadoop-common-project/hadoop-auth/src/test/java/org/apache/hadoop/security/authentication/util/TestFileSignerSecretProvider.java

@@ -16,12 +16,16 @@
 import org.apache.hadoop.security.authentication.server.AuthenticationFilter;
 import org.junit.Assert;
 import org.junit.Test;
+import org.junit.function.ThrowingRunnable;
 
 import java.io.File;
 import java.io.FileWriter;
 import java.io.Writer;
 import java.util.Properties;
 
+import static org.junit.Assert.assertThrows;
+import static org.junit.Assert.assertTrue;
+
 public class TestFileSignerSecretProvider {
 
   @Test
@@ -48,4 +52,27 @@ public void testGetSecrets() throws Exception {
     Assert.assertEquals(1, allSecrets.length);
     Assert.assertArrayEquals(secretValue.getBytes(), allSecrets[0]);
   }
+
+  @Test
+  public void testEmptySecretFileThrows() throws Exception {
+    File secretFile = File.createTempFile("test_empty_secret", ".txt");
+    assertTrue(secretFile.exists());
+
+    FileSignerSecretProvider secretProvider
+            = new FileSignerSecretProvider();
+    Properties secretProviderProps = new Properties();
+    secretProviderProps.setProperty(
+            AuthenticationFilter.SIGNATURE_SECRET_FILE,
+            secretFile.getAbsolutePath());
+
+    Exception exception =
+        assertThrows(RuntimeException.class, new ThrowingRunnable() {
+          @Override
+          public void run() throws Throwable {
+            secretProvider.init(secretProviderProps, null, -1);
+          }
+        });
+    assertTrue(exception.getMessage().startsWith(
+        "No secret in signature secret file:"));
+  }
 }