From 8996b7f6d6f9aeead067681fe69cba5a7e1f0482 Mon Sep 17 00:00:00 2001 From: Andy LoPresto Date: Thu, 14 Jun 2018 20:38:11 -0700 Subject: [PATCH] NIFI-5193 Added logic to handle complex user filter expressions. Added unit tests. Added unit test resources. Fixed comments. Refactored XmlSlurper instantiation to keep ignorable whitespace. Added logic to handle LIP complex user search filter. Added unit tests. Added unit test resources. Removed unnecessary substitution/repopulation logic from encrypt|decryptAuthorizers. All unit tests pass. Removed unnecessary substitution/repopulation logic from CET. Removed unnecessary unit tests. Removed unnecessary commons-text dependency from pom.xml. This closes #2797. Signed-off-by: Bryan Bende --- .../nifi-toolkit-encrypt-config/pom.xml | 12 + .../properties/ConfigEncryptionTool.groovy | 103 +++--- .../ConfigEncryptionToolTest.groovy | 156 ++++++++- .../authorizers-populated-complex-filter.xml | 309 ++++++++++++++++++ ...ity-providers-populated-complex-filter.xml | 105 ++++++ 5 files changed, 644 insertions(+), 41 deletions(-) create mode 100644 nifi-toolkit/nifi-toolkit-encrypt-config/src/test/resources/authorizers-populated-complex-filter.xml create mode 100644 nifi-toolkit/nifi-toolkit-encrypt-config/src/test/resources/login-identity-providers-populated-complex-filter.xml diff --git a/nifi-toolkit/nifi-toolkit-encrypt-config/pom.xml b/nifi-toolkit/nifi-toolkit-encrypt-config/pom.xml index 8acf23a1a4..2969e4e5ab 100644 --- a/nifi-toolkit/nifi-toolkit-encrypt-config/pom.xml +++ b/nifi-toolkit/nifi-toolkit-encrypt-config/pom.xml @@ -97,6 +97,18 @@ 2.2.2 test + + org.xmlunit + xmlunit-core + 2.6.0 + test + + + org.xmlunit + xmlunit-matchers + 2.6.0 + test + 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 6ebcd0aa31..327735bd57 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 @@ -17,6 +17,7 @@ package org.apache.nifi.properties import groovy.io.GroovyPrintWriter +import groovy.util.slurpersupport.GPathResult import groovy.xml.XmlUtil import org.apache.commons.cli.CommandLine import org.apache.commons.cli.CommandLineParser @@ -47,6 +48,7 @@ import java.nio.charset.StandardCharsets import java.security.KeyException import java.security.SecureRandom import java.security.Security +import java.util.regex.Matcher import java.util.zip.GZIPInputStream import java.util.zip.GZIPOutputStream @@ -187,12 +189,12 @@ class ConfigEncryptionTool { private static final String DEFAULT_FLOW_ALGORITHM = "PBEWITHMD5AND256BITAES-CBC-OPENSSL" private static final Map PROPERTY_KEY_MAP = [ - "nifi.security.keystore": "keystore", - "nifi.security.keystoreType": "keystoreType", - "nifi.security.keystorePasswd": "keystorePasswd", - "nifi.security.keyPasswd": "keyPasswd", - "nifi.security.truststore": "truststore", - "nifi.security.truststoreType": "truststoreType", + "nifi.security.keystore" : "keystore", + "nifi.security.keystoreType" : "keystoreType", + "nifi.security.keystorePasswd" : "keystorePasswd", + "nifi.security.keyPasswd" : "keyPasswd", + "nifi.security.truststore" : "truststore", + "nifi.security.truststoreType" : "truststoreType", "nifi.security.truststorePasswd": "truststorePasswd", ] @@ -251,11 +253,11 @@ class ConfigEncryptionTool { return staticOptions } -/** - * Prints the usage message and available arguments for this tool (along with a specific error message if provided). - * - * @param errorMessage the optional error message - */ + /** + * Prints the usage message and available arguments for this tool (along with a specific error message if provided). + * + * @param errorMessage the optional error message + */ void printUsage(String errorMessage) { if (errorMessage) { System.out.println(errorMessage) @@ -491,15 +493,15 @@ class ConfigEncryptionTool { } } -/** - * The method returns the provided, derived, or securely-entered key in hex format. The reason the parameters must be provided instead of read from the fields is because this is used for the regular key/password and the migration key/password. - * - * @param device - * @param keyHex - * @param password - * @param usingPassword - * @return - */ + /** + * The method returns the provided, derived, or securely-entered key in hex format. The reason the parameters must be provided instead of read from the fields is because this is used for the regular key/password and the migration key/password. + * + * @param device + * @param keyHex + * @param password + * @param usingPassword + * @return + */ private String getKeyInternal(TextDevice device = TextDevices.defaultTextDevice(), String keyHex, String password, boolean usingPassword) { if (usingPassword) { if (!password) { @@ -527,7 +529,7 @@ class ConfigEncryptionTool { } private String getMigrationKey() { - return getKeyInternal(TextDevices.defaultTextDevice(), migrationKeyHex, migrationPassword, usingPasswordMigration) + return getKeyInternal(TextDevices.defaultTextDevice(), migrationKeyHex, migrationPassword, usingPasswordMigration) } private static String getFlowPassword(TextDevice textDevice = TextDevices.defaultTextDevice()) { @@ -874,7 +876,7 @@ class ConfigEncryptionTool { AESSensitivePropertyProvider sensitivePropertyProvider = new AESSensitivePropertyProvider(existingKeyHex) try { - def doc = new XmlSlurper().parseText(encryptedXml) + def doc = getXmlSlurper().parseText(encryptedXml) // Find the provider element by class even if it has been renamed def passwords = doc.provider.find { it.'class' as String == LDAP_PROVIDER_CLASS }.property.findAll { it.@name =~ "Password" && it.@encryption =~ "aes/gcm/\\d{3}" @@ -910,7 +912,8 @@ class ConfigEncryptionTool { AESSensitivePropertyProvider sensitivePropertyProvider = new AESSensitivePropertyProvider(existingKeyHex) try { - def doc = new XmlSlurper().parseText(encryptedXml) + def filename = "authorizers.xml" + def doc = getXmlSlurper().parseText(encryptedXml) // Find the provider element by class even if it has been renamed def passwords = doc.userGroupProvider.find { it.'class' as String == LDAP_USER_GROUP_PROVIDER_CLASS @@ -920,7 +923,7 @@ class ConfigEncryptionTool { if (passwords.isEmpty()) { if (isVerbose) { - logger.info("No encrypted password property elements found in authorizers.xml") + logger.info("No encrypted password property elements found in ${filename}") } return encryptedXml } @@ -938,7 +941,9 @@ class ConfigEncryptionTool { // Does not preserve whitespace formatting or comments String updatedXml = XmlUtil.serialize(doc) - logger.info("Updated XML content: ${updatedXml}") + if (isVerbose) { + logger.info("Updated XML content: ${updatedXml}") + } updatedXml } catch (Exception e) { printUsageAndThrow("Cannot decrypt authorizers XML content", ExitCode.SERVICE_ERROR) @@ -950,7 +955,7 @@ class ConfigEncryptionTool { // TODO: Switch to XmlParser & XmlNodePrinter to maintain "empty" element structure try { - def doc = new XmlSlurper().parseText(plainXml) + def doc = getXmlSlurper().parseText(plainXml) // Find the provider element by class even if it has been renamed def passwords = doc.provider.find { it.'class' as String == LDAP_PROVIDER_CLASS } .property.findAll { @@ -992,7 +997,8 @@ class ConfigEncryptionTool { // TODO: Switch to XmlParser & XmlNodePrinter to maintain "empty" element structure try { - def doc = new XmlSlurper().parseText(plainXml) + def filename = "authorizers.xml" + def doc = getXmlSlurper().parseText(plainXml) // Find the provider element by class even if it has been renamed def passwords = doc.userGroupProvider.find { it.'class' as String == LDAP_USER_GROUP_PROVIDER_CLASS } .property.findAll { @@ -1002,7 +1008,7 @@ class ConfigEncryptionTool { if (passwords.isEmpty()) { if (isVerbose) { - logger.info("No unencrypted password property elements found in authorizers.xml") + logger.info("No unencrypted password property elements found in ${filename}") } return plainXml } @@ -1019,7 +1025,9 @@ class ConfigEncryptionTool { // Does not preserve whitespace formatting or comments String updatedXml = XmlUtil.serialize(doc) - logger.info("Updated XML content: ${updatedXml}") + if (isVerbose) { + logger.info("Updated XML content: ${updatedXml}") + } updatedXml } catch (Exception e) { if (isVerbose) { @@ -1088,6 +1096,16 @@ class ConfigEncryptionTool { mergedProperties } + /** + * Returns the XML fragment serialized from the {@code GPathResult} without the leading XML declaration. + * + * @param gPathResult the XML node + * @return serialized XML without an inserted header declaration + */ + static String serializeXMLFragment(GPathResult gPathResult) { + XmlUtil.serialize(gPathResult).replaceFirst(XML_DECLARATION_REGEX, '') + } + /** * Reads the existing {@code bootstrap.conf} file, updates it to contain the master key, and persists it back to the same location. * @@ -1298,13 +1316,11 @@ class ConfigEncryptionTool { // Find the provider element of the new XML in the file contents String fileContents = originalLoginIdentityProvidersFile.text try { - def parsedXml = new XmlSlurper().parseText(xmlContent) + def parsedXml = getXmlSlurper().parseText(xmlContent) def provider = parsedXml.provider.find { it.'class' as String == LDAP_PROVIDER_CLASS } if (provider) { - def serializedProvider = new XmlUtil().serialize(provider) - // Remove XML declaration from top - serializedProvider = serializedProvider.replaceFirst(XML_DECLARATION_REGEX, "") - fileContents = fileContents.replaceFirst(LDAP_PROVIDER_REGEX, serializedProvider) + def serializedProvider = serializeXMLFragment(provider) + fileContents = fileContents.replaceFirst(LDAP_PROVIDER_REGEX, Matcher.quoteReplacement(serializedProvider)) return fileContents.split("\n") } else { throw new SAXException("No ldap-provider element found") @@ -1320,13 +1336,11 @@ class ConfigEncryptionTool { // Find the provider element of the new XML in the file contents String fileContents = originalAuthorizersFile.text try { - def parsedXml = new XmlSlurper().parseText(xmlContent) + def parsedXml = getXmlSlurper().parseText(xmlContent) def provider = parsedXml.userGroupProvider.find { it.'class' as String == LDAP_USER_GROUP_PROVIDER_CLASS } if (provider) { - def serializedProvider = new XmlUtil().serialize(provider) - // Remove XML declaration from top - serializedProvider = serializedProvider.replaceFirst(XML_DECLARATION_REGEX, "") - fileContents = fileContents.replaceFirst(LDAP_USER_GROUP_PROVIDER_REGEX, serializedProvider) + def serializedProvider = serializeXMLFragment(provider) + fileContents = fileContents.replaceFirst(LDAP_USER_GROUP_PROVIDER_REGEX, Matcher.quoteReplacement(serializedProvider)) return fileContents.split("\n") } else { throw new SAXException("No ldap-user-group-provider element found") @@ -1403,6 +1417,17 @@ class ConfigEncryptionTool { } } + /** + * Returns an {@link XmlSlurper} which is configured to maintain ignorable whitespace. + * + * @return a configured XmlSlurper + */ + static XmlSlurper getXmlSlurper() { + XmlSlurper xs = new XmlSlurper() + xs.setKeepIgnorableWhitespace(true) + xs + } + /** * Runs main tool logic (parsing arguments, reading files, protecting properties, and writing key and properties out to destination files). * 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 b6bc722618..e2d1ec136c 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 @@ -43,6 +43,10 @@ import org.junit.runner.RunWith import org.junit.runners.JUnit4 import org.slf4j.Logger import org.slf4j.LoggerFactory +import org.xmlunit.builder.DiffBuilder +import org.xmlunit.diff.DefaultNodeMatcher +import org.xmlunit.diff.Diff +import org.xmlunit.diff.ElementSelectors import javax.crypto.Cipher import javax.crypto.SecretKey @@ -644,8 +648,8 @@ class ConfigEncryptionToolTest extends GroovyTestCase { // Arrange Map keyValues = [ (KEY_HEX) : KEY_HEX, - " ${KEY_HEX} " : KEY_HEX, - "xxx${KEY_HEX}zzz" : KEY_HEX, + (" ${KEY_HEX} " as String) : KEY_HEX, + ("xxx${KEY_HEX}zzz" as String) : KEY_HEX, ((["0123", "4567"] * 4).join("-")): "01234567" * 4, ((["89ab", "cdef"] * 4).join(" ")): "89ABCDEF" * 4, (KEY_HEX.toLowerCase()) : KEY_HEX, @@ -2454,6 +2458,33 @@ class ConfigEncryptionToolTest extends GroovyTestCase { } } + @Test + void testSerializeLoginIdentityProvidersAndPreserveFormatShouldHandleComplexProperty() { + // Arrange + String providersPath = "src/test/resources/login-identity-providers-populated-complex-filter.xml" + File providersFile = new File(providersPath) + + File tmpDir = setupTmpDir() + + File workingFile = new File("target/tmp/tmp-providers.xml") + workingFile.delete() + Files.copy(providersFile.toPath(), workingFile.toPath()) + ConfigEncryptionTool tool = new ConfigEncryptionTool() + tool.isVerbose = true + + tool.keyHex = KEY_HEX + + def lines = workingFile.readLines() + logger.info("Read lines: \n${lines.join("\n")}") + + // Act + def serializedLines = ConfigEncryptionTool.serializeLoginIdentityProvidersAndPreserveFormat(lines.join("\n"), workingFile) + logger.info("Serialized lines: \n${serializedLines.join("\n")}") + + // Assert + assert compareXMLFragments(lines.join("\n"), serializedLines.join("\n")) + } + @Test void testWriteLoginIdentityProvidersShouldHandleUnreadableFile() { // Arrange @@ -3199,6 +3230,33 @@ class ConfigEncryptionToolTest extends GroovyTestCase { assert writtenLines.findAll { it =~ "encryption=" }.size() == AUTHORIZERS_PASSWORD_LINE_COUNT } + @Test + void testSerializeAuthorizersAndPreserveFormatShouldHandleComplexProperty() { + // Arrange + String authorizersPath = "src/test/resources/authorizers-populated-complex-filter.xml" + File authorizersFile = new File(authorizersPath) + + File tmpDir = setupTmpDir() + + File workingFile = new File("target/tmp/tmp-authorizers.xml") + workingFile.delete() + Files.copy(authorizersFile.toPath(), workingFile.toPath()) + ConfigEncryptionTool tool = new ConfigEncryptionTool() + tool.isVerbose = true + + tool.keyHex = KEY_HEX + + def lines = workingFile.readLines() + logger.info("Read lines: \n${lines.join("\n")}") + + // Act + def serializedLines = ConfigEncryptionTool.serializeAuthorizersAndPreserveFormat(lines.join("\n"), workingFile) + logger.info("Serialized lines: \n${serializedLines.join("\n")}") + + // Assert + assert compareXMLFragments(lines.join("\n"), serializedLines.join("\n")) + } + @Test void testShouldPerformFullOperationForAuthorizers() { // Arrange @@ -3361,6 +3419,87 @@ class ConfigEncryptionToolTest extends GroovyTestCase { // Assertions defined above } + @Test + void testShouldPerformFullOperationForAuthorizersWithComplexUserSearchFilter() { + // Arrange + exit.expectSystemExitWithStatus(0) + + File tmpDir = setupTmpDir() + + File emptyKeyFile = new File("src/test/resources/bootstrap_with_empty_master_key.conf") + File bootstrapFile = new File("target/tmp/tmp_bootstrap.conf") + bootstrapFile.delete() + + Files.copy(emptyKeyFile.toPath(), bootstrapFile.toPath()) + final List originalBootstrapLines = bootstrapFile.readLines() + String originalKeyLine = originalBootstrapLines.find { + it.startsWith(ConfigEncryptionTool.BOOTSTRAP_KEY_PREFIX) + } + logger.info("Original key line from bootstrap.conf: ${originalKeyLine}") + assert originalKeyLine == ConfigEncryptionTool.BOOTSTRAP_KEY_PREFIX + + final String EXPECTED_KEY_LINE = ConfigEncryptionTool.BOOTSTRAP_KEY_PREFIX + KEY_HEX + + File inputAuthorizersFile = new File("src/test/resources/authorizers-populated-complex-filter.xml") + File outputAuthorizersFile = new File("target/tmp/tmp-authorizers.xml") + outputAuthorizersFile.delete() + + String originalXmlContent = inputAuthorizersFile.text + logger.info("Original XML content: ${originalXmlContent}") + + String[] args = ["-a", inputAuthorizersFile.path, "-b", bootstrapFile.path, "-u", outputAuthorizersFile.path, "-k", KEY_HEX, "-v"] + + AESSensitivePropertyProvider spp = new AESSensitivePropertyProvider(KEY_HEX) + + exit.checkAssertionAfterwards(new Assertion() { + void checkAssertion() { + final String updatedXmlContent = outputAuthorizersFile.text + logger.info("Updated XML content: ${updatedXmlContent}") + + // Check that the output values for sensitive properties are not the same as the original (i.e. it was encrypted) + def originalParsedXml = new XmlSlurper().parseText(originalXmlContent) + def updatedParsedXml = new XmlSlurper().parseText(updatedXmlContent) + assert originalParsedXml != updatedParsedXml + assert originalParsedXml.'**'.findAll { it.@encryption } != updatedParsedXml.'**'.findAll { + it.@encryption + } + + def encryptedValues = updatedParsedXml.userGroupProvider.find { + it.identifier == 'ldap-user-group-provider' + }.property.findAll { + it.@name =~ "Password" && it.@encryption =~ "aes/gcm/\\d{3}" + } + + encryptedValues.each { + assert spp.unprotect(it.text()) == PASSWORD + } + + // Check that the key was persisted to the bootstrap.conf + final List updatedBootstrapLines = bootstrapFile.readLines() + String updatedKeyLine = updatedBootstrapLines.find { + it.startsWith(ConfigEncryptionTool.BOOTSTRAP_KEY_PREFIX) + } + logger.info("Updated key line: ${updatedKeyLine}") + + assert updatedKeyLine == EXPECTED_KEY_LINE + assert originalBootstrapLines.size() == updatedBootstrapLines.size() + + // Clean up + outputAuthorizersFile.deleteOnExit() + bootstrapFile.deleteOnExit() + tmpDir.deleteOnExit() + } + }) + + // Act + ConfigEncryptionTool.main(args) + logger.info("Invoked #main with ${args.join(" ")}") + + // Assert + + // Assertions defined above + } + @Test void testShouldPerformFullOperationForNiFiPropertiesAndLoginIdentityProvidersAndAuthorizers() { // Arrange @@ -4920,6 +5059,19 @@ class ConfigEncryptionToolTest extends GroovyTestCase { } } + static boolean compareXMLFragments(String expectedXML, String actualXML) { + Diff diffSimilar = DiffBuilder.compare(expectedXML).withTest(actualXML) + .withNodeMatcher(new DefaultNodeMatcher(ElementSelectors.byName)) + .ignoreWhitespace().checkForSimilar().build() + def allDifferences = diffSimilar.getDifferences() + if (diffSimilar.hasDifferences()) { + allDifferences.each { diff -> + logger.info("Difference: ${diff.toString()}") + } + } + !diffSimilar.hasDifferences() + } + // TODO: Test with 128/256-bit available } diff --git a/nifi-toolkit/nifi-toolkit-encrypt-config/src/test/resources/authorizers-populated-complex-filter.xml b/nifi-toolkit/nifi-toolkit-encrypt-config/src/test/resources/authorizers-populated-complex-filter.xml new file mode 100644 index 0000000000..79e7eafda2 --- /dev/null +++ b/nifi-toolkit/nifi-toolkit-encrypt-config/src/test/resources/authorizers-populated-complex-filter.xml @@ -0,0 +1,309 @@ + + + + + + + + + + + ldap-user-group-provider + org.apache.nifi.ldap.tenants.LdapUserGroupProvider + START_TLS + + someuser + thisIsABadPassword + + + thisIsABadPassword + + + thisIsABadPassword + + + + + + FOLLOW + 10 secs + 10 secs + + + + 30 mins + + + person + ONE_LEVEL + (& (objectCategory=Person)(sAMAccountName=*)(!(UserAccountControl:1.2.840.113556.1.4.803:=2))(!(sAMAccountName=$*))) + + + + + + group + ONE_LEVEL + (& (objectCategory=Person)(sAMAccountName=*)(!(UserAccountControl:1.2.840.113556.1.4.803:=2))(!(sAMAccountName=$*))) + + + + + + + + + + + + + + file-access-policy-provider + org.apache.nifi.authorization.FileAccessPolicyProvider + ldap-user-group-provider + ./conf/authorizations.xml + + + + + + + + + managed-authorizer + org.apache.nifi.authorization.StandardManagedAuthorizer + file-access-policy-provider + + + + + \ No newline at end of file diff --git a/nifi-toolkit/nifi-toolkit-encrypt-config/src/test/resources/login-identity-providers-populated-complex-filter.xml b/nifi-toolkit/nifi-toolkit-encrypt-config/src/test/resources/login-identity-providers-populated-complex-filter.xml new file mode 100644 index 0000000000..b9ba74d14c --- /dev/null +++ b/nifi-toolkit/nifi-toolkit-encrypt-config/src/test/resources/login-identity-providers-populated-complex-filter.xml @@ -0,0 +1,105 @@ + + + + + + + ldap-provider + org.apache.nifi.ldap.LdapProvider + START_TLS + + someuser + thisIsABadPassword + + + thisIsABadPassword + + + thisIsABadPassword + + + + + + FOLLOW + 10 secs + 10 secs + + + + (& (objectCategory=Person)(sAMAccountName=*)(!(UserAccountControl:1.2.840.113556.1.4.803:=2))(!(sAMAccountName=$*))) + + 12 hours + + + + + \ No newline at end of file