HDFS-14305. Fix serial number calculation in BlockTokenSecretManager …

…to avoid token key ID overlap between NameNodes. Contributed by Konstantin V Shvachko.
apache · Sep 30, 2019 · b3275ab · b3275ab
1 parent e5bba59
commit b3275ab
Show file tree

Hide file tree

Showing 3 changed files with 33 additions and 7 deletions.
diff --git a/...fs/src/main/java/org/apache/hadoop/hdfs/security/token/block/BlockTokenSecretManager.java b/...fs/src/main/java/org/apache/hadoop/hdfs/security/token/block/BlockTokenSecretManager.java
@@ -122,8 +122,6 @@ public BlockTokenSecretManager(long keyUpdateInterval,
         encryptionAlgorithm, nnIndex, numNNs, useProto, shouldWrapQOP);
     Preconditions.checkArgument(nnIndex >= 0);
     Preconditions.checkArgument(numNNs > 0);
-    setSerialNo(new SecureRandom().nextInt());
-    generateKeys();
   }
 
   /**
@@ -152,13 +150,19 @@ private BlockTokenSecretManager(boolean isMaster, long keyUpdateInterval,
     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/src/test/java/org/apache/hadoop/hdfs/security/token/block/TestBlockToken.java b/...hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/security/token/block/TestBlockToken.java
@@ -819,4 +819,27 @@ public void testBadStorageIDCheckAccess() throws IOException {
     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/...st/java/org/apache/hadoop/hdfs/server/namenode/ha/TestFailoverWithBlockTokensEnabled.java b/...st/java/org/apache/hadoop/hdfs/server/namenode/ha/TestFailoverWithBlockTokensEnabled.java
@@ -92,11 +92,10 @@ public void ensureSerialNumbersNeverOverlap() {
 
     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) {