From ad3294c294fadb39d15a2973dd4c117356aeeafe Mon Sep 17 00:00:00 2001 From: Anders Breindahl Date: Tue, 6 Dec 2016 14:00:38 +0100 Subject: [PATCH 1/4] NIFI-2656: replace -k [password] with -K [passwordfile]. This approaches a proper solution on how to hand over the key from RunNiFi to Nifi. Insofar the password file is pruned as part of the startup, NiFi processors can't read it. See also: NIFI-2656/NIFI-3045. --- .../org/apache/nifi/bootstrap/RunNiFi.java | 20 +++-- .../src/main/java/org/apache/nifi/NiFi.java | 75 ++++++++++++++----- 2 files changed, 69 insertions(+), 26 deletions(-) diff --git a/nifi-bootstrap/src/main/java/org/apache/nifi/bootstrap/RunNiFi.java b/nifi-bootstrap/src/main/java/org/apache/nifi/bootstrap/RunNiFi.java index 8d92c442925e..d94c54e3d834 100644 --- a/nifi-bootstrap/src/main/java/org/apache/nifi/bootstrap/RunNiFi.java +++ b/nifi-bootstrap/src/main/java/org/apache/nifi/bootstrap/RunNiFi.java @@ -21,6 +21,7 @@ import java.io.FileInputStream; import java.io.FileNotFoundException; import java.io.FileOutputStream; +import java.io.FileWriter; import java.io.FilenameFilter; import java.io.IOException; import java.io.InputStream; @@ -1027,19 +1028,24 @@ public boolean accept(final File dir, final String filename) { cmd.add("-Dorg.apache.nifi.bootstrap.config.log.dir=" + nifiLogDir); cmd.add("org.apache.nifi.NiFi"); if (props.containsKey(NIFI_BOOTSTRAP_SENSITIVE_KEY) && !StringUtils.isBlank(props.get(NIFI_BOOTSTRAP_SENSITIVE_KEY))) { - cmd.add("-k " + props.get(NIFI_BOOTSTRAP_SENSITIVE_KEY)); + File password_file = new File(confDir, "sensitive.key"); + + // Restrict permissions: + password_file.setReadable(false,false); + password_file.setReadable(true,true); + password_file.setWritable(false,false); + password_file.setWritable(true,true); + FileWriter sensitive_key_writer = new FileWriter(password_file,false); + sensitive_key_writer.write(props.get(NIFI_BOOTSTRAP_SENSITIVE_KEY)); + sensitive_key_writer.close(); + cmd.add("-K " + password_file.getAbsolutePath()); } builder.command(cmd); final StringBuilder cmdBuilder = new StringBuilder(); for (final String s : cmd) { - // Mask the key - if (s.startsWith("-k ")) { - cmdBuilder.append("-k ****"); - } else { - cmdBuilder.append(s).append(" "); - } + cmdBuilder.append(s).append(" "); } cmdLogger.info("Starting Apache NiFi..."); diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-runtime/src/main/java/org/apache/nifi/NiFi.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-runtime/src/main/java/org/apache/nifi/NiFi.java index bb2a2f612cef..451f18cfbd15 100644 --- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-runtime/src/main/java/org/apache/nifi/NiFi.java +++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-runtime/src/main/java/org/apache/nifi/NiFi.java @@ -17,6 +17,7 @@ package org.apache.nifi; import java.io.File; +import java.io.FileWriter; import java.io.IOException; import java.lang.Thread.UncaughtExceptionHandler; import java.lang.reflect.Constructor; @@ -25,11 +26,13 @@ import java.net.MalformedURLException; import java.net.URL; import java.net.URLClassLoader; +import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Paths; import java.util.ArrayList; import java.util.Arrays; import java.util.List; +import java.util.Random; import java.util.Timer; import java.util.TimerTask; import java.util.concurrent.Executors; @@ -53,7 +56,7 @@ public class NiFi { private static final Logger LOGGER = LoggerFactory.getLogger(NiFi.class); - private static final String KEY_FLAG = "-k"; + private static final String KEY_FILE_FLAG = "-K"; private final NiFiServer nifiServer; private final BootstrapListener bootstrapListener; @@ -305,39 +308,73 @@ private static String loadFormattedKey(String[] args) { String key = null; List parsedArgs = parseArgs(args); // Check if args contain protection key - if (parsedArgs.contains(KEY_FLAG)) { - key = getKeyFromArgs(parsedArgs); - + if (parsedArgs.contains(KEY_FILE_FLAG)) { + key = getKeyFromKeyFileAndPrune(parsedArgs); // Format the key (check hex validity and remove spaces) key = formatHexKey(key); - if (!isHexKeyValid(key)) { - throw new IllegalArgumentException("The key was not provided in valid hex format and of the correct length"); - } - return key; - } else { - // throw new IllegalStateException("No key provided from bootstrap"); + } + + if (null == key) { return ""; + } else if (!isHexKeyValid(key)) { + throw new IllegalArgumentException("The key \""+key+"\" was not provided in valid hex format and of the correct length"); + } else { + return key; } } - private static String getKeyFromArgs(List parsedArgs) { - String key; - LOGGER.debug("The bootstrap process provided the " + KEY_FLAG + " flag"); - int i = parsedArgs.indexOf(KEY_FLAG); + private static String getKeyFromKeyFileAndPrune(List parsedArgs) { + String key = null; + LOGGER.debug("The bootstrap process provided the " + KEY_FILE_FLAG + " flag"); + int i = parsedArgs.indexOf(KEY_FILE_FLAG); if (parsedArgs.size() <= i + 1) { - LOGGER.error("The bootstrap process passed the {} flag without a key", KEY_FLAG); - throw new IllegalArgumentException("The bootstrap process provided the " + KEY_FLAG + " flag but no key"); + LOGGER.error("The bootstrap process passed the {} flag without a filename", KEY_FILE_FLAG); + throw new IllegalArgumentException("The bootstrap process provided the " + KEY_FILE_FLAG + " flag but no key"); } - key = parsedArgs.get(i + 1); - LOGGER.info("Read property protection key from bootstrap process"); + try { + String passwordfile_path = parsedArgs.get(i + 1); + // Slurp in the contents of the file: + byte[] encoded = Files.readAllBytes(Paths.get(passwordfile_path)); + key = new String(encoded,StandardCharsets.UTF_8); + if (0 == key.length()) + throw new IllegalArgumentException("Key in keyfile " + passwordfile_path + " yielded an empty key"); + + LOGGER.info("Now overwriting file in "+passwordfile_path); + + // Overwrite the contents of the file (to avoid littering file system + // unlinked with key material): + File password_file = new File(passwordfile_path); + FileWriter overwriter = new FileWriter(password_file,false); + + // Construe a random pad: + Random r = new Random(); + StringBuffer sb = new StringBuffer(); + // Note on correctness: this pad is longer, but equally sufficient. + while(sb.length() < encoded.length){ + sb.append(Integer.toHexString(r.nextInt())); + } + String pad = sb.toString(); + LOGGER.info("Overwriting key material with pad: "+pad); + overwriter.write(pad); + overwriter.close(); + + LOGGER.info("Removing/unlinking file: "+passwordfile_path); + password_file.delete(); + + } catch (IOException e) { + LOGGER.error("Caught IOException while retrieving the "+KEY_FILE_FLAG+"-passed keyfile; aborting: "+e.toString()); + System.exit(1); + } + + LOGGER.info("Read property protection key from key file provided by bootstrap process"); return key; } private static List parseArgs(String[] args) { List parsedArgs = new ArrayList<>(Arrays.asList(args)); for (int i = 0; i < parsedArgs.size(); i++) { - if (parsedArgs.get(i).startsWith(KEY_FLAG + " ")) { + if (parsedArgs.get(i).startsWith(KEY_FILE_FLAG + " ")) { String[] split = parsedArgs.get(i).split(" ", 2); parsedArgs.set(i, split[0]); parsedArgs.add(i + 1, split[1]); From e700b885be875da980097914a32458d572eb1d05 Mon Sep 17 00:00:00 2001 From: Anders Breindahl Date: Wed, 8 Feb 2017 15:38:09 +0100 Subject: [PATCH 2/4] NIFI-2656: correct handling of file system permissions. With added Thread.sleep(1000)'s, the file goes through permissions thusly: ---------- 1 nifi nifi 0 Feb 8 06:35 /opt/nifi/conf/sensitive.key -rw------- 1 nifi nifi 0 Feb 8 06:35 /opt/nifi/conf/sensitive.key -rw------- 1 nifi nifi 64 Feb 8 06:35 /opt/nifi/conf/sensitive.key -rw------- 1 nifi nifi 64 Feb 8 06:35 /opt/nifi/conf/sensitive.key -rw------- 1 nifi nifi 64 Feb 8 06:35 /opt/nifi/conf/sensitive.key -rw------- 1 nifi nifi 64 Feb 8 06:35 /opt/nifi/conf/sensitive.key ls: cannot access /opt/nifi/conf/sensitive.key: No such file or directory The latter being when NiFi spawns and overwrites/unlinks the file. --- .../org/apache/nifi/bootstrap/RunNiFi.java | 47 ++++++++++++++----- 1 file changed, 36 insertions(+), 11 deletions(-) diff --git a/nifi-bootstrap/src/main/java/org/apache/nifi/bootstrap/RunNiFi.java b/nifi-bootstrap/src/main/java/org/apache/nifi/bootstrap/RunNiFi.java index d94c54e3d834..ff781321f12a 100644 --- a/nifi-bootstrap/src/main/java/org/apache/nifi/bootstrap/RunNiFi.java +++ b/nifi-bootstrap/src/main/java/org/apache/nifi/bootstrap/RunNiFi.java @@ -17,6 +17,7 @@ package org.apache.nifi.bootstrap; import java.io.BufferedReader; +import java.io.BufferedWriter; import java.io.File; import java.io.FileInputStream; import java.io.FileNotFoundException; @@ -35,7 +36,12 @@ import java.net.Socket; import java.nio.charset.StandardCharsets; import java.nio.file.Files; +import java.nio.file.Paths; +import java.nio.file.Path; +import java.nio.file.attribute.FileAttribute; import java.nio.file.attribute.PosixFilePermission; +import java.nio.file.attribute.PosixFilePermissions; +import java.nio.file.FileAlreadyExistsException; import java.text.SimpleDateFormat; import java.util.ArrayList; import java.util.Arrays; @@ -1028,17 +1034,36 @@ public boolean accept(final File dir, final String filename) { cmd.add("-Dorg.apache.nifi.bootstrap.config.log.dir=" + nifiLogDir); cmd.add("org.apache.nifi.NiFi"); if (props.containsKey(NIFI_BOOTSTRAP_SENSITIVE_KEY) && !StringUtils.isBlank(props.get(NIFI_BOOTSTRAP_SENSITIVE_KEY))) { - File password_file = new File(confDir, "sensitive.key"); - - // Restrict permissions: - password_file.setReadable(false,false); - password_file.setReadable(true,true); - password_file.setWritable(false,false); - password_file.setWritable(true,true); - FileWriter sensitive_key_writer = new FileWriter(password_file,false); - sensitive_key_writer.write(props.get(NIFI_BOOTSTRAP_SENSITIVE_KEY)); - sensitive_key_writer.close(); - cmd.add("-K " + password_file.getAbsolutePath()); + Path sensitiveKeyFile = Paths.get(confDir+"/sensitive.key"); + + + try { + // Initially create file with the empty permission set (so nobody can get a file descriptor on it): + Set perms = new HashSet(); + FileAttribute> attr = PosixFilePermissions.asFileAttribute(perms); + sensitiveKeyFile = Files.createFile(sensitiveKeyFile, attr); + + // Then, once created, add owner-only rights: + perms.add(PosixFilePermission.OWNER_WRITE); + perms.add(PosixFilePermission.OWNER_READ); + attr = PosixFilePermissions.asFileAttribute(perms); + Files.setPosixFilePermissions(sensitiveKeyFile, perms); + + } catch (final FileAlreadyExistsException faee) { + cmdLogger.error("The sensitive.key file {} already exists. That shouldn't have been. Aborting.", sensitiveKeyFile); + System.exit(1); + } catch (final Exception e) { + cmdLogger.error("Other failure relating to setting permissions on {}. " + + "(so that only the owner can read it). " + + "This is fatal to the bootstrap process for security reasons. Exception was: {}", sensitiveKeyFile, e); + System.exit(1); + } + + //FileWriter sensitiveKeyWriter = new FileWriter(sensitiveKeyFile,false); + BufferedWriter sensitiveKeyWriter = Files.newBufferedWriter(sensitiveKeyFile, StandardCharsets.UTF_8); + sensitiveKeyWriter.write(props.get(NIFI_BOOTSTRAP_SENSITIVE_KEY)); + sensitiveKeyWriter.close(); + cmd.add("-K " + sensitiveKeyFile.toFile().getAbsolutePath()); } builder.command(cmd); From 2c5d5fd675c1c4aef94461f80798ec15b880b447 Mon Sep 17 00:00:00 2001 From: Anders Breindahl Date: Wed, 8 Feb 2017 15:54:25 +0100 Subject: [PATCH 3/4] NIFI-2656: don't emit the key. Even if it's invalid. --- .../nifi-runtime/src/main/java/org/apache/nifi/NiFi.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-runtime/src/main/java/org/apache/nifi/NiFi.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-runtime/src/main/java/org/apache/nifi/NiFi.java index 54e06b9ae980..f8e5601cec39 100644 --- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-runtime/src/main/java/org/apache/nifi/NiFi.java +++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-runtime/src/main/java/org/apache/nifi/NiFi.java @@ -320,7 +320,7 @@ private static String loadFormattedKey(String[] args) { if (null == key) { return ""; } else if (!isHexKeyValid(key)) { - throw new IllegalArgumentException("The key \""+key+"\" was not provided in valid hex format and of the correct length"); + throw new IllegalArgumentException("The key was not provided in valid hex format and of the correct length"); } else { return key; } From 4bdcb7bcfe268e4e091ad370999b08248c72f3e7 Mon Sep 17 00:00:00 2001 From: Anders Breindahl Date: Thu, 9 Feb 2017 11:50:31 +0100 Subject: [PATCH 4/4] NIFI-2656: unused import; getting rid of FileWriter. --- .../src/main/java/org/apache/nifi/bootstrap/RunNiFi.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/nifi-bootstrap/src/main/java/org/apache/nifi/bootstrap/RunNiFi.java b/nifi-bootstrap/src/main/java/org/apache/nifi/bootstrap/RunNiFi.java index ff781321f12a..a999d45367c8 100644 --- a/nifi-bootstrap/src/main/java/org/apache/nifi/bootstrap/RunNiFi.java +++ b/nifi-bootstrap/src/main/java/org/apache/nifi/bootstrap/RunNiFi.java @@ -22,7 +22,6 @@ import java.io.FileInputStream; import java.io.FileNotFoundException; import java.io.FileOutputStream; -import java.io.FileWriter; import java.io.FilenameFilter; import java.io.IOException; import java.io.InputStream; @@ -1059,7 +1058,6 @@ public boolean accept(final File dir, final String filename) { System.exit(1); } - //FileWriter sensitiveKeyWriter = new FileWriter(sensitiveKeyFile,false); BufferedWriter sensitiveKeyWriter = Files.newBufferedWriter(sensitiveKeyFile, StandardCharsets.UTF_8); sensitiveKeyWriter.write(props.get(NIFI_BOOTSTRAP_SENSITIVE_KEY)); sensitiveKeyWriter.close();