From b3275ab1f2f4546ba4bdc0e48cfa60b5b05071b9 Mon Sep 17 00:00:00 2001
From: Konstantin V Shvachko <shv@apache.org>
Date: Mon, 30 Sep 2019 16:48:10 -0700
Subject: [PATCH] HDFS-14305. Fix serial number calculation in
 BlockTokenSecretManager to avoid token key ID overlap between NameNodes.
 Contributed by Konstantin V Shvachko.

---
 .../token/block/BlockTokenSecretManager.java  | 12 ++++++----
 .../security/token/block/TestBlockToken.java  | 23 +++++++++++++++++++
 .../TestFailoverWithBlockTokensEnabled.java   |  5 ++--
 3 files changed, 33 insertions(+), 7 deletions(-)

diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/security/token/block/BlockTokenSecretManager.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/security/token/block/BlockTokenSecretManager.java
index 335bb9fc9cb..a56074a0345 100644
--- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/security/token/block/BlockTokenSecretManager.java
+++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/security/token/block/BlockTokenSecretManager.java
@@ -122,8 +122,6 @@ public class BlockTokenSecretManager extends
         encryptionAlgorithm, nnIndex, numNNs, useProto, shouldWrapQOP);
     Preconditions.checkArgument(nnIndex >= 0);
     Preconditions.checkArgument(numNNs > 0);
-    setSerialNo(new SecureRandom().nextInt());
-    generateKeys();
   }
 
   /**
@@ -152,13 +150,19 @@ public class BlockTokenSecretManager extends
     this.useProto = useProto;
     this.shouldWrapQOP = shouldWrapQOP;
     this.timer = new Timer();
+    setSerialNo(new SecureRandom().nextInt(Integer.MAX_VALUE));
+    LOG.info("Block token key range: [{}, {})",
+        nnRangeStart, nnRangeStart + intRange);
     generateKeys();
   }
 
   @VisibleForTesting
-  public synchronized void setSerialNo(int serialNo) {
+  public synchronized void setSerialNo(int nextNo) {
     // we mod the serial number by the range and then add that times the index
-    this.serialNo = (serialNo % intRange) + (nnRangeStart);
+    this.serialNo = (nextNo % intRange) + (nnRangeStart);
+    assert serialNo >= nnRangeStart && serialNo < (nnRangeStart + intRange) :
+      "serialNo " + serialNo + " is not in the designated range: [" +
+      nnRangeStart + ", " + (nnRangeStart + intRange) + ")";
   }
 
   public void setBlockPoolId(String blockPoolId) {
diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/security/token/block/TestBlockToken.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/security/token/block/TestBlockToken.java
index 20e0d4629f1..d993b66c832 100644
--- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/security/token/block/TestBlockToken.java
+++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/security/token/block/TestBlockToken.java
@@ -819,4 +819,27 @@ public class TestBlockToken {
     testBadStorageIDCheckAccess(true);
   }
 
+  /**
+   * Verify that block token serialNo is always within the range designated to
+   * to the NameNode.
+   */
+  @Test
+  public void testBlockTokenRanges() throws IOException {
+    final int interval = 1024;
+    final int numNNs = Integer.MAX_VALUE / interval;
+    for(int nnIdx = 0; nnIdx < 64; nnIdx++) {
+      BlockTokenSecretManager sm = new BlockTokenSecretManager(
+          blockKeyUpdateInterval, blockTokenLifetime, nnIdx, numNNs,
+          "fake-pool", null, false);
+      int rangeStart = nnIdx * interval;
+      for(int i = 0; i < interval * 3; i++) {
+        int serialNo = sm.getSerialNoForTesting();
+        assertTrue(
+            "serialNo " + serialNo + " is not in the designated range: [" +
+                rangeStart + ", " + (rangeStart + interval) + ")",
+                serialNo >= rangeStart && serialNo < (rangeStart + interval));
+        sm.updateKeys();
+      }
+    }
+  }
 }
diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/ha/TestFailoverWithBlockTokensEnabled.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/ha/TestFailoverWithBlockTokensEnabled.java
index 43ab69d82d1..ff901219abb 100644
--- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/ha/TestFailoverWithBlockTokensEnabled.java
+++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/ha/TestFailoverWithBlockTokensEnabled.java
@@ -92,11 +92,10 @@ public class TestFailoverWithBlockTokensEnabled {
 
     setAndCheckSerialNumber(0, btsm1, btsm2, btsm3);
     setAndCheckSerialNumber(Integer.MAX_VALUE, btsm1, btsm2, btsm3);
-    setAndCheckSerialNumber(Integer.MIN_VALUE, btsm1, btsm2, btsm3);
     setAndCheckSerialNumber(Integer.MAX_VALUE / 2, btsm1, btsm2, btsm3);
-    setAndCheckSerialNumber(Integer.MIN_VALUE / 2, btsm1, btsm2, btsm3);
     setAndCheckSerialNumber(Integer.MAX_VALUE / 3, btsm1, btsm2, btsm3);
-    setAndCheckSerialNumber(Integer.MIN_VALUE / 3, btsm1, btsm2, btsm3);
+    setAndCheckSerialNumber(Integer.MAX_VALUE / 171717,
+        btsm1, btsm2, btsm3);
   }
 
   private void setAndCheckSerialNumber(int serialNumber, BlockTokenSecretManager... btsms) {