From 2c6cfad5a31ca4d9126ecd2b3c43cca8543aacb4 Mon Sep 17 00:00:00 2001 From: Xiao Chen Date: Thu, 29 Mar 2018 15:36:31 -0700 Subject: [PATCH] HDFS-13087. Snapshotted encryption zone information should be immutable. Contributed by LiXin Ge. --- .../namenode/EncryptionZoneManager.java | 74 +++++++++++++++++-- .../namenode/FSDirEncryptionZoneOp.java | 15 ++-- .../hdfs/server/namenode/FSDirXAttrOp.java | 8 +- .../hdfs/server/namenode/XAttrStorage.java | 9 +-- .../hadoop/hdfs/TestEncryptionZones.java | 15 +++- 5 files changed, 97 insertions(+), 24 deletions(-) diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/EncryptionZoneManager.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/EncryptionZoneManager.java index 176ae1dc00..b1bca98406 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/EncryptionZoneManager.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/EncryptionZoneManager.java @@ -33,6 +33,9 @@ import com.google.common.base.Preconditions; import com.google.common.collect.Lists; import com.google.common.util.concurrent.ThreadFactoryBuilder; +import com.google.protobuf.InvalidProtocolBufferException; +import org.apache.commons.lang.builder.EqualsBuilder; +import org.apache.commons.lang.builder.HashCodeBuilder; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.crypto.CipherSuite; import org.apache.hadoop.crypto.CryptoProtocolVersion; @@ -50,6 +53,7 @@ import org.apache.hadoop.hdfs.protocol.proto.HdfsProtos; import org.apache.hadoop.hdfs.protocolPB.PBHelperClient; import org.apache.hadoop.hdfs.server.namenode.FSDirectory.DirOp; +import org.apache.hadoop.hdfs.server.namenode.snapshot.Snapshot; import org.apache.hadoop.security.AccessControlException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -107,6 +111,34 @@ CipherSuite getSuite() { String getKeyName() { return keyName; } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (!(o instanceof EncryptionZoneInt)) { + return false; + } + + EncryptionZoneInt b = (EncryptionZoneInt)o; + return new EqualsBuilder() + .append(inodeId, b.getINodeId()) + .append(suite, b.getSuite()) + .append(version, b.getVersion()) + .append(keyName, b.getKeyName()) + .isEquals(); + } + + @Override + public int hashCode() { + return new HashCodeBuilder(). + append(inodeId). + append(suite). + append(version). + append(keyName). + toHashCode(); + } } private TreeMap encryptionZones = null; @@ -315,8 +347,8 @@ void removeEncryptionZone(Long inodeId) { *

* Called while holding the FSDirectory lock. */ - boolean isInAnEZ(INodesInPath iip) - throws UnresolvedLinkException, SnapshotAccessControlException { + boolean isInAnEZ(INodesInPath iip) throws UnresolvedLinkException, + SnapshotAccessControlException, IOException { assert dir.hasReadLock(); return (getEncryptionZoneForPath(iip) != null); } @@ -341,7 +373,7 @@ String getFullPathName(Long nodeId) { *

* Called while holding the FSDirectory lock. */ - String getKeyName(final INodesInPath iip) { + String getKeyName(final INodesInPath iip) throws IOException { assert dir.hasReadLock(); EncryptionZoneInt ezi = getEncryptionZoneForPath(iip); if (ezi == null) { @@ -356,19 +388,43 @@ String getKeyName(final INodesInPath iip) { *

* Called while holding the FSDirectory lock. */ - private EncryptionZoneInt getEncryptionZoneForPath(INodesInPath iip) { + private EncryptionZoneInt getEncryptionZoneForPath(INodesInPath iip) + throws IOException{ assert dir.hasReadLock(); Preconditions.checkNotNull(iip); if (!hasCreatedEncryptionZone()) { return null; } + + int snapshotID = iip.getPathSnapshotId(); for (int i = iip.length() - 1; i >= 0; i--) { final INode inode = iip.getINode(i); - if (inode != null) { + if (inode == null || !inode.isDirectory()) { + //not found or not a directory, encryption zone is supported on + //directory only. + continue; + } + if (snapshotID == Snapshot.CURRENT_STATE_ID) { final EncryptionZoneInt ezi = encryptionZones.get(inode.getId()); if (ezi != null) { return ezi; } + } else { + XAttr xAttr = FSDirXAttrOp.unprotectedGetXAttrByPrefixedName( + inode, snapshotID, CRYPTO_XATTR_ENCRYPTION_ZONE); + if (xAttr != null) { + try { + final HdfsProtos.ZoneEncryptionInfoProto ezProto = + HdfsProtos.ZoneEncryptionInfoProto.parseFrom(xAttr.getValue()); + return new EncryptionZoneInt( + inode.getId(), PBHelperClient.convert(ezProto.getSuite()), + PBHelperClient.convert(ezProto.getCryptoProtocolVersion()), + ezProto.getKeyName()); + } catch (InvalidProtocolBufferException e) { + throw new IOException("Could not parse encryption zone for inode " + + iip.getPath(), e); + } + } } } return null; @@ -381,7 +437,8 @@ private EncryptionZoneInt getEncryptionZoneForPath(INodesInPath iip) { *

* Called while holding the FSDirectory lock. */ - private EncryptionZoneInt getParentEncryptionZoneForPath(INodesInPath iip) { + private EncryptionZoneInt getParentEncryptionZoneForPath(INodesInPath iip) + throws IOException { assert dir.hasReadLock(); Preconditions.checkNotNull(iip); INodesInPath parentIIP = iip.getParentINodesInPath(); @@ -395,7 +452,8 @@ private EncryptionZoneInt getParentEncryptionZoneForPath(INodesInPath iip) { * @param iip The INodesInPath of the path to check * @return the EncryptionZone representing the ez for the path. */ - EncryptionZone getEZINodeForPath(INodesInPath iip) { + EncryptionZone getEZINodeForPath(INodesInPath iip) + throws IOException { final EncryptionZoneInt ezi = getEncryptionZoneForPath(iip); if (ezi == null) { return null; @@ -437,7 +495,7 @@ void checkMoveValidity(INodesInPath srcIIP, INodesInPath dstIIP) } if (srcInEZ) { - if (srcParentEZI != dstParentEZI) { + if (!srcParentEZI.equals(dstParentEZI)) { final String srcEZPath = getFullPathName(srcParentEZI.getINodeId()); final String dstEZPath = getFullPathName(dstParentEZI.getINodeId()); final StringBuilder sb = new StringBuilder(srcIIP.getPath()); diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirEncryptionZoneOp.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirEncryptionZoneOp.java index bf5652d6fe..3d78172f92 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirEncryptionZoneOp.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirEncryptionZoneOp.java @@ -205,7 +205,7 @@ static Map.Entry getEZForPath( } static EncryptionZone getEZForPath(final FSDirectory fsd, - final INodesInPath iip) { + final INodesInPath iip) throws IOException { fsd.readLock(); try { return fsd.ezManager.getEZINodeForPath(iip); @@ -360,8 +360,9 @@ static XAttr generateNewXAttrForReencryptionFinish(final INodesInPath iip, private static ZoneEncryptionInfoProto getZoneEncryptionInfoProto( final INodesInPath iip) throws IOException { - final XAttr fileXAttr = FSDirXAttrOp - .unprotectedGetXAttrByPrefixedName(iip, CRYPTO_XATTR_ENCRYPTION_ZONE); + final XAttr fileXAttr = FSDirXAttrOp.unprotectedGetXAttrByPrefixedName( + iip.getLastINode(), iip.getPathSnapshotId(), + CRYPTO_XATTR_ENCRYPTION_ZONE); if (fileXAttr == null) { throw new IOException( "Could not find reencryption XAttr for file " + iip.getPath()); @@ -457,7 +458,8 @@ static FileEncryptionInfo getFileEncryptionInfo(final FSDirectory fsd, } XAttr fileXAttr = FSDirXAttrOp.unprotectedGetXAttrByPrefixedName( - iip, CRYPTO_XATTR_FILE_ENCRYPTION_INFO); + iip.getLastINode(), iip.getPathSnapshotId(), + CRYPTO_XATTR_FILE_ENCRYPTION_INFO); if (fileXAttr == null) { NameNode.LOG.warn("Could not find encryption XAttr for file " + iip.getPath() + " in encryption zone " + encryptionZone.getPath()); @@ -494,7 +496,7 @@ static FileEncryptionInfo getFileEncryptionInfo(final FSDirectory fsd, */ static FileEncryptionInfo getFileEncryptionInfo(FSDirectory dir, INodesInPath iip, EncryptionKeyInfo ezInfo) - throws RetryStartFileException { + throws RetryStartFileException, IOException { FileEncryptionInfo feInfo = null; final EncryptionZone zone = getEZForPath(dir, iip); if (zone != null) { @@ -517,7 +519,8 @@ static FileEncryptionInfo getFileEncryptionInfo(FSDirectory dir, } static boolean isInAnEZ(final FSDirectory fsd, final INodesInPath iip) - throws UnresolvedLinkException, SnapshotAccessControlException { + throws UnresolvedLinkException, SnapshotAccessControlException, + IOException { if (!fsd.ezManager.hasCreatedEncryptionZone()) { return false; } diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirXAttrOp.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirXAttrOp.java index 24a475fcc0..9e95f90d86 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirXAttrOp.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirXAttrOp.java @@ -378,16 +378,18 @@ static XAttr getXAttrByPrefixedName(FSDirectory fsd, INodesInPath iip, String prefixedName) throws IOException { fsd.readLock(); try { - return XAttrStorage.readINodeXAttrByPrefixedName(iip, prefixedName); + return XAttrStorage.readINodeXAttrByPrefixedName(iip.getLastINode(), + iip.getPathSnapshotId(), prefixedName); } finally { fsd.readUnlock(); } } static XAttr unprotectedGetXAttrByPrefixedName( - INodesInPath iip, String prefixedName) + INode inode, int snapshotId, String prefixedName) throws IOException { - return XAttrStorage.readINodeXAttrByPrefixedName(iip, prefixedName); + return XAttrStorage.readINodeXAttrByPrefixedName( + inode, snapshotId, prefixedName); } private static void checkXAttrChangeAccess( diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/XAttrStorage.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/XAttrStorage.java index 8a91e2a723..3b3747b1d8 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/XAttrStorage.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/XAttrStorage.java @@ -47,14 +47,13 @@ public static String getName(int n) { *

* * @param inode INode to read - * @param snapshotId + * @param snapshotId the snapshotId of the requested path * @param prefixedName xAttr name with prefix * @return the xAttr */ - public static XAttr readINodeXAttrByPrefixedName(INodesInPath iip, - String prefixedName) { - XAttrFeature f = - iip.getLastINode().getXAttrFeature(iip.getPathSnapshotId()); + public static XAttr readINodeXAttrByPrefixedName(INode inode, int snapshotId, + String prefixedName) { + XAttrFeature f = inode.getXAttrFeature(snapshotId); return f == null ? null : f.getXAttr(prefixedName); } diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestEncryptionZones.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestEncryptionZones.java index c5414342ea..6f9ef290a7 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestEncryptionZones.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestEncryptionZones.java @@ -1413,11 +1413,20 @@ public void testSnapshotsOnEncryptionZones() throws Exception { fsWrapper.mkdir(zone, FsPermission.getDirDefault(), true); final Path snap2 = fs.createSnapshot(zoneParent, "snap2"); final Path snap2Zone = new Path(snap2, zone.getName()); + assertEquals("Got unexpected ez path", zone.toString(), + dfsAdmin.getEncryptionZoneForPath(snap1Zone).getPath().toString()); assertNull("Expected null ez path", dfsAdmin.getEncryptionZoneForPath(snap2Zone)); - // Create the encryption zone again + // Create the encryption zone again, and that shouldn't affect old snapshot dfsAdmin.createEncryptionZone(zone, TEST_KEY2, NO_TRASH); + EncryptionZone ezSnap1 = dfsAdmin.getEncryptionZoneForPath(snap1Zone); + assertEquals("Got unexpected ez path", zone.toString(), + ezSnap1.getPath().toString()); + assertEquals("Unexpected ez key", TEST_KEY, ezSnap1.getKeyName()); + assertNull("Expected null ez path", + dfsAdmin.getEncryptionZoneForPath(snap2Zone)); + final Path snap3 = fs.createSnapshot(zoneParent, "snap3"); final Path snap3Zone = new Path(snap3, zone.getName()); // Check that snap3's EZ has the correct settings @@ -1426,10 +1435,12 @@ public void testSnapshotsOnEncryptionZones() throws Exception { ezSnap3.getPath().toString()); assertEquals("Unexpected ez key", TEST_KEY2, ezSnap3.getKeyName()); // Check that older snapshots still have the old EZ settings - EncryptionZone ezSnap1 = dfsAdmin.getEncryptionZoneForPath(snap1Zone); + ezSnap1 = dfsAdmin.getEncryptionZoneForPath(snap1Zone); assertEquals("Got unexpected ez path", zone.toString(), ezSnap1.getPath().toString()); assertEquals("Unexpected ez key", TEST_KEY, ezSnap1.getKeyName()); + assertNull("Expected null ez path", + dfsAdmin.getEncryptionZoneForPath(snap2Zone)); // Check that listEZs only shows the current filesystem state ArrayList listZones = Lists.newArrayList();