From 6566402a1b22d2fa8311db7c7583f7200a3de88d Mon Sep 17 00:00:00 2001
From: Chen Liang <cliang@apache.org>
Date: Thu, 10 Oct 2019 13:29:30 -0700
Subject: [PATCH] HDFS-14509. DN throws InvalidToken due to inequality of
 password when upgrade NN 2.x to 3.x. Contributed by Yuxuan Wang and
 Konstantin Shvachko.

---
 .../token/block/BlockTokenIdentifier.java     | 13 +++++
 .../security/token/block/TestBlockToken.java  | 50 +++++++++++++++++++
 2 files changed, 63 insertions(+)

diff --git a/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/security/token/block/BlockTokenIdentifier.java b/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/security/token/block/BlockTokenIdentifier.java
index 87c831af193..4d2037b6138 100644
--- a/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/security/token/block/BlockTokenIdentifier.java
+++ b/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/security/token/block/BlockTokenIdentifier.java
@@ -19,12 +19,14 @@
 package org.apache.hadoop.hdfs.security.token.block;
 
 import java.io.DataInput;
+import java.io.DataInputStream;
 import java.io.DataOutput;
 import java.io.EOFException;
 import java.io.IOException;
 import java.util.EnumSet;
 
 import org.apache.hadoop.classification.InterfaceAudience;
+import org.apache.hadoop.io.IOUtils;
 import org.apache.hadoop.io.Text;
 import org.apache.hadoop.io.WritableUtils;
 import org.apache.hadoop.security.UserGroupInformation;
@@ -116,6 +118,7 @@ public class BlockTokenIdentifier extends TokenIdentifier {
   }
 
   public void setHandshakeMsg(byte[] bytes) {
+    cache = null; // invalidate the cache
     handshakeMsg = bytes;
   }
 
@@ -159,6 +162,16 @@ public class BlockTokenIdentifier extends TokenIdentifier {
   @Override
   public void readFields(DataInput in) throws IOException {
     this.cache = null;
+    if (in instanceof DataInputStream) {
+      final DataInputStream dis = (DataInputStream) in;
+      // this.cache should be assigned the raw bytes from the input data for
+      // upgrading compatibility. If we won't mutate fields and call getBytes()
+      // for something (e.g retrieve password), we should return the raw bytes
+      // instead of serializing the instance self fields to bytes, because we
+      // may lose newly added fields which we can't recognize.
+      this.cache = IOUtils.readFullyToByteArray(dis);
+      dis.reset();
+    }
     expiryDate = WritableUtils.readVLong(in);
     keyId = WritableUtils.readVInt(in);
     userId = WritableUtils.readString(in);
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 7d0c90fc7c6..07aaf091291 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
@@ -19,6 +19,7 @@
 package org.apache.hadoop.hdfs.security.token.block;
 
 import static org.apache.hadoop.fs.CommonConfigurationKeysPublic.HADOOP_SECURITY_AUTHENTICATION;
+import static org.junit.Assert.assertArrayEquals;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
@@ -30,6 +31,7 @@ import java.io.ByteArrayInputStream;
 import java.io.DataInputStream;
 import java.io.File;
 import java.io.IOException;
+import java.io.DataOutput;
 import java.net.InetSocketAddress;
 import java.util.EnumSet;
 import java.util.Set;
@@ -76,6 +78,7 @@ import org.junit.Assert;
 import org.junit.Assume;
 import org.junit.Before;
 import org.junit.Test;
+import org.mockito.Mockito;
 import org.mockito.invocation.InvocationOnMock;
 import org.mockito.stubbing.Answer;
 
@@ -435,4 +438,51 @@ public class TestBlockToken {
       }
     }
   }
+
+  @Test
+  public void testRetrievePasswordWithUnknownFields() throws IOException {
+    BlockTokenIdentifier id = new BlockTokenIdentifier();
+    BlockTokenIdentifier spyId = Mockito.spy(id);
+    Mockito.doAnswer(new Answer<Void>() {
+      @Override
+      public Void answer(InvocationOnMock invocation) throws Throwable {
+        DataOutput out = (DataOutput) invocation.getArguments()[0];
+        invocation.callRealMethod();
+        // write something at the end that BlockTokenIdentifier#readFields()
+        // will ignore, but which is still a part of the password
+        out.write(7);
+        return null;
+      }
+    }).when(spyId).write((DataOutput) Mockito.any());
+
+    BlockTokenSecretManager sm =
+        new BlockTokenSecretManager(blockKeyUpdateInterval, blockTokenLifetime,
+            0, 1, "fake-pool", null, false);
+    // master create password
+    byte[] password = sm.createPassword(spyId);
+
+    BlockTokenIdentifier slaveId = new BlockTokenIdentifier();
+    slaveId.readFields(
+        new DataInputStream(new ByteArrayInputStream(spyId.getBytes())));
+
+    // slave retrieve password
+    assertArrayEquals(password, sm.retrievePassword(slaveId));
+  }
+
+  @Test
+  public void testRetrievePasswordWithRecognizableFieldsOnly()
+      throws IOException {
+    BlockTokenSecretManager sm =
+        new BlockTokenSecretManager(blockKeyUpdateInterval, blockTokenLifetime,
+            0, 1, "fake-pool", null, false);
+    // master create password
+    BlockTokenIdentifier masterId = new BlockTokenIdentifier();
+    byte[] password = sm.createPassword(masterId);
+    // set cache to null, so that master getBytes() were only recognizable bytes
+    masterId.setExpiryDate(masterId.getExpiryDate());
+    BlockTokenIdentifier slaveId = new BlockTokenIdentifier();
+    slaveId.readFields(
+        new DataInputStream(new ByteArrayInputStream(masterId.getBytes())));
+    assertArrayEquals(password, sm.retrievePassword(slaveId));
+  }
 }