From b408f76d010b613e58b4f68d33c4e6d3149dc78f Mon Sep 17 00:00:00 2001 From: Clay Baenziger Date: Sun, 9 Oct 2022 16:40:16 -0600 Subject: [PATCH 1/2] HADOOP-18235. vulnerability: we may leak sensitive information in LocalKeyStoreProvider --- .../security/alias/LocalKeyStoreProvider.java | 34 +++++++++++-------- 1 file changed, 20 insertions(+), 14 deletions(-) diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/alias/LocalKeyStoreProvider.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/alias/LocalKeyStoreProvider.java index b355bbc9cd62f..d6f4ba118faef 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/alias/LocalKeyStoreProvider.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/alias/LocalKeyStoreProvider.java @@ -142,21 +142,27 @@ protected void initFileSystem(URI uri) @Override public void flush() throws IOException { - super.flush(); - if (LOG.isDebugEnabled()) { - LOG.debug("Resetting permissions to '" + permissions + "'"); - } - if (!Shell.WINDOWS) { - Files.setPosixFilePermissions(Paths.get(file.getCanonicalPath()), - permissions); - } else { - // FsPermission expects a 10-character string because of the leading - // directory indicator, i.e. "drwx------". The JDK toString method returns - // a 9-character string, so prepend a leading character. - FsPermission fsPermission = FsPermission.valueOf( - "-" + PosixFilePermissions.toString(permissions)); - FileUtil.setPermission(file, fsPermission); + try { + super.getWriteLock().lock(); + file.createNewFile(); + if (LOG.isDebugEnabled()) { + LOG.debug("Resetting permissions to '" + permissions + "'"); + } + if (!Shell.WINDOWS) { + Files.setPosixFilePermissions(Paths.get(file.getCanonicalPath()), + permissions); + } else { + // FsPermission expects a 10-character string because of the leading + // directory indicator, i.e. "drwx------". The JDK toString method returns + // a 9-character string, so prepend a leading character. + FsPermission fsPermission = FsPermission.valueOf( + "-" + PosixFilePermissions.toString(permissions)); + FileUtil.setPermission(file, fsPermission); + } + } finally { + super.getWriteLock().unlock(); } + super.flush(); } private static Set modeToPosixFilePermission( From 5d045909b32ff03a576e18822b4235a5c6dc07bf Mon Sep 17 00:00:00 2001 From: Clay Baenziger Date: Tue, 11 Oct 2022 10:20:03 -0600 Subject: [PATCH 2/2] Correct lock and flush scoping --- .../apache/hadoop/security/alias/LocalKeyStoreProvider.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/alias/LocalKeyStoreProvider.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/alias/LocalKeyStoreProvider.java index d6f4ba118faef..d44eadc91059d 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/alias/LocalKeyStoreProvider.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/alias/LocalKeyStoreProvider.java @@ -142,8 +142,8 @@ protected void initFileSystem(URI uri) @Override public void flush() throws IOException { + super.getWriteLock().lock(); try { - super.getWriteLock().lock(); file.createNewFile(); if (LOG.isDebugEnabled()) { LOG.debug("Resetting permissions to '" + permissions + "'"); @@ -159,10 +159,10 @@ public void flush() throws IOException { "-" + PosixFilePermissions.toString(permissions)); FileUtil.setPermission(file, fsPermission); } + super.flush(); } finally { super.getWriteLock().unlock(); } - super.flush(); } private static Set modeToPosixFilePermission(