From 995c7ce2fa5799bd2bbcc09c205dc823b2b405d5 Mon Sep 17 00:00:00 2001 From: Anders Breindahl Date: Tue, 6 Dec 2016 14:00:38 +0100 Subject: [PATCH] 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-3045. This closes #1302. Signed-off-by: Andy LoPresto --- .../org/apache/nifi/bootstrap/RunNiFi.java | 43 +++++++++-- .../src/main/java/org/apache/nifi/NiFi.java | 75 ++++++++++++++----- .../nifi/audit/RemoteProcessGroupAuditor.java | 46 ++++++------ 3 files changed, 114 insertions(+), 50 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 edca894878..60c546d3e0 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; @@ -33,7 +34,12 @@ import java.net.InetSocketAddress; 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; @@ -1027,19 +1033,42 @@ public class RunNiFi { 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)); + 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); + } + + 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); 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 902b088518..ced9fedd53 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 @@ -28,6 +28,7 @@ import org.slf4j.LoggerFactory; import org.slf4j.bridge.SLF4JBridgeHandler; import java.io.File; +import java.io.FileWriter; import java.io.IOException; import java.lang.Thread.UncaughtExceptionHandler; import java.lang.reflect.Constructor; @@ -36,12 +37,14 @@ import java.lang.reflect.Method; 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.Set; +import java.util.Random; import java.util.Timer; import java.util.TimerTask; import java.util.concurrent.Executors; @@ -55,7 +58,7 @@ import java.util.concurrent.atomic.AtomicLong; 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; @@ -307,39 +310,73 @@ public class NiFi { 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 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]); diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/audit/RemoteProcessGroupAuditor.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/audit/RemoteProcessGroupAuditor.java index 27a121a651..5a69cfe49e 100644 --- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/audit/RemoteProcessGroupAuditor.java +++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/audit/RemoteProcessGroupAuditor.java @@ -16,27 +16,7 @@ */ package org.apache.nifi.audit; -import org.apache.nifi.action.Action; -import org.apache.nifi.action.Component; -import org.apache.nifi.action.FlowChangeAction; -import org.apache.nifi.action.Operation; -import org.apache.nifi.action.component.details.FlowChangeRemoteProcessGroupDetails; -import org.apache.nifi.action.details.ActionDetails; -import org.apache.nifi.action.details.ConfigureDetails; -import org.apache.nifi.action.details.FlowChangeConfigureDetails; -import org.apache.nifi.authorization.user.NiFiUserUtils; -import org.apache.nifi.groups.RemoteProcessGroup; -import org.apache.nifi.remote.RemoteGroupPort; -import org.apache.nifi.authorization.user.NiFiUser; -import org.apache.nifi.util.StringUtils; -import org.apache.nifi.web.api.dto.RemoteProcessGroupDTO; -import org.apache.nifi.web.api.dto.RemoteProcessGroupPortDTO; -import org.apache.nifi.web.dao.RemoteProcessGroupDAO; -import org.aspectj.lang.ProceedingJoinPoint; -import org.aspectj.lang.annotation.Around; -import org.aspectj.lang.annotation.Aspect; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; +import static org.apache.nifi.web.api.dto.DtoFactory.SENSITIVE_VALUE_MASK; import java.util.ArrayList; import java.util.Arrays; @@ -48,8 +28,27 @@ import java.util.Map; import java.util.Objects; import java.util.function.BiFunction; import java.util.function.Function; - -import static org.apache.nifi.web.api.dto.DtoFactory.SENSITIVE_VALUE_MASK; +import org.apache.nifi.action.Action; +import org.apache.nifi.action.Component; +import org.apache.nifi.action.FlowChangeAction; +import org.apache.nifi.action.Operation; +import org.apache.nifi.action.component.details.FlowChangeRemoteProcessGroupDetails; +import org.apache.nifi.action.details.ActionDetails; +import org.apache.nifi.action.details.ConfigureDetails; +import org.apache.nifi.action.details.FlowChangeConfigureDetails; +import org.apache.nifi.authorization.user.NiFiUser; +import org.apache.nifi.authorization.user.NiFiUserUtils; +import org.apache.nifi.groups.RemoteProcessGroup; +import org.apache.nifi.remote.RemoteGroupPort; +import org.apache.nifi.util.StringUtils; +import org.apache.nifi.web.api.dto.RemoteProcessGroupDTO; +import org.apache.nifi.web.api.dto.RemoteProcessGroupPortDTO; +import org.apache.nifi.web.dao.RemoteProcessGroupDAO; +import org.aspectj.lang.ProceedingJoinPoint; +import org.aspectj.lang.annotation.Around; +import org.aspectj.lang.annotation.Aspect; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** * Audits remote process group creation/removal and configuration changes. @@ -355,7 +354,6 @@ public class RemoteProcessGroupAuditor extends NiFiAuditor { private RemoteGroupPort auditUpdateProcessGroupPortConfiguration(ProceedingJoinPoint proceedingJoinPoint, RemoteProcessGroupPortDTO remoteProcessGroupPortDto, RemoteProcessGroup remoteProcessGroup, RemoteGroupPort remoteProcessGroupPort) throws Throwable { - final Map previousValues = ConfigurationRecorder.capturePreviousValues(PORT_CONFIG_RECORDERS, remoteProcessGroupPort); // perform the underlying operation