Apache/nifi
1
0
Fork 0
mirror of https://github.com/apache/nifi.git synced 2025-02-11 20:45:11 +00:00
Code Issues Packages Projects Releases Wiki Activity

NIFI-7679 Changed flow fingerprint masking logic to derive a static key once (slowly) from nifi.sensitive.props.key and use a (fast) HMAC/SHA-256 operation during fingerprinting to resolve unacceptable delays.

Browse Source 
Added unit tests.

Signed-off-by: Pierre Villard <pierre.villard.fr@gmail.com>

This closes #4434.
This commit is contained in:
Andy LoPresto 2020-07-27 12:07:50 -07:00 committed by Pierre Villard
parent 0b9f2fbe3b
commit 90c9db8208
No known key found for this signature in database
GPG Key ID: F92A93B30C07C6D5
5 changed files with 192 additions and 22 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

58
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/fingerprint/FingerprintFactory.java
View File

@ -18,7 +18,11 @@ package org.apache.nifi.fingerprint;
import java.io.ByteArrayInputStream;
import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.security.InvalidKeyException;
import java.security.NoSuchAlgorithmException;
import java.util.ArrayList;
import java.util.Base64;
import java.util.Collections;
import java.util.Comparator;
import java.util.List;
@ -26,6 +30,8 @@ import java.util.Map;
import java.util.SortedMap;
import java.util.TreeMap;
import java.util.stream.Stream;
import javax.crypto.Mac;
import javax.crypto.spec.SecretKeySpec;
import javax.xml.XMLConstants;
import javax.xml.parsers.DocumentBuilder;
import javax.xml.parsers.DocumentBuilderFactory;
@ -40,10 +46,12 @@ import org.apache.nifi.controller.serialization.FlowEncodingVersion;
import org.apache.nifi.controller.serialization.FlowFromDOMFactory;
import org.apache.nifi.encrypt.StringEncryptor;
import org.apache.nifi.nar.ExtensionManager;
import org.apache.nifi.security.util.crypto.CipherUtility;
import org.apache.nifi.properties.NiFiPropertiesLoader;
import org.apache.nifi.security.util.crypto.Argon2SecureHasher;
import org.apache.nifi.util.BundleUtils;
import org.apache.nifi.util.DomUtils;
import org.apache.nifi.util.LoggingXmlParserErrorHandler;
import org.apache.nifi.util.NiFiProperties;
import org.apache.nifi.web.api.dto.BundleDTO;
import org.apache.nifi.web.api.dto.ControllerServiceDTO;
import org.apache.nifi.web.api.dto.ReportingTaskDTO;
@ -85,6 +93,8 @@ public class FingerprintFactory {
private final DocumentBuilder flowConfigDocBuilder;
private final ExtensionManager extensionManager;
private byte[] sensitivePropertyKeyBytes;
private static final Logger logger = LoggerFactory.getLogger(FingerprintFactory.class);
public FingerprintFactory(final StringEncryptor encryptor, final ExtensionManager extensionManager) {
@ -122,9 +132,7 @@ public class FingerprintFactory {
* that is not present in Flow A, then the two will have different fingerprints.
*
* @param flowBytes the flow represented as bytes
*
* @return a generated fingerprint
*
* @throws FingerprintException if the fingerprint failed to be generated
*/
public synchronized String createFingerprint(final byte[] flowBytes) throws FingerprintException {
@ -134,11 +142,9 @@ public class FingerprintFactory {
/**
* Creates a fingerprint of a flow. The order of elements or attributes in the flow does not influence the fingerprint generation.
*
* @param flowBytes the flow represented as bytes
* @param flowBytes the flow represented as bytes
* @param controller the controller
*
* @return a generated fingerprint
*
* @throws FingerprintException if the fingerprint failed to be generated
*/
public synchronized String createFingerprint(final byte[] flowBytes, final FlowController controller) throws FingerprintException {
@ -149,7 +155,6 @@ public class FingerprintFactory {
* Creates a fingerprint from an XML document representing the flow.xml.
*
* @param flowDoc the DOM
*
* @return the fingerprint
*/
public synchronized String createFingerprint(final Document flowDoc, final FlowController controller) {
@ -178,9 +183,7 @@ public class FingerprintFactory {
* Parse the given flow.xml bytes into a Document instance.
*
* @param flow a flow
*
* @return the DOM
*
* @throws FingerprintException if the flow could not be parsed
*/
private Document parseFlow(final byte[] flow) throws FingerprintException {
@ -297,7 +300,7 @@ public class FingerprintFactory {
private void orderByChildElement(final List<Element> toSort, final String childTagName) {
toSort.sort((a, b) -> {
final String valueA = DomUtils.getChildText(a, childTagName);
final String valueB = DomUtils.getChildText(b, childTagName );
final String valueB = DomUtils.getChildText(b, childTagName);
return valueA.compareTo(valueB);
});
}
@ -540,7 +543,7 @@ public class FingerprintFactory {
* trouble inheriting a flow, so the sensitive value should not be disclosed through the
* log. However, the equality or difference of the sensitive value can influence in the
* inheritability of the flow, so it cannot be ignored completely.
*
* <p>
* The specific derivation process is unimportant as long as it is a salted,
* cryptographically-secure hash function with an iteration cost sufficient for password
* storage in other applications.
@ -549,7 +552,34 @@ public class FingerprintFactory {
* @return a deterministic string value which represents this input but is safe to print in a log
*/
private String getLoggableRepresentationOfSensitiveValue(String encryptedPropertyValue) {
return CipherUtility.getLoggableRepresentationOfSensitiveValue(decrypt(encryptedPropertyValue));
initializeSensitivePropertyKeyBytes();
final String plaintextValue = decrypt(encryptedPropertyValue);
try {
Mac mac = Mac.getInstance("HmacSHA256");
mac.init(new SecretKeySpec(sensitivePropertyKeyBytes, "HmacSHA256"));
byte[] hashedBytes = mac.doFinal(plaintextValue.getBytes(StandardCharsets.UTF_8));
return "[MASKED] (" + Base64.getEncoder().encodeToString(hashedBytes) + ")";
} catch (NoSuchAlgorithmException | InvalidKeyException e) {
// This should not occur on any system which runs NiFi as HmacSHA256 is provided by BouncyCastle and SunJCE
logger.error("Encountered an error making the sensitive value loggable: {}", e.getLocalizedMessage());
return "[Unable to mask value]";
}
}
private void initializeSensitivePropertyKeyBytes() {
if (sensitivePropertyKeyBytes == null || sensitivePropertyKeyBytes.length == 0) {
Argon2SecureHasher a2sh = new Argon2SecureHasher();
// Derive the reusable HMAC key from the nifi.sensitive.props.key to ensure deterministic output across nodes
try {
String npsk = NiFiPropertiesLoader.loadDefaultWithKeyFromBootstrap().getProperty(NiFiProperties.SENSITIVE_PROPS_KEY);
// The output will be 32B (256b)
sensitivePropertyKeyBytes = a2sh.hashRaw(npsk.getBytes(StandardCharsets.UTF_8));
} catch (IOException e) {
logger.error("Encountered an error loading NiFi properties while fingerprinting flow: ", e);
}
}
}
private StringBuilder addPortFingerprint(final StringBuilder builder, final Element portElem) throws FingerprintException {
@ -600,7 +630,7 @@ public class FingerprintFactory {
private StringBuilder addRemoteProcessGroupFingerprint(final StringBuilder builder, final Element remoteProcessGroupElem) throws FingerprintException {
for (String tagName : new String[] {"id", "versionedComponentId", "urls", "networkInterface", "timeout", "yieldPeriod",
for (String tagName : new String[]{"id", "versionedComponentId", "urls", "networkInterface", "timeout", "yieldPeriod",
"transportProtocol", "proxyHost", "proxyPort", "proxyUser", "proxyPassword"}) {
final String value = getFirstValue(DomUtils.getChildNodesByTagName(remoteProcessGroupElem, tagName));
if (isEncrypted(value)) {
@ -667,7 +697,7 @@ public class FingerprintFactory {
}
private StringBuilder addRemoteGroupPortFingerprint(final StringBuilder builder, final Element remoteGroupPortElement) {
for (final String childName : new String[] {"id", "targetId", "versionedComponentId", "maxConcurrentTasks", "useCompression", "batchCount", "batchSize", "batchDuration"}) {
for (final String childName : new String[]{"id", "targetId", "versionedComponentId", "maxConcurrentTasks", "useCompression", "batchCount", "batchSize", "batchDuration"}) {
appendFirstValue(builder, DomUtils.getChildNodesByTagName(remoteGroupPortElement, childName));
}

70
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/test/groovy/org/apache/nifi/fingerprint/FingerprintFactoryGroovyTest.groovy
View File

@ -16,12 +16,13 @@
*/
package org.apache.nifi.fingerprint
import org.apache.nifi.encrypt.StringEncryptor
import org.apache.nifi.nar.ExtensionManager
import org.apache.nifi.nar.StandardExtensionDiscoveringManager
import org.apache.nifi.util.NiFiProperties
import org.bouncycastle.jce.provider.BouncyCastleProvider
import org.junit.After
import org.junit.AfterClass
import org.junit.Before
import org.junit.BeforeClass
import org.junit.Test
@ -41,6 +42,9 @@ class FingerprintFactoryGroovyTest extends GroovyTestCase {
decrypt: { String cipherText -> cipherText.reverse() }] as StringEncryptor
private static ExtensionManager extensionManager = new StandardExtensionDiscoveringManager()
private static String originalPropertiesPath = System.getProperty(NiFiProperties.PROPERTIES_FILE_PATH)
private static final String NIFI_PROPERTIES_PATH = "src/test/resources/conf/nifi.properties"
@BeforeClass
static void setUpOnce() throws Exception {
Security.addProvider(new BouncyCastleProvider())
@ -60,6 +64,13 @@ class FingerprintFactoryGroovyTest extends GroovyTestCase {
}
@AfterClass
static void tearDownOnce() {
if (originalPropertiesPath) {
System.setProperty(NiFiProperties.PROPERTIES_FILE_PATH, originalPropertiesPath)
}
}
/**
* The flow fingerprint should not disclose sensitive property values.
*/
@ -88,4 +99,61 @@ class FingerprintFactoryGroovyTest extends GroovyTestCase {
assert maskedValue
logger.info("Masked value: ${maskedValue[0]}")
}
/**
* The initial implementation derived the hashed value using a time/memory-hard algorithm (Argon2) every time.
* For large flow definitions, this blocked startup for minutes. Deriving a secure key with the Argon2
* algorithm once at startup (~1 second) and using this cached key for a simple HMAC/SHA-256 operation on every
* fingerprint should be much faster.
*/
@Test
void testCreateFingerprintShouldNotBeSlow() {
// Arrange
int testIterations = 100 //_000
// Set up test nifi.properties
System.setProperty(NiFiProperties.PROPERTIES_FILE_PATH, NIFI_PROPERTIES_PATH)
// Create flow
String initialFlowXML = new File("src/test/resources/nifi/fingerprint/initial.xml").text
logger.info("Read initial flow: ${initialFlowXML[0..<100]}...")
// Create the FingerprintFactory with collaborators
FingerprintFactory fingerprintFactory = new FingerprintFactory(mockEncryptor, extensionManager)
def results = []
def resultDurations = []
// Act
testIterations.times { int i ->
long startNanos = System.nanoTime()
// Create the fingerprint from the flow
String fingerprint = fingerprintFactory.createFingerprint(initialFlowXML.bytes)
long endNanos = System.nanoTime()
long durationNanos = endNanos - startNanos
logger.info("Generated flow fingerprint: ${fingerprint} in ${durationNanos} ns")
results << fingerprint
resultDurations << durationNanos
}
def milliDurations = [resultDurations.min(), resultDurations.max(), resultDurations.sum() / resultDurations.size()].collect { it / 1_000_000 }
logger.info("Min/Max/Avg durations in ms: ${milliDurations}")
// Assert
final long MAX_DURATION_NANOS = 1_000_000_000 // 1 second
assert resultDurations.max() <= MAX_DURATION_NANOS * 2
assert resultDurations.sum() / testIterations < MAX_DURATION_NANOS
// Assert the fingerprint does not contain the password
results.each { String fingerprint ->
assert !(fingerprint =~ "originalPlaintextPassword")
def maskedValue = (fingerprint =~ /\[MASKED\] \([\w\/\+=]+\)/)
assert maskedValue
logger.info("Masked value: ${maskedValue[0]}")
}
}
}

33
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/test/java/org/apache/nifi/fingerprint/FingerprintFactoryTest.java
View File

@ -28,8 +28,12 @@ import static org.mockito.Mockito.when;
import java.io.File;
import java.io.IOException;
import java.lang.reflect.Method;
import java.nio.charset.StandardCharsets;
import java.util.Base64;
import java.util.Collections;
import java.util.Optional;
import javax.crypto.Mac;
import javax.crypto.spec.SecretKeySpec;
import javax.xml.XMLConstants;
import javax.xml.parsers.DocumentBuilder;
import javax.xml.parsers.DocumentBuilderFactory;
@ -48,9 +52,12 @@ import org.apache.nifi.encrypt.StringEncryptor;
import org.apache.nifi.groups.RemoteProcessGroup;
import org.apache.nifi.nar.ExtensionManager;
import org.apache.nifi.nar.StandardExtensionDiscoveringManager;
import org.apache.nifi.properties.NiFiPropertiesLoader;
import org.apache.nifi.remote.RemoteGroupPort;
import org.apache.nifi.remote.protocol.SiteToSiteTransportProtocol;
import org.apache.nifi.security.util.crypto.Argon2SecureHasher;
import org.apache.nifi.util.NiFiProperties;
import org.junit.AfterClass;
import org.junit.Before;
import org.junit.Test;
import org.w3c.dom.Document;
@ -67,6 +74,9 @@ public class FingerprintFactoryTest {
private ExtensionManager extensionManager;
private FingerprintFactory fingerprinter;
private static final String ORIGINAL_NIFI_PROPS_PATH = System.getProperty(NiFiProperties.PROPERTIES_FILE_PATH);
private static final String TEST_NIFI_PROPS_PATH = "src/test/resources/conf/nifi.properties";
@Before
public void setup() {
encryptor = new StringEncryptor("PBEWITHMD5AND256BITAES-CBC-OPENSSL", "BC", "nififtw!");
@ -74,6 +84,13 @@ public class FingerprintFactoryTest {
fingerprinter = new FingerprintFactory(encryptor, extensionManager);
}
@AfterClass
public static void tearDownOnce() {
if (ORIGINAL_NIFI_PROPS_PATH != null) {
System.setProperty(NiFiProperties.PROPERTIES_FILE_PATH, ORIGINAL_NIFI_PROPS_PATH);
}
}
@Test
public void testSameFingerprint() throws IOException {
final String fp1 = fingerprinter.createFingerprint(getResourceBytes("/nifi/fingerprint/flow1a.xml"), null);
@ -267,8 +284,22 @@ public class FingerprintFactoryTest {
when(component.getProxyPassword()).thenReturn(proxyPassword);
when(component.getVersionedComponentId()).thenReturn(Optional.empty());
// Build the same secure hasher to derive the HMAC key
Argon2SecureHasher a2sh = new Argon2SecureHasher();
// The nifi.properties file needs to be present
System.setProperty(NiFiProperties.PROPERTIES_FILE_PATH, TEST_NIFI_PROPS_PATH);
String npsk = NiFiPropertiesLoader.loadDefaultWithKeyFromBootstrap().getProperty(NiFiProperties.SENSITIVE_PROPS_KEY);
// The output will be 32B (256b)
byte[] sensitivePropertyKeyBytes = a2sh.hashRaw(npsk.getBytes(StandardCharsets.UTF_8));
Mac mac = Mac.getInstance("HmacSHA256");
mac.init(new SecretKeySpec(sensitivePropertyKeyBytes, "HmacSHA256"));
byte[] hashedBytes = mac.doFinal(proxyPassword.getBytes(StandardCharsets.UTF_8));
final String hashedProxyPassword = "[MASKED] (" + Base64.getEncoder().encodeToString(hashedBytes) + ")";
// Assert fingerprints with expected one.
final String hashedProxyPassword = "[MASKED] (" + new Argon2SecureHasher().hashBase64(proxyPassword) + ")";
final String expected = "id" +
"NO_VALUE" +
"http://node1:8080/nifi, http://node2:8080/nifi" +

15
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-properties-loader/src/main/java/org/apache/nifi/properties/NiFiPropertiesLoader.java
View File

@ -86,6 +86,13 @@ public class NiFiPropertiesLoader {
* or nifi.properties files
*/
public static NiFiProperties loadDefaultWithKeyFromBootstrap() throws IOException {
// The nifi.properties file may not be encrypted, so attempt to naively load it first
try {
return new NiFiPropertiesLoader().loadDefault();
} catch (Exception e) {
logger.warn("Encountered an error naively loading the nifi.properties file because one or more properties are protected: {}", e.getLocalizedMessage());
}
try {
String keyHex = CryptoUtils.extractKeyFromBootstrapFile();
return NiFiPropertiesLoader.withKey(keyHex).loadDefault();
@ -98,11 +105,9 @@ public class NiFiPropertiesLoader {
/**
* Returns the key (if any) used to encrypt sensitive properties, extracted from {@code $NIFI_HOME/conf/bootstrap.conf}.
*
* @deprecated
* Use {@link CryptoUtils#extractKeyFromBootstrapFile()} instead.
*
* @return the key in hexadecimal format
* @throws IOException if the file is not readable
* @deprecated Use {@link CryptoUtils#extractKeyFromBootstrapFile()} instead.
*/
@Deprecated
public static String extractKeyFromBootstrapFile() throws IOException {
@ -113,12 +118,10 @@ public class NiFiPropertiesLoader {
/**
* Returns the key (if any) used to encrypt sensitive properties, extracted from {@code $NIFI_HOME/conf/bootstrap.conf}.
*
* @deprecated
* Use {@link CryptoUtils#extractKeyFromBootstrapFile(String)} instead.
*
* @param bootstrapPath the path to the bootstrap file
* @return the key in hexadecimal format
* @throws IOException if the file is not readable
* @deprecated Use {@link CryptoUtils#extractKeyFromBootstrapFile(String)} instead.
*/
@Deprecated
public static String extractKeyFromBootstrapFile(String bootstrapPath) throws IOException {

38
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-properties-loader/src/test/groovy/org/apache/nifi/properties/NiFiPropertiesLoaderGroovyTest.groovy
View File

@ -163,6 +163,44 @@ class NiFiPropertiesLoaderGroovyTest extends GroovyTestCase {
assert niFiProperties instanceof StandardNiFiProperties
}
@Test
void testShouldLoadUnprotectedPropertiesFromFileWithoutBootstrap() throws Exception {
// Arrange
File unprotectedFile = new File("src/test/resources/conf/nifi.properties")
// Set the system property to the test file
System.setProperty(NiFiProperties.PROPERTIES_FILE_PATH, unprotectedFile.absolutePath)
logger.info("Set ${NiFiProperties.PROPERTIES_FILE_PATH} to ${unprotectedFile.absolutePath}")
// Act
NiFiProperties niFiProperties = NiFiPropertiesLoader.loadDefaultWithKeyFromBootstrap()
// Assert
assert niFiProperties.size() > 0
// Ensure it is not a ProtectedNiFiProperties
assert niFiProperties instanceof StandardNiFiProperties
}
@Test
void testShouldNotLoadProtectedPropertiesFromFileWithoutBootstrap() throws Exception {
// Arrange
File protectedFile = new File("src/test/resources/conf/nifi_with_sensitive_properties_protected_aes.properties")
// Set the system property to the test file
System.setProperty(NiFiProperties.PROPERTIES_FILE_PATH, protectedFile.absolutePath)
logger.info("Set ${NiFiProperties.PROPERTIES_FILE_PATH} to ${protectedFile.absolutePath}")
// Act
def msg = shouldFail(IOException) {
NiFiProperties niFiProperties = NiFiPropertiesLoader.loadDefaultWithKeyFromBootstrap()
}
logger.expected(msg)
// Assert
assert msg =~ "Cannot read from bootstrap.conf"
}
@Test
void testShouldNotLoadUnprotectedPropertiesFromNullFile() throws Exception {
// Arrange