From commits-return-41827-archive-asf-public=cust-asf.ponee.io@nifi.apache.org Tue Oct 1 17:26:54 2019 Return-Path: X-Original-To: archive-asf-public@cust-asf.ponee.io Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [207.244.88.153]) by mx-eu-01.ponee.io (Postfix) with SMTP id C50F6180608 for ; Tue, 1 Oct 2019 19:26:53 +0200 (CEST) Received: (qmail 30173 invoked by uid 500); 1 Oct 2019 17:26:53 -0000 Mailing-List: contact commits-help@nifi.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@nifi.apache.org Delivered-To: mailing list commits@nifi.apache.org Received: (qmail 30157 invoked by uid 99); 1 Oct 2019 17:26:53 -0000 Received: from ec2-52-202-80-70.compute-1.amazonaws.com (HELO gitbox.apache.org) (52.202.80.70) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 01 Oct 2019 17:26:52 +0000 Received: by gitbox.apache.org (ASF Mail Server at gitbox.apache.org, from userid 33) id D3F2D890A3; Tue, 1 Oct 2019 17:26:52 +0000 (UTC) Date: Tue, 01 Oct 2019 17:26:53 +0000 To: "commits@nifi.apache.org" Subject: [nifi] 02/02: NIFI-4573 This closes #3460. Refactored logic handling flow XML encryption migration. Added unit tests. MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit From: joewitt@apache.org In-Reply-To: <156995081032.1605.9636484594180806802@gitbox.apache.org> References: <156995081032.1605.9636484594180806802@gitbox.apache.org> X-Git-Host: gitbox.apache.org X-Git-Repo: nifi X-Git-Refname: refs/heads/master X-Git-Reftype: branch X-Git-Rev: c2746fac5ff210a56704bf5ffe278359b9c85b54 X-Git-NotificationType: diff X-Git-Multimail-Version: 1.5.dev Auto-Submitted: auto-generated Message-Id: <20191001172652.D3F2D890A3@gitbox.apache.org> This is an automated email from the ASF dual-hosted git repository. joewitt pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/nifi.git commit c2746fac5ff210a56704bf5ffe278359b9c85b54 Author: Andy LoPresto AuthorDate: Thu May 2 18:53:03 2019 -0700 NIFI-4573 This closes #3460. Refactored logic handling flow XML encryption migration. Added unit tests. Signed-off-by: Joe Witt --- .../nifi/properties/ConfigEncryptionTool.groovy | 109 ++++++++++++--------- .../properties/ConfigEncryptionToolTest.groovy | 97 ++++++++++++++++++ 2 files changed, 157 insertions(+), 49 deletions(-) diff --git a/nifi-toolkit/nifi-toolkit-encrypt-config/src/main/groovy/org/apache/nifi/properties/ConfigEncryptionTool.groovy b/nifi-toolkit/nifi-toolkit-encrypt-config/src/main/groovy/org/apache/nifi/properties/ConfigEncryptionTool.groovy index b20399d..7c9164b 100644 --- a/nifi-toolkit/nifi-toolkit-encrypt-config/src/main/groovy/org/apache/nifi/properties/ConfigEncryptionTool.groovy +++ b/nifi-toolkit/nifi-toolkit-encrypt-config/src/main/groovy/org/apache/nifi/properties/ConfigEncryptionTool.groovy @@ -39,6 +39,7 @@ import org.slf4j.Logger import org.slf4j.LoggerFactory import org.xml.sax.SAXException +import javax.crypto.BadPaddingException import javax.crypto.Cipher import javax.crypto.SecretKey import javax.crypto.SecretKeyFactory @@ -1536,58 +1537,12 @@ class ConfigEncryptionTool { try { tool.flowXml = tool.loadFlowXml() } catch (Exception e) { - tool.printUsageAndThrow("Cannot load flow.xml.gz", ExitCode.ERROR_READING_NIFI_PROPERTIES) - } - - // If the flow password was not set in nifi.properties, use the hard-coded default - String existingFlowPassword = tool.getExistingFlowPassword() - - // If the new password was not provided in the arguments, read from the console. If that is empty, use the same value (essentially a copy no-op) - String newFlowPassword = tool.flowPropertiesPassword ?: tool.getFlowPassword() - if (!newFlowPassword) { - newFlowPassword = existingFlowPassword - } - - // Get the algorithms and providers - NiFiProperties nfp = tool.niFiProperties - String existingAlgorithm = nfp?.getProperty(NiFiProperties.SENSITIVE_PROPS_ALGORITHM) ?: DEFAULT_FLOW_ALGORITHM - String existingProvider = nfp?.getProperty(NiFiProperties.SENSITIVE_PROPS_PROVIDER) ?: DEFAULT_PROVIDER - - String newAlgorithm = tool.newFlowAlgorithm ?: existingAlgorithm - String newProvider = tool.newFlowProvider ?: existingProvider - - try { - tool.flowXml = tool.migrateFlowXmlContent(tool.flowXml, existingFlowPassword, newFlowPassword, existingAlgorithm, existingProvider, newAlgorithm, newProvider) - } catch (Exception e) { if (tool.isVerbose) { - logger.error("Encountered an error", e, "Check your password?") + logger.error("Encountered an error: ", e) } - tool.printUsageAndThrow("Encountered an error migrating flow content", ExitCode.ERROR_MIGRATING_FLOW) - } - - // If the new key is the hard-coded internal value, don't persist it to nifi.properties - if (newFlowPassword != DEFAULT_NIFI_SENSITIVE_PROPS_KEY && newFlowPassword != existingFlowPassword) { - // Update the NiFiProperties object with the new flow password before it gets encrypted (wasteful, but NiFiProperties instances are immutable) - Properties rawProperties = new Properties() - nfp.getPropertyKeys().each { String k -> - rawProperties.put(k, nfp.getProperty(k)) - } - - // If the tool is not going to encrypt NiFiProperties and the existing file is already encrypted, encrypt and update the new sensitive props key - if (!tool.handlingNiFiProperties && existingNiFiPropertiesAreEncrypted) { - AESSensitivePropertyProvider spp = new AESSensitivePropertyProvider(tool.keyHex) - String encryptedSPK = spp.protect(newFlowPassword) - rawProperties.put(NiFiProperties.SENSITIVE_PROPS_KEY, encryptedSPK) - // Manually update the protection scheme or it will be lost - rawProperties.put(ProtectedNiFiProperties.getProtectionKey(NiFiProperties.SENSITIVE_PROPS_KEY), spp.getIdentifierKey()) - if (tool.isVerbose) { - logger.info("Tool is not configured to encrypt nifi.properties, but the existing nifi.properties is encrypted and flow.xml.gz was migrated, so manually persisting the new encrypted value to nifi.properties") - } - } else { - rawProperties.put(NiFiProperties.SENSITIVE_PROPS_KEY, newFlowPassword) - } - tool.niFiProperties = new StandardNiFiProperties(rawProperties) + tool.printUsageAndThrow("Cannot load flow.xml.gz", ExitCode.ERROR_READING_NIFI_PROPERTIES) } + tool.handleFlowXml(existingNiFiPropertiesAreEncrypted) } if (tool.handlingNiFiProperties) { @@ -1637,6 +1592,62 @@ class ConfigEncryptionTool { System.exit(ExitCode.SUCCESS.ordinal()) } + void handleFlowXml(boolean existingNiFiPropertiesAreEncrypted = false) { + // If the flow password was not set in nifi.properties, use the hard-coded default + String existingFlowPassword = getExistingFlowPassword() + + // If the new password was not provided in the arguments, read from the console. If that is empty, use the same value (essentially a copy no-op) + String newFlowPassword = flowPropertiesPassword ?: getFlowPassword() + if (!newFlowPassword) { + newFlowPassword = existingFlowPassword + } + + // Get the algorithms and providers + NiFiProperties nfp = niFiProperties + String existingAlgorithm = nfp?.getProperty(NiFiProperties.SENSITIVE_PROPS_ALGORITHM) ?: DEFAULT_FLOW_ALGORITHM + String existingProvider = nfp?.getProperty(NiFiProperties.SENSITIVE_PROPS_PROVIDER) ?: DEFAULT_PROVIDER + + String newAlgorithm = newFlowAlgorithm ?: existingAlgorithm + String newProvider = newFlowProvider ?: existingProvider + + try { + flowXml = migrateFlowXmlContent(flowXml, existingFlowPassword, newFlowPassword, existingAlgorithm, existingProvider, newAlgorithm, newProvider) + } catch (Exception e) { + logger.error("Encountered an error: ${e.getLocalizedMessage()}") + if (e instanceof BadPaddingException) { + logger.error("This error is likely caused by providing the wrong existing flow password. Check that the existing flow password [-p] is the one used to encrypt the provided flow.xml.gz file") + } + if (isVerbose) { + logger.error("Exception: ", e) + } + printUsageAndThrow("Encountered an error migrating flow content", ExitCode.ERROR_MIGRATING_FLOW) + } + + // If the new key is the hard-coded internal value, don't persist it to nifi.properties + if (newFlowPassword != DEFAULT_NIFI_SENSITIVE_PROPS_KEY && newFlowPassword != existingFlowPassword) { + // Update the NiFiProperties object with the new flow password before it gets encrypted (wasteful, but NiFiProperties instances are immutable) + Properties rawProperties = new Properties() + nfp.getPropertyKeys().each { String k -> + rawProperties.put(k, nfp.getProperty(k)) + } + + // If the tool is not going to encrypt NiFiProperties and the existing file is already encrypted, encrypt and update the new sensitive props key + if (!handlingNiFiProperties && existingNiFiPropertiesAreEncrypted) { + AESSensitivePropertyProvider spp = new AESSensitivePropertyProvider(keyHex) + String encryptedSPK = spp.protect(newFlowPassword) + rawProperties.put(NiFiProperties.SENSITIVE_PROPS_KEY, encryptedSPK) + // Manually update the protection scheme or it will be lost + rawProperties.put(ProtectedNiFiProperties.getProtectionKey(NiFiProperties.SENSITIVE_PROPS_KEY), spp.getIdentifierKey()) + if (isVerbose) { + logger.info("Tool is not configured to encrypt nifi.properties, but the existing nifi.properties is encrypted and flow.xml.gz was migrated, so manually persisting the new encrypted value to nifi.properties") + } + } else { + rawProperties.put(NiFiProperties.SENSITIVE_PROPS_KEY, newFlowPassword) + } + niFiProperties = new StandardNiFiProperties(rawProperties) + } + } + String translateNiFiPropertiesToCLI() { // Assemble the baseUrl String baseUrl = determineBaseUrl(niFiProperties) diff --git a/nifi-toolkit/nifi-toolkit-encrypt-config/src/test/groovy/org/apache/nifi/properties/ConfigEncryptionToolTest.groovy b/nifi-toolkit/nifi-toolkit-encrypt-config/src/test/groovy/org/apache/nifi/properties/ConfigEncryptionToolTest.groovy index e2d1ec1..317c779 100644 --- a/nifi-toolkit/nifi-toolkit-encrypt-config/src/test/groovy/org/apache/nifi/properties/ConfigEncryptionToolTest.groovy +++ b/nifi-toolkit/nifi-toolkit-encrypt-config/src/test/groovy/org/apache/nifi/properties/ConfigEncryptionToolTest.groovy @@ -38,6 +38,7 @@ import org.junit.Rule import org.junit.Test import org.junit.contrib.java.lang.system.Assertion import org.junit.contrib.java.lang.system.ExpectedSystemExit +import org.junit.contrib.java.lang.system.SystemErrRule import org.junit.contrib.java.lang.system.SystemOutRule import org.junit.runner.RunWith import org.junit.runners.JUnit4 @@ -48,6 +49,7 @@ import org.xmlunit.diff.DefaultNodeMatcher import org.xmlunit.diff.Diff import org.xmlunit.diff.ElementSelectors +import javax.crypto.BadPaddingException import javax.crypto.Cipher import javax.crypto.SecretKey import javax.crypto.SecretKeyFactory @@ -68,6 +70,9 @@ class ConfigEncryptionToolTest extends GroovyTestCase { @Rule public final SystemOutRule systemOutRule = new SystemOutRule().enableLog() + @Rule + public final SystemErrRule systemErrRule = new SystemErrRule().enableLog() + private static final String KEY_HEX_128 = "0123456789ABCDEFFEDCBA9876543210" private static final String KEY_HEX_256 = KEY_HEX_128 * 2 public static final String KEY_HEX = isUnlimitedStrengthCryptoAvailable() ? KEY_HEX_256 : KEY_HEX_128 @@ -4527,6 +4532,98 @@ class ConfigEncryptionToolTest extends GroovyTestCase { } } + /** + * This test is tightly scoped to the migration of the flow XML content to ensure the expected exception type is thrown. + */ + @Test + void testMigrateFlowXmlContentWithIncorrectExistingPasswordShouldFailWithBadPaddingException() { + // Arrange + String flowXmlPath = "src/test/resources/flow.xml" + File flowXmlFile = new File(flowXmlPath) + + File tmpDir = setupTmpDir() + + File workingFile = new File("target/tmp/tmp-flow.xml") + workingFile.delete() + Files.copy(flowXmlFile.toPath(), workingFile.toPath()) + ConfigEncryptionTool tool = new ConfigEncryptionTool() + tool.isVerbose = true + + // Use the wrong existing password + String wrongExistingFlowPassword = DEFAULT_LEGACY_SENSITIVE_PROPS_KEY.reverse() + String newFlowPassword = FLOW_PASSWORD + + String xmlContent = workingFile.text + + // Act + def message = shouldFail(BadPaddingException) { + String migratedXmlContent = tool.migrateFlowXmlContent(xmlContent, wrongExistingFlowPassword, newFlowPassword) + logger.info("Migrated flow.xml: \n${migratedXmlContent}") + } + logger.expected(message) + + // Assert + assert message =~ "pad block corrupted" + } + + /** + * This test is scoped to the higher-level method to ensure that if a bad padding exception is thrown, the right errors are displayed. + */ + @Test + void testHandleFlowXmlMigrationWithIncorrectExistingPasswordShouldProvideHelpfulErrorMessage() { + // Arrange +// exit.expectSystemExitWithStatus(ExitCode.ERROR_MIGRATING_FLOW.ordinal()) + systemOutRule.clearLog() + + String flowXmlPath = "src/test/resources/flow.xml" + File flowXmlFile = new File(flowXmlPath) + + File tmpDir = setupTmpDir() + + File workingFile = new File("target/tmp/tmp-flow.xml") + workingFile.delete() + Files.copy(flowXmlFile.toPath(), workingFile.toPath()) + ConfigEncryptionTool tool = new ConfigEncryptionTool() + tool.isVerbose = true + + // Use the wrong existing password + String wrongExistingFlowPassword = DEFAULT_LEGACY_SENSITIVE_PROPS_KEY.reverse() + String newFlowPassword = FLOW_PASSWORD + + tool.flowXml = workingFile.text + def nifiProperties = wrapNFP([(NiFiProperties.SENSITIVE_PROPS_KEY): wrongExistingFlowPassword]) + tool.niFiProperties = nifiProperties + tool.flowPropertiesPassword = newFlowPassword + tool.handlingNiFiProperties = false + + // Act + def message = shouldFail(Exception) { + tool.handleFlowXml() + logger.info("Migrated flow.xml: \n${tool.flowXml}") + } + logger.expected(message) + +// final String standardOutput = systemOutRule.getLog() +// List lines = standardOutput.split("\n") +// logger.info("Captured ${lines.size()} lines of log output") +// lines.each { String l -> logger.info("\t$l") } + +// final String errorOutput = systemErrRule.getLog() +// List errorlines = errorOutput.split("\n") +// logger.info("Captured ${errorlines.size()} lines of error log output") +// errorlines.each { String l -> logger.info("\t$l") } + + // Assert + // TODO: Assert that this message was in the log output (neither the STDOUT and STDERR buffers contain it, but it is printed) +// assert message =~ "Error performing flow XML content migration because some sensitive values could not be decrypted. Ensure that the existing flow password \\[\\-p\\] is correct." + assert message == "Encountered an error migrating flow content" + } + + private static StandardNiFiProperties wrapNFP(Map map) { + new StandardNiFiProperties( + new Properties(map)) + } + @Test void testShouldLoadFlowXmlContent() { // Arrange