You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by GitBox <gi...@apache.org> on 2022/08/09 15:23:58 UTC

[GitHub] [nifi] exceptionfactory commented on a diff in pull request #6273: NIFI-9953 - The config encryption tool is too complicated to use and can be simplified

exceptionfactory commented on code in PR #6273:
URL: https://github.com/apache/nifi/pull/6273#discussion_r941394985


##########
nifi-commons/nifi-property-protection-factory/src/main/java/org/apache/nifi/properties/scheme/PropertyProtectionScheme.java:
##########
@@ -19,7 +19,7 @@
 /**
  * Property Protection Schemes supported as arguments for encryption commands should not have direct references
  */
-enum PropertyProtectionScheme implements ProtectionScheme {
+public enum PropertyProtectionScheme implements ProtectionScheme {

Review Comment:
   This enum was intentionally package-private to encapsulate resolution of the scheme. As indicated in the enum values, there are different ways of referring to the scheme. With the introduction of a new command, this seems like a good opportunity to change the argument contract to use the scheme values, instead of the enum names.



##########
nifi-toolkit/nifi-property-encryptor-tool/pom.xml:
##########
@@ -0,0 +1,129 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+  Licensed to the Apache Software Foundation (ASF) under one or more
+  contributor license agreements. See the NOTICE file distributed with
+  this work for additional information regarding copyright ownership.
+  The ASF licenses this file to You under the Apache License, Version 2.0
+  (the "License"); you may not use this file except in compliance with
+  the License. You may obtain a copy of the License at
+  http://www.apache.org/licenses/LICENSE-2.0
+  Unless required by applicable law or agreed to in writing, software
+  distributed under the License is distributed on an "AS IS" BASIS,
+  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+  See the License for the specific language governing permissions and
+  limitations under the License.
+-->
+<project xmlns="http://maven.apache.org/POM/4.0.0"
+         xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+    <parent>
+        <artifactId>nifi</artifactId>
+        <groupId>org.apache.nifi</groupId>
+        <version>1.18.0-SNAPSHOT</version>
+    </parent>
+    <modelVersion>4.0.0</modelVersion>
+
+    <artifactId>nifi-property-encryptor-tool</artifactId>
+
+    <properties>
+        <maven.compiler.source>8</maven.compiler.source>
+        <maven.compiler.target>8</maven.compiler.target>
+    </properties>
+
+    <dependencies>
+        <dependency>
+            <groupId>info.picocli</groupId>
+            <artifactId>picocli</artifactId>
+            <version>4.6.3</version>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.nifi</groupId>
+            <artifactId>nifi-property-protection-factory</artifactId>
+            <version>1.18.0-SNAPSHOT</version>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.nifi</groupId>
+            <artifactId>nifi-properties-loader</artifactId>
+            <version>1.18.0-SNAPSHOT</version>
+            <scope>compile</scope>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.nifi.registry</groupId>
+            <artifactId>nifi-registry-properties</artifactId>
+            <version>1.18.0-SNAPSHOT</version>
+            <scope>compile</scope>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.nifi.registry</groupId>
+            <artifactId>nifi-registry-properties-loader</artifactId>
+            <version>1.18.0-SNAPSHOT</version>
+            <scope>compile</scope>
+        </dependency>
+        <dependency>
+            <groupId>org.slf4j</groupId>
+            <artifactId>slf4j-api</artifactId>
+            <version>${org.slf4j.version}</version>
+        </dependency>
+        <dependency>
+            <groupId>org.slf4j</groupId>
+            <artifactId>slf4j-simple</artifactId>
+            <version>${org.slf4j.version}</version>
+        </dependency>
+    </dependencies>
+    <build>
+        <plugins>
+            <plugin>
+                <artifactId>maven-assembly-plugin</artifactId>
+                <configuration>
+                    <archive>
+                        <manifest>
+                            <mainClass>org.apache.nifi.util.console.PropertyEncryptorCLI</mainClass>
+                        </manifest>
+                    </archive>
+                    <descriptorRefs>
+                        <descriptorRef>jar-with-dependencies</descriptorRef>
+                    </descriptorRefs>
+                </configuration>
+                <executions>
+                    <execution>
+                        <id>make-assembly</id>
+                        <phase>package</phase>
+                        <goals>
+                            <goal>single</goal>
+                        </goals>
+                    </execution>
+                </executions>
+            </plugin>
+            <plugin>
+                <groupId>org.codehaus.mojo</groupId>
+                <artifactId>appassembler-maven-plugin</artifactId>
+                <version>2.1.0</version>
+                <configuration>
+                    <programs>
+                        <program>
+                            <mainClass>org.apache.nifi.util.console.PropertyEncryptorCLI</mainClass>
+                            <id>property-encryptor</id>
+                        </program>
+                    </programs>
+                </configuration>
+            </plugin>
+            <plugin>
+                <groupId>org.apache.maven.plugins</groupId>
+                <artifactId>maven-compiler-plugin</artifactId>
+                <!-- annotationProcessorPaths requires maven-compiler-plugin version 3.5 or higher -->

Review Comment:
   Is this comment necessary? The Maven compiler plugin version should be set in the root configuration.



##########
nifi-toolkit/nifi-property-encryptor-tool/src/main/java/org/apache/nifi/PropertyEncryptorMain.java:
##########
@@ -0,0 +1,173 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi;
+
+import org.apache.nifi.properties.AbstractBootstrapPropertiesLoader;
+import org.apache.nifi.properties.BootstrapProperties;
+import org.apache.nifi.properties.MutableBootstrapProperties;
+import org.apache.nifi.properties.SensitivePropertyProviderFactory;
+import org.apache.nifi.properties.StandardSensitivePropertyProviderFactory;
+import org.apache.nifi.properties.scheme.PropertyProtectionScheme;
+import org.apache.nifi.properties.scheme.ProtectionScheme;
+import org.apache.nifi.registry.properties.util.NiFiRegistryBootstrapPropertiesLoader;
+import org.apache.nifi.serde.PropertiesWriter;
+import org.apache.nifi.util.NiFiBootstrapPropertiesLoader;
+import org.apache.nifi.util.crypto.CryptographyUtils;
+import org.apache.nifi.xml.XmlDecryptor;
+import org.apache.nifi.xml.XmlEncryptor;
+import org.apache.nifi.util.file.ConfigurationFileResolver;
+import org.apache.nifi.util.file.FileUtilities;
+import org.apache.nifi.properties.ApplicationProperties;
+import org.apache.nifi.properties.NiFiPropertiesLoader;
+import org.apache.nifi.properties.PropertiesLoader;
+import org.apache.nifi.registry.properties.NiFiRegistryPropertiesLoader;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.nio.file.Path;
+import java.util.List;
+
+public class PropertyEncryptorMain {
+
+    private static final Logger logger = LoggerFactory.getLogger(PropertyEncryptorMain.class);
+    private final AbstractBootstrapPropertiesLoader bootstrapLoader;
+    private final PropertiesLoader<ApplicationProperties> propertiesLoader;
+    private final ConfigurationFileResolver fileResolver;
+    private final List<File> configurationFiles;
+    private String hexKey;
+    private final Path confDirectory;
+
+    public PropertyEncryptorMain(final Path baseDirectory, final String passphrase) throws PropertyEncryptorException {
+        confDirectory = FileUtilities.resolveAbsoluteConfDirectory(baseDirectory);
+        try {
+            bootstrapLoader = getBootstrapPropertiesLoader(confDirectory);
+            fileResolver = new ConfigurationFileResolver(confDirectory);
+            final File applicationProperties = FileUtilities.resolvePropertiesFile(confDirectory);
+            propertiesLoader = getPropertiesLoader(confDirectory);
+            configurationFiles = fileResolver.resolveConfigurationFilesFromApplicationProperties(propertiesLoader.load(applicationProperties));
+            hexKey = getKeyHex(confDirectory, passphrase);
+        } catch (final Exception e) {
+            throw new PropertyEncryptorException("Failed to run property encryptor", e);
+        }
+    }
+
+    private AbstractBootstrapPropertiesLoader getBootstrapPropertiesLoader(final Path baseDirectory) {
+        if (FileUtilities.isNiFiRegistryConfDirectory(baseDirectory)) {
+            return new NiFiRegistryBootstrapPropertiesLoader();
+        } else if (FileUtilities.isNiFiConfDirectory(baseDirectory)) {
+            return new NiFiBootstrapPropertiesLoader();
+        } else {
+            throw new PropertyEncryptorException("The base directory provided does not contain a recognized bootstrap.conf file");
+        }
+    }
+
+    private PropertiesLoader<ApplicationProperties> getPropertiesLoader(final Path baseDirectory) {
+        if (FileUtilities.isNiFiRegistryConfDirectory(baseDirectory)) {
+            return new NiFiRegistryPropertiesLoader();
+        } else if (FileUtilities.isNiFiConfDirectory(baseDirectory)) {
+            return new NiFiPropertiesLoader();
+        } else {
+            throw new PropertyEncryptorException("The base directory provided does not contain a recognized .properties file");
+        }
+    }
+
+    /**
+     * @param baseDirectory The base directory of a NiFi / NiFi Registry installation that should be encrypted
+     */
+    public int encryptConfigurationFiles(final Path baseDirectory, final PropertyProtectionScheme scheme) {
+        XmlEncryptor encryptor = getXmlEncryptor(scheme);
+        try {
+            encryptConfigurationFiles(configurationFiles, encryptor);
+            outputKeyToBootstrap();
+            logger.info("The Property Encryptor successfully encrypted configuration files in the [{}] directory with the scheme [{}]", baseDirectory, scheme.getPath());
+            return 0;
+        } catch (Exception e) {
+            logger.error("The Property Encryptor failed to encrypt configuration files in the [{}] directory with the scheme [{}]", baseDirectory, scheme.getPath(), e);
+            return 1;
+        }
+    }
+
+    private void outputKeyToBootstrap() throws IOException {
+        final File bootstrapFile = bootstrapLoader.getBootstrapFileWithinConfDirectory(confDirectory);
+        File tempBootstrapFile = FileUtilities.getTemporaryOutputFile("tmp", bootstrapFile);

Review Comment:
   The reference to `tmp` should be promoted to a static variable and reused.



##########
nifi-toolkit/nifi-property-encryptor-tool/src/main/java/org/apache/nifi/util/console/ConfigSubcommand.java:
##########
@@ -0,0 +1,50 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.util.console;
+
+import org.apache.nifi.PropertyEncryptorMain;
+import org.apache.nifi.properties.scheme.PropertyProtectionScheme;
+import org.apache.nifi.util.console.utils.BaseCommandParameters;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import picocli.CommandLine;
+
+@CommandLine.Command(name = "config",
+        description = "Operate on application configs",
+        usageHelpWidth=140
+)
+class ConfigSubcommand extends BaseCommandParameters implements Runnable {
+
+    private static final Logger logger = LoggerFactory.getLogger(ConfigSubcommand.class);
+
+    @CommandLine.ParentCommand
+    private DefaultCLIOptions parent;
+
+    @CommandLine.Parameters(description="The encryption scheme to use, from one of the following schemes: [@|bold ${COMPLETION-CANDIDATES}|@]")
+    PropertyProtectionScheme scheme;

Review Comment:
   As mentioned on the enum, this is a good opportunity to revisit the supported arguments. Right now, the supported arguments are different than the values used to reference encrypted properties, so it would be helpful to have a unified approach.



##########
nifi-toolkit/nifi-property-encryptor-tool/src/main/java/org/apache/nifi/util/console/PropertyEncryptorTranslateForCLI.java:
##########
@@ -0,0 +1,32 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.util.console;
+
+import picocli.CommandLine;
+
+@CommandLine.Command(name = "translate-for-nifi-cli", description = "@|bold,fg(blue) Translates|@ the nifi.properties file to a format suitable for use with the NiFi CLI tool")
+class PropertyEncryptorTranslateForCLI implements Runnable {

Review Comment:
   Are there applicable NiFi CLI commands for encryption and decryption? It seems like this may be unnecessary.



##########
nifi-toolkit/nifi-property-encryptor-tool/src/main/java/org/apache/nifi/util/crypto/CryptographyUtils.java:
##########
@@ -0,0 +1,75 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.util.crypto;
+
+import org.apache.commons.codec.binary.Hex;
+import org.bouncycastle.crypto.generators.SCrypt;
+
+import javax.crypto.Cipher;
+import java.nio.charset.StandardCharsets;
+import java.security.KeyException;
+import java.security.NoSuchAlgorithmException;
+import java.util.Arrays;
+import java.util.List;
+
+public class CryptographyUtils {
+
+    private static final String NIFI_SCRYPT_SALT = "NIFI_SCRYPT_SALT";
+    private static final int DEFAULT_MIN_PASSPHRASE_LENGTH = 12;
+
+    // Strong parameters as of 12 Aug 2016
+    private static final int SCRYPT_N = (int) Math.pow(2, 16);
+    private static final int SCRYPT_R = 8;
+    private static final int SCRYPT_P = 1;

Review Comment:
   It seems the like scrypt details should be abstracted in some kind of interface, perhaps a `SaltGenerator`?



##########
nifi-toolkit/nifi-property-encryptor-tool/src/main/java/org/apache/nifi/xml/XmlCryptoParser.java:
##########
@@ -0,0 +1,163 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.xml;
+
+import org.apache.nifi.properties.SensitivePropertyProvider;
+import org.apache.nifi.properties.SensitivePropertyProviderFactory;
+import org.apache.nifi.properties.scheme.ProtectionScheme;
+
+import javax.xml.namespace.QName;
+import javax.xml.stream.XMLEventFactory;
+import javax.xml.stream.XMLEventReader;
+import javax.xml.stream.XMLEventWriter;
+import javax.xml.stream.XMLInputFactory;
+import javax.xml.stream.XMLOutputFactory;
+import javax.xml.stream.XMLStreamException;
+import javax.xml.stream.events.Attribute;
+import javax.xml.stream.events.Characters;
+import javax.xml.stream.events.StartElement;
+import javax.xml.stream.events.XMLEvent;
+import java.io.FileNotFoundException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.util.ArrayList;
+import java.util.Iterator;
+import java.util.Objects;
+
+public abstract class XmlCryptoParser {

Review Comment:
   Similar to other components, defining an interface would provide a clear contract, as opposed to an abstract base class.



##########
nifi-toolkit/nifi-property-encryptor-tool/src/test/java/org/apache/nifi/encryptor/XmlEncryptorTest.java:
##########
@@ -0,0 +1,111 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.encryptor;
+
+import org.apache.nifi.util.crypto.CryptographyUtils;
+import org.apache.nifi.util.file.FileUtilities;
+import org.apache.nifi.properties.StandardSensitivePropertyProviderFactory;
+import org.apache.nifi.properties.scheme.ProtectionScheme;
+import org.apache.nifi.properties.scheme.StandardProtectionScheme;
+import org.apache.nifi.xml.XmlDecryptor;
+import org.apache.nifi.xml.XmlEncryptor;
+import org.junit.jupiter.api.Test;
+
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.net.URISyntaxException;
+import java.nio.file.Files;
+import java.util.List;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+class XmlEncryptorTest {
+
+    private static final String KEY_HEX_128 = "0123456789ABCDEFFEDCBA9876543210";
+    private static final String KEY_HEX_256 = KEY_HEX_128 + KEY_HEX_128;
+    static final ProtectionScheme DEFAULT_PROTECTION_SCHEME = new StandardProtectionScheme("aes/gcm");
+    public static final String KEY_HEX = CryptographyUtils.isUnlimitedStrengthCryptoAvailable() ? KEY_HEX_256 : KEY_HEX_128;

Review Comment:
   This can be refactored to just use AES-256 since Java 8 Update 171 and later do not limit key sizes.



##########
nifi-toolkit/nifi-property-encryptor-tool/src/main/java/org/apache/nifi/xml/XmlDecryptor.java:
##########
@@ -0,0 +1,104 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.xml;
+
+import org.apache.nifi.properties.SensitivePropertyProviderFactory;
+import org.apache.nifi.properties.scheme.ProtectionScheme;
+import javax.xml.namespace.QName;
+import javax.xml.stream.XMLEventFactory;
+import javax.xml.stream.XMLEventReader;
+import javax.xml.stream.XMLInputFactory;
+import javax.xml.stream.XMLStreamException;
+import javax.xml.stream.events.Characters;
+import javax.xml.stream.events.StartElement;
+import javax.xml.stream.events.XMLEvent;
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.util.Objects;
+
+public class XmlDecryptor extends XmlCryptoParser {
+
+    protected static final String ENCRYPTION_NONE = "none";
+    protected static final String PROPERTY_ELEMENT = "property";
+    protected static final String ENCRYPTION_ATTRIBUTE_NAME = "encryption";
+
+    public XmlDecryptor(final SensitivePropertyProviderFactory providerFactory, final ProtectionScheme scheme) {
+        super(providerFactory, scheme);
+        this.providerFactory = providerFactory;
+    }
+
+    public void decrypt(final InputStream encryptedXmlContent, final OutputStream decryptedOutputStream) {
+        cryptographicXmlOperation(encryptedXmlContent, decryptedOutputStream);
+    }
+
+    @Override
+    protected StartElement updateStartElementEncryptionAttribute(XMLEvent xmlEvent) {
+        return convertToDecryptedElement(xmlEvent);
+    }
+
+    @Override
+    protected Characters cryptoOperationOnCharacters(XMLEvent xmlEvent, String groupIdentifier, final String propertyName) {
+        return decryptElementCharacters(xmlEvent, groupIdentifier, propertyName);
+    }
+
+    /**
+     * Decrypt the XMLEvent element characters/value, which should contain an encrypted value
+     * @param xmlEvent The encrypted Characters event to be decrypted
+     * @return The decrypted Characters event
+     */
+    private Characters decryptElementCharacters(final XMLEvent xmlEvent, final String groupIdentifier, final String propertyName) {
+        final XMLEventFactory eventFactory = XMLEventFactory.newInstance();
+        final String encryptedCharacters = xmlEvent.asCharacters().getData().trim();
+        String decryptedCharacters = cryptoProvider.unprotect(encryptedCharacters, providerFactory.getPropertyContext(groupIdentifier, propertyName));
+        return eventFactory.createCharacters(decryptedCharacters);
+    }
+
+    /**
+     * Takes a StartElement and updates the 'encrypted' attribute to empty string to remove the encryption method/scheme
+     * @param xmlEvent The opening/start XMLEvent for an encrypted property
+     * @return The updated element to be written to XML file
+     */
+    private StartElement convertToDecryptedElement(final XMLEvent xmlEvent) {
+        if (isEncryptedElement(xmlEvent)) {
+            return updateElementAttribute(xmlEvent, ENCRYPTION_ATTRIBUTE_NAME, ENCRYPTION_NONE);
+        } else {
+            throw new XmlCryptoException(String.format("Failed to update the element's [%s] attribute when decrypting the element value", ENCRYPTION_ATTRIBUTE_NAME));
+        }
+    }
+
+    public boolean isEncryptedElement(final XMLEvent xmlEvent) {
+        return xmlEvent.isStartElement()
+                && xmlEvent.asStartElement().getName().toString().equals(PROPERTY_ELEMENT)
+                && elementHasEncryptionAttribute(xmlEvent.asStartElement());
+    }
+
+    private boolean elementHasEncryptionAttribute(final StartElement xmlEvent) {
+        return xmlElementHasAttribute(xmlEvent, ENCRYPTION_ATTRIBUTE_NAME);
+    }
+
+    private boolean xmlElementHasAttribute(final StartElement xmlEvent, final String attributeName) {
+        return !Objects.isNull(xmlEvent.getAttributeByName(new QName(attributeName)));
+    }
+
+    public XMLEventReader getXMLReader(final InputStream fileStream) throws XMLStreamException {
+        XMLInputFactory xmlInputFactory = XMLInputFactory.newFactory();
+        xmlInputFactory.setProperty(XMLInputFactory.IS_SUPPORTING_EXTERNAL_ENTITIES, false);
+        xmlInputFactory.setProperty(XMLInputFactory.SUPPORT_DTD, false);
+
+        return xmlInputFactory.createXMLEventReader(fileStream);
+    }

Review Comment:
   This should be refactored to use components from `nifi-xml-processing`.



##########
nifi-toolkit/nifi-property-encryptor-tool/src/main/java/org/apache/nifi/serde/PropertiesWriter.java:
##########
@@ -0,0 +1,51 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.serde;
+
+import org.apache.nifi.properties.ReadableProperties;
+
+import java.io.BufferedReader;
+import java.io.BufferedWriter;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.InputStreamReader;
+import java.io.OutputStream;
+import java.io.OutputStreamWriter;
+import java.util.Set;
+
+public class PropertiesWriter {

Review Comment:
   Recommend declaring an interface and creating a standard implementation, even if it is just one method.



##########
nifi-toolkit/nifi-property-encryptor-tool/src/main/java/org/apache/nifi/PropertyEncryptorMain.java:
##########
@@ -0,0 +1,173 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi;
+
+import org.apache.nifi.properties.AbstractBootstrapPropertiesLoader;
+import org.apache.nifi.properties.BootstrapProperties;
+import org.apache.nifi.properties.MutableBootstrapProperties;
+import org.apache.nifi.properties.SensitivePropertyProviderFactory;
+import org.apache.nifi.properties.StandardSensitivePropertyProviderFactory;
+import org.apache.nifi.properties.scheme.PropertyProtectionScheme;
+import org.apache.nifi.properties.scheme.ProtectionScheme;
+import org.apache.nifi.registry.properties.util.NiFiRegistryBootstrapPropertiesLoader;
+import org.apache.nifi.serde.PropertiesWriter;
+import org.apache.nifi.util.NiFiBootstrapPropertiesLoader;
+import org.apache.nifi.util.crypto.CryptographyUtils;
+import org.apache.nifi.xml.XmlDecryptor;
+import org.apache.nifi.xml.XmlEncryptor;
+import org.apache.nifi.util.file.ConfigurationFileResolver;
+import org.apache.nifi.util.file.FileUtilities;
+import org.apache.nifi.properties.ApplicationProperties;
+import org.apache.nifi.properties.NiFiPropertiesLoader;
+import org.apache.nifi.properties.PropertiesLoader;
+import org.apache.nifi.registry.properties.NiFiRegistryPropertiesLoader;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.nio.file.Path;
+import java.util.List;
+
+public class PropertyEncryptorMain {
+
+    private static final Logger logger = LoggerFactory.getLogger(PropertyEncryptorMain.class);
+    private final AbstractBootstrapPropertiesLoader bootstrapLoader;
+    private final PropertiesLoader<ApplicationProperties> propertiesLoader;
+    private final ConfigurationFileResolver fileResolver;
+    private final List<File> configurationFiles;
+    private String hexKey;
+    private final Path confDirectory;
+
+    public PropertyEncryptorMain(final Path baseDirectory, final String passphrase) throws PropertyEncryptorException {
+        confDirectory = FileUtilities.resolveAbsoluteConfDirectory(baseDirectory);
+        try {
+            bootstrapLoader = getBootstrapPropertiesLoader(confDirectory);
+            fileResolver = new ConfigurationFileResolver(confDirectory);
+            final File applicationProperties = FileUtilities.resolvePropertiesFile(confDirectory);
+            propertiesLoader = getPropertiesLoader(confDirectory);
+            configurationFiles = fileResolver.resolveConfigurationFilesFromApplicationProperties(propertiesLoader.load(applicationProperties));
+            hexKey = getKeyHex(confDirectory, passphrase);
+        } catch (final Exception e) {
+            throw new PropertyEncryptorException("Failed to run property encryptor", e);
+        }
+    }
+
+    private AbstractBootstrapPropertiesLoader getBootstrapPropertiesLoader(final Path baseDirectory) {
+        if (FileUtilities.isNiFiRegistryConfDirectory(baseDirectory)) {
+            return new NiFiRegistryBootstrapPropertiesLoader();
+        } else if (FileUtilities.isNiFiConfDirectory(baseDirectory)) {
+            return new NiFiBootstrapPropertiesLoader();
+        } else {
+            throw new PropertyEncryptorException("The base directory provided does not contain a recognized bootstrap.conf file");
+        }
+    }
+
+    private PropertiesLoader<ApplicationProperties> getPropertiesLoader(final Path baseDirectory) {
+        if (FileUtilities.isNiFiRegistryConfDirectory(baseDirectory)) {
+            return new NiFiRegistryPropertiesLoader();
+        } else if (FileUtilities.isNiFiConfDirectory(baseDirectory)) {
+            return new NiFiPropertiesLoader();
+        } else {
+            throw new PropertyEncryptorException("The base directory provided does not contain a recognized .properties file");

Review Comment:
   It would be helpful to include the `baseDirectory` in the message.



##########
nifi-toolkit/nifi-property-encryptor-tool/src/main/java/org/apache/nifi/PropertyEncryptorMain.java:
##########
@@ -0,0 +1,173 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi;
+
+import org.apache.nifi.properties.AbstractBootstrapPropertiesLoader;
+import org.apache.nifi.properties.BootstrapProperties;
+import org.apache.nifi.properties.MutableBootstrapProperties;
+import org.apache.nifi.properties.SensitivePropertyProviderFactory;
+import org.apache.nifi.properties.StandardSensitivePropertyProviderFactory;
+import org.apache.nifi.properties.scheme.PropertyProtectionScheme;
+import org.apache.nifi.properties.scheme.ProtectionScheme;
+import org.apache.nifi.registry.properties.util.NiFiRegistryBootstrapPropertiesLoader;
+import org.apache.nifi.serde.PropertiesWriter;
+import org.apache.nifi.util.NiFiBootstrapPropertiesLoader;
+import org.apache.nifi.util.crypto.CryptographyUtils;
+import org.apache.nifi.xml.XmlDecryptor;
+import org.apache.nifi.xml.XmlEncryptor;
+import org.apache.nifi.util.file.ConfigurationFileResolver;
+import org.apache.nifi.util.file.FileUtilities;
+import org.apache.nifi.properties.ApplicationProperties;
+import org.apache.nifi.properties.NiFiPropertiesLoader;
+import org.apache.nifi.properties.PropertiesLoader;
+import org.apache.nifi.registry.properties.NiFiRegistryPropertiesLoader;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.nio.file.Path;
+import java.util.List;
+
+public class PropertyEncryptorMain {
+
+    private static final Logger logger = LoggerFactory.getLogger(PropertyEncryptorMain.class);
+    private final AbstractBootstrapPropertiesLoader bootstrapLoader;
+    private final PropertiesLoader<ApplicationProperties> propertiesLoader;
+    private final ConfigurationFileResolver fileResolver;
+    private final List<File> configurationFiles;
+    private String hexKey;
+    private final Path confDirectory;
+
+    public PropertyEncryptorMain(final Path baseDirectory, final String passphrase) throws PropertyEncryptorException {
+        confDirectory = FileUtilities.resolveAbsoluteConfDirectory(baseDirectory);
+        try {
+            bootstrapLoader = getBootstrapPropertiesLoader(confDirectory);
+            fileResolver = new ConfigurationFileResolver(confDirectory);
+            final File applicationProperties = FileUtilities.resolvePropertiesFile(confDirectory);
+            propertiesLoader = getPropertiesLoader(confDirectory);
+            configurationFiles = fileResolver.resolveConfigurationFilesFromApplicationProperties(propertiesLoader.load(applicationProperties));
+            hexKey = getKeyHex(confDirectory, passphrase);
+        } catch (final Exception e) {
+            throw new PropertyEncryptorException("Failed to run property encryptor", e);
+        }
+    }
+
+    private AbstractBootstrapPropertiesLoader getBootstrapPropertiesLoader(final Path baseDirectory) {
+        if (FileUtilities.isNiFiRegistryConfDirectory(baseDirectory)) {
+            return new NiFiRegistryBootstrapPropertiesLoader();
+        } else if (FileUtilities.isNiFiConfDirectory(baseDirectory)) {
+            return new NiFiBootstrapPropertiesLoader();
+        } else {
+            throw new PropertyEncryptorException("The base directory provided does not contain a recognized bootstrap.conf file");
+        }
+    }
+
+    private PropertiesLoader<ApplicationProperties> getPropertiesLoader(final Path baseDirectory) {
+        if (FileUtilities.isNiFiRegistryConfDirectory(baseDirectory)) {
+            return new NiFiRegistryPropertiesLoader();
+        } else if (FileUtilities.isNiFiConfDirectory(baseDirectory)) {
+            return new NiFiPropertiesLoader();
+        } else {
+            throw new PropertyEncryptorException("The base directory provided does not contain a recognized .properties file");
+        }
+    }
+
+    /**
+     * @param baseDirectory The base directory of a NiFi / NiFi Registry installation that should be encrypted
+     */
+    public int encryptConfigurationFiles(final Path baseDirectory, final PropertyProtectionScheme scheme) {
+        XmlEncryptor encryptor = getXmlEncryptor(scheme);
+        try {
+            encryptConfigurationFiles(configurationFiles, encryptor);
+            outputKeyToBootstrap();
+            logger.info("The Property Encryptor successfully encrypted configuration files in the [{}] directory with the scheme [{}]", baseDirectory, scheme.getPath());
+            return 0;
+        } catch (Exception e) {
+            logger.error("The Property Encryptor failed to encrypt configuration files in the [{}] directory with the scheme [{}]", baseDirectory, scheme.getPath(), e);
+            return 1;
+        }
+    }
+
+    private void outputKeyToBootstrap() throws IOException {
+        final File bootstrapFile = bootstrapLoader.getBootstrapFileWithinConfDirectory(confDirectory);
+        File tempBootstrapFile = FileUtilities.getTemporaryOutputFile("tmp", bootstrapFile);
+        final MutableBootstrapProperties bootstrapProperties = bootstrapLoader.loadMutableBootstrapProperties(bootstrapFile.getPath());
+        bootstrapProperties.setProperty(BootstrapProperties.BootstrapPropertyKey.SENSITIVE_KEY.getKey(), hexKey);
+        PropertiesWriter.writePropertiesFile(new FileInputStream(bootstrapFile), new FileOutputStream(tempBootstrapFile), bootstrapProperties);
+        logger.info("Output the bootstrap key to {}", tempBootstrapFile);
+    }
+
+    private int encryptConfigurationFiles(final List<File> configurationFiles, final XmlEncryptor encryptor) throws IOException {
+        for (final File configurationFile : configurationFiles) {
+            File temp = FileUtilities.getTemporaryOutputFile("tmp", configurationFile);
+            try (InputStream inputStream = new FileInputStream(configurationFile);
+                FileOutputStream outputStream = new FileOutputStream(temp)) {
+                encryptor.encrypt(inputStream, outputStream);
+                logger.info("Successfully encrypted [{}]", configurationFile.getAbsolutePath());
+            } catch (Exception e) {
+                throw new PropertyEncryptorException(String.format("Failed to encrypt configuration file: [%s]", configurationFile.getAbsolutePath()), e);
+            }
+
+            //Files.copy(temp.toPath(), configurationFile.toPath());
+        }
+        return 0;
+    }
+
+    public int migrateConfigurationFiles(final File baseDirectory) {
+        logger.info("Not yet implemented.");
+        return 0;
+    }
+
+    public int encryptFlowDefinition(final File baseDirectory) {
+        logger.info("Not yet implemented.");
+        return 0;
+    }
+
+    private XmlEncryptor getXmlEncryptor(final PropertyProtectionScheme scheme) {
+        final SensitivePropertyProviderFactory providerFactory = StandardSensitivePropertyProviderFactory.withKey(hexKey);
+        return new XmlEncryptor(providerFactory, scheme);
+    }
+
+    private XmlDecryptor getXmlDecryptor(final SensitivePropertyProviderFactory providerFactory, final ProtectionScheme scheme) {
+        return new XmlDecryptor(providerFactory, scheme);
+    }
+
+    private String getKeyHex(final Path confDirectory, final String passphrase) {

Review Comment:
   This is a good opportunity to name this method more clearly than the current tool. Perhaps `getEncodedRootKey`?



##########
nifi-toolkit/nifi-property-encryptor-tool/pom.xml:
##########
@@ -0,0 +1,129 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+  Licensed to the Apache Software Foundation (ASF) under one or more
+  contributor license agreements. See the NOTICE file distributed with
+  this work for additional information regarding copyright ownership.
+  The ASF licenses this file to You under the Apache License, Version 2.0
+  (the "License"); you may not use this file except in compliance with
+  the License. You may obtain a copy of the License at
+  http://www.apache.org/licenses/LICENSE-2.0
+  Unless required by applicable law or agreed to in writing, software
+  distributed under the License is distributed on an "AS IS" BASIS,
+  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+  See the License for the specific language governing permissions and
+  limitations under the License.
+-->
+<project xmlns="http://maven.apache.org/POM/4.0.0"
+         xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+    <parent>
+        <artifactId>nifi</artifactId>
+        <groupId>org.apache.nifi</groupId>
+        <version>1.18.0-SNAPSHOT</version>
+    </parent>
+    <modelVersion>4.0.0</modelVersion>
+
+    <artifactId>nifi-property-encryptor-tool</artifactId>
+
+    <properties>
+        <maven.compiler.source>8</maven.compiler.source>
+        <maven.compiler.target>8</maven.compiler.target>
+    </properties>

Review Comment:
   These properties are not necessary and should be removed.



##########
nifi-toolkit/nifi-property-encryptor-tool/src/main/java/org/apache/nifi/util/crypto/CryptographyUtils.java:
##########
@@ -0,0 +1,75 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.util.crypto;
+
+import org.apache.commons.codec.binary.Hex;
+import org.bouncycastle.crypto.generators.SCrypt;
+
+import javax.crypto.Cipher;
+import java.nio.charset.StandardCharsets;
+import java.security.KeyException;
+import java.security.NoSuchAlgorithmException;
+import java.util.Arrays;
+import java.util.List;
+
+public class CryptographyUtils {
+
+    private static final String NIFI_SCRYPT_SALT = "NIFI_SCRYPT_SALT";
+    private static final int DEFAULT_MIN_PASSPHRASE_LENGTH = 12;
+
+    // Strong parameters as of 12 Aug 2016
+    private static final int SCRYPT_N = (int) Math.pow(2, 16);
+    private static final int SCRYPT_R = 8;
+    private static final int SCRYPT_P = 1;
+
+    public static String deriveKeyFromPassphrase(final String passphrase) throws KeyException, NoSuchAlgorithmException {
+        return deriveKeyFromPassphrase(passphrase, DEFAULT_MIN_PASSPHRASE_LENGTH);
+    }
+
+    public static boolean isUnlimitedStrengthCryptoAvailable() {
+        try {
+            return Cipher.getMaxAllowedKeyLength("AES") > 128;
+        } catch (NoSuchAlgorithmException e) {
+            return false;
+        }
+    }

Review Comment:
   This method is no longer necessary for versions of Java 8 after Update 171, so it can be removed.



##########
nifi-toolkit/nifi-property-encryptor-tool/src/main/java/org/apache/nifi/util/console/ConfigSubcommand.java:
##########
@@ -0,0 +1,50 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.util.console;
+
+import org.apache.nifi.PropertyEncryptorMain;
+import org.apache.nifi.properties.scheme.PropertyProtectionScheme;
+import org.apache.nifi.util.console.utils.BaseCommandParameters;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import picocli.CommandLine;
+
+@CommandLine.Command(name = "config",
+        description = "Operate on application configs",
+        usageHelpWidth=140
+)
+class ConfigSubcommand extends BaseCommandParameters implements Runnable {
+
+    private static final Logger logger = LoggerFactory.getLogger(ConfigSubcommand.class);
+
+    @CommandLine.ParentCommand
+    private DefaultCLIOptions parent;
+
+    @CommandLine.Parameters(description="The encryption scheme to use, from one of the following schemes: [@|bold ${COMPLETION-CANDIDATES}|@]")
+    PropertyProtectionScheme scheme;
+
+    @Override
+    public void run() {
+        final PropertyEncryptorMain propertyEncryptorMain = new PropertyEncryptorMain(baseDirectory, passphrase);

Review Comment:
   The instantiation and invocation of `PropertyEncryptorMain` calls into question the approach. Instead of having that `Main` class, it seems like more of the methods should be implemented in this class, or broken into their component parts.



##########
nifi-toolkit/nifi-property-encryptor-tool/src/main/java/org/apache/nifi/PropertyEncryptorMain.java:
##########
@@ -0,0 +1,173 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi;
+
+import org.apache.nifi.properties.AbstractBootstrapPropertiesLoader;
+import org.apache.nifi.properties.BootstrapProperties;
+import org.apache.nifi.properties.MutableBootstrapProperties;
+import org.apache.nifi.properties.SensitivePropertyProviderFactory;
+import org.apache.nifi.properties.StandardSensitivePropertyProviderFactory;
+import org.apache.nifi.properties.scheme.PropertyProtectionScheme;
+import org.apache.nifi.properties.scheme.ProtectionScheme;
+import org.apache.nifi.registry.properties.util.NiFiRegistryBootstrapPropertiesLoader;
+import org.apache.nifi.serde.PropertiesWriter;
+import org.apache.nifi.util.NiFiBootstrapPropertiesLoader;
+import org.apache.nifi.util.crypto.CryptographyUtils;
+import org.apache.nifi.xml.XmlDecryptor;
+import org.apache.nifi.xml.XmlEncryptor;
+import org.apache.nifi.util.file.ConfigurationFileResolver;
+import org.apache.nifi.util.file.FileUtilities;
+import org.apache.nifi.properties.ApplicationProperties;
+import org.apache.nifi.properties.NiFiPropertiesLoader;
+import org.apache.nifi.properties.PropertiesLoader;
+import org.apache.nifi.registry.properties.NiFiRegistryPropertiesLoader;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.nio.file.Path;
+import java.util.List;
+
+public class PropertyEncryptorMain {

Review Comment:
   As a general convention, recommend naming this `PropertyEncryptorCommand`.



##########
nifi-toolkit/nifi-property-encryptor-tool/src/main/java/org/apache/nifi/util/crypto/CryptographyUtils.java:
##########
@@ -0,0 +1,75 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.util.crypto;
+
+import org.apache.commons.codec.binary.Hex;
+import org.bouncycastle.crypto.generators.SCrypt;
+
+import javax.crypto.Cipher;
+import java.nio.charset.StandardCharsets;
+import java.security.KeyException;
+import java.security.NoSuchAlgorithmException;
+import java.util.Arrays;
+import java.util.List;
+
+public class CryptographyUtils {

Review Comment:
   Rather than reintroducing a generalized utility class, it would be better to define one or more interface for specific purposes. In this case, a method for returning a derived key seems like the right interface contract.



##########
nifi-toolkit/nifi-property-encryptor-tool/src/main/java/org/apache/nifi/PropertyEncryptorMain.java:
##########
@@ -0,0 +1,173 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi;
+
+import org.apache.nifi.properties.AbstractBootstrapPropertiesLoader;
+import org.apache.nifi.properties.BootstrapProperties;
+import org.apache.nifi.properties.MutableBootstrapProperties;
+import org.apache.nifi.properties.SensitivePropertyProviderFactory;
+import org.apache.nifi.properties.StandardSensitivePropertyProviderFactory;
+import org.apache.nifi.properties.scheme.PropertyProtectionScheme;
+import org.apache.nifi.properties.scheme.ProtectionScheme;
+import org.apache.nifi.registry.properties.util.NiFiRegistryBootstrapPropertiesLoader;
+import org.apache.nifi.serde.PropertiesWriter;
+import org.apache.nifi.util.NiFiBootstrapPropertiesLoader;
+import org.apache.nifi.util.crypto.CryptographyUtils;
+import org.apache.nifi.xml.XmlDecryptor;
+import org.apache.nifi.xml.XmlEncryptor;
+import org.apache.nifi.util.file.ConfigurationFileResolver;
+import org.apache.nifi.util.file.FileUtilities;
+import org.apache.nifi.properties.ApplicationProperties;
+import org.apache.nifi.properties.NiFiPropertiesLoader;
+import org.apache.nifi.properties.PropertiesLoader;
+import org.apache.nifi.registry.properties.NiFiRegistryPropertiesLoader;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.nio.file.Path;
+import java.util.List;
+
+public class PropertyEncryptorMain {
+
+    private static final Logger logger = LoggerFactory.getLogger(PropertyEncryptorMain.class);
+    private final AbstractBootstrapPropertiesLoader bootstrapLoader;
+    private final PropertiesLoader<ApplicationProperties> propertiesLoader;
+    private final ConfigurationFileResolver fileResolver;
+    private final List<File> configurationFiles;
+    private String hexKey;
+    private final Path confDirectory;
+
+    public PropertyEncryptorMain(final Path baseDirectory, final String passphrase) throws PropertyEncryptorException {
+        confDirectory = FileUtilities.resolveAbsoluteConfDirectory(baseDirectory);
+        try {
+            bootstrapLoader = getBootstrapPropertiesLoader(confDirectory);
+            fileResolver = new ConfigurationFileResolver(confDirectory);
+            final File applicationProperties = FileUtilities.resolvePropertiesFile(confDirectory);
+            propertiesLoader = getPropertiesLoader(confDirectory);
+            configurationFiles = fileResolver.resolveConfigurationFilesFromApplicationProperties(propertiesLoader.load(applicationProperties));
+            hexKey = getKeyHex(confDirectory, passphrase);
+        } catch (final Exception e) {
+            throw new PropertyEncryptorException("Failed to run property encryptor", e);
+        }
+    }
+
+    private AbstractBootstrapPropertiesLoader getBootstrapPropertiesLoader(final Path baseDirectory) {
+        if (FileUtilities.isNiFiRegistryConfDirectory(baseDirectory)) {
+            return new NiFiRegistryBootstrapPropertiesLoader();
+        } else if (FileUtilities.isNiFiConfDirectory(baseDirectory)) {
+            return new NiFiBootstrapPropertiesLoader();
+        } else {
+            throw new PropertyEncryptorException("The base directory provided does not contain a recognized bootstrap.conf file");
+        }
+    }
+
+    private PropertiesLoader<ApplicationProperties> getPropertiesLoader(final Path baseDirectory) {
+        if (FileUtilities.isNiFiRegistryConfDirectory(baseDirectory)) {
+            return new NiFiRegistryPropertiesLoader();
+        } else if (FileUtilities.isNiFiConfDirectory(baseDirectory)) {
+            return new NiFiPropertiesLoader();
+        } else {
+            throw new PropertyEncryptorException("The base directory provided does not contain a recognized .properties file");
+        }
+    }
+
+    /**
+     * @param baseDirectory The base directory of a NiFi / NiFi Registry installation that should be encrypted
+     */
+    public int encryptConfigurationFiles(final Path baseDirectory, final PropertyProtectionScheme scheme) {

Review Comment:
   Returning an `int` from this method does not provide a very clear indication of the status. Recommend at least using an enum, and incorporating a numeric status if needed.



##########
nifi-toolkit/nifi-property-encryptor-tool/src/main/java/org/apache/nifi/util/crypto/CryptographyUtils.java:
##########
@@ -0,0 +1,75 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.util.crypto;
+
+import org.apache.commons.codec.binary.Hex;
+import org.bouncycastle.crypto.generators.SCrypt;
+
+import javax.crypto.Cipher;
+import java.nio.charset.StandardCharsets;
+import java.security.KeyException;
+import java.security.NoSuchAlgorithmException;
+import java.util.Arrays;
+import java.util.List;
+
+public class CryptographyUtils {
+
+    private static final String NIFI_SCRYPT_SALT = "NIFI_SCRYPT_SALT";
+    private static final int DEFAULT_MIN_PASSPHRASE_LENGTH = 12;
+
+    // Strong parameters as of 12 Aug 2016
+    private static final int SCRYPT_N = (int) Math.pow(2, 16);
+    private static final int SCRYPT_R = 8;
+    private static final int SCRYPT_P = 1;
+
+    public static String deriveKeyFromPassphrase(final String passphrase) throws KeyException, NoSuchAlgorithmException {
+        return deriveKeyFromPassphrase(passphrase, DEFAULT_MIN_PASSPHRASE_LENGTH);
+    }
+
+    public static boolean isUnlimitedStrengthCryptoAvailable() {
+        try {
+            return Cipher.getMaxAllowedKeyLength("AES") > 128;
+        } catch (NoSuchAlgorithmException e) {
+            return false;
+        }
+    }
+
+    private static String deriveKeyFromPassphrase(final String passphrase, final int minPassphraseLength) throws KeyException, NoSuchAlgorithmException {
+        final String trimmedPassphrase = passphrase.trim();
+        if (trimmedPassphrase.length() < minPassphraseLength) {
+            throw new KeyException(String.format("Cannot derive key from empty/short passphrase -- passphrase must be at least %d characters", DEFAULT_MIN_PASSPHRASE_LENGTH));
+        }
+
+        // Generate a 128 bit salt
+        byte[] salt = generateScryptSalt();
+        int keyLengthInBytes = getValidKeyLengths().stream().max(Integer::compare).get() / 8;
+        byte[] derivedKeyBytes = SCrypt.generate(passphrase.getBytes(StandardCharsets.UTF_8), salt, SCRYPT_N, SCRYPT_R, SCRYPT_P, keyLengthInBytes);
+        return Hex.encodeHexString(derivedKeyBytes).toUpperCase();

Review Comment:
   This seems like an opportunity to introduce support for a different algorithm. It is probably unnecessary to support the same level of configuration as currently supported for the Sensitive Properties Algorithm, but perhaps standardizing on PBKDF2 would be better, as long as there is a way to maintain compatibility with current implementations.



##########
nifi-toolkit/nifi-property-encryptor-tool/src/main/java/org/apache/nifi/util/file/ConfigurationFileResolver.java:
##########
@@ -0,0 +1,91 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.util.file;
+
+import org.apache.nifi.properties.ApplicationProperties;
+import org.apache.nifi.registry.properties.NiFiRegistryProperties;
+import org.apache.nifi.util.NiFiProperties;
+
+import java.io.File;
+import java.nio.file.Path;
+import java.util.ArrayList;
+import java.util.List;
+
+/**
+ * Resolve configuration files that need to be encrypted from a given ApplicationProperties
+ */
+public class ConfigurationFileResolver {

Review Comment:
   As noted with other classes, this looks like a good opportunity to define an interface and then provide a standard implementation.



##########
nifi-toolkit/nifi-property-encryptor-tool/src/main/java/org/apache/nifi/PropertyEncryptorMain.java:
##########
@@ -0,0 +1,173 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi;
+
+import org.apache.nifi.properties.AbstractBootstrapPropertiesLoader;
+import org.apache.nifi.properties.BootstrapProperties;
+import org.apache.nifi.properties.MutableBootstrapProperties;
+import org.apache.nifi.properties.SensitivePropertyProviderFactory;
+import org.apache.nifi.properties.StandardSensitivePropertyProviderFactory;
+import org.apache.nifi.properties.scheme.PropertyProtectionScheme;
+import org.apache.nifi.properties.scheme.ProtectionScheme;
+import org.apache.nifi.registry.properties.util.NiFiRegistryBootstrapPropertiesLoader;
+import org.apache.nifi.serde.PropertiesWriter;
+import org.apache.nifi.util.NiFiBootstrapPropertiesLoader;
+import org.apache.nifi.util.crypto.CryptographyUtils;
+import org.apache.nifi.xml.XmlDecryptor;
+import org.apache.nifi.xml.XmlEncryptor;
+import org.apache.nifi.util.file.ConfigurationFileResolver;
+import org.apache.nifi.util.file.FileUtilities;
+import org.apache.nifi.properties.ApplicationProperties;
+import org.apache.nifi.properties.NiFiPropertiesLoader;
+import org.apache.nifi.properties.PropertiesLoader;
+import org.apache.nifi.registry.properties.NiFiRegistryPropertiesLoader;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.nio.file.Path;
+import java.util.List;
+
+public class PropertyEncryptorMain {
+
+    private static final Logger logger = LoggerFactory.getLogger(PropertyEncryptorMain.class);
+    private final AbstractBootstrapPropertiesLoader bootstrapLoader;
+    private final PropertiesLoader<ApplicationProperties> propertiesLoader;
+    private final ConfigurationFileResolver fileResolver;
+    private final List<File> configurationFiles;
+    private String hexKey;
+    private final Path confDirectory;
+
+    public PropertyEncryptorMain(final Path baseDirectory, final String passphrase) throws PropertyEncryptorException {
+        confDirectory = FileUtilities.resolveAbsoluteConfDirectory(baseDirectory);
+        try {
+            bootstrapLoader = getBootstrapPropertiesLoader(confDirectory);
+            fileResolver = new ConfigurationFileResolver(confDirectory);
+            final File applicationProperties = FileUtilities.resolvePropertiesFile(confDirectory);
+            propertiesLoader = getPropertiesLoader(confDirectory);
+            configurationFiles = fileResolver.resolveConfigurationFilesFromApplicationProperties(propertiesLoader.load(applicationProperties));
+            hexKey = getKeyHex(confDirectory, passphrase);
+        } catch (final Exception e) {
+            throw new PropertyEncryptorException("Failed to run property encryptor", e);
+        }
+    }
+
+    private AbstractBootstrapPropertiesLoader getBootstrapPropertiesLoader(final Path baseDirectory) {
+        if (FileUtilities.isNiFiRegistryConfDirectory(baseDirectory)) {
+            return new NiFiRegistryBootstrapPropertiesLoader();
+        } else if (FileUtilities.isNiFiConfDirectory(baseDirectory)) {
+            return new NiFiBootstrapPropertiesLoader();
+        } else {
+            throw new PropertyEncryptorException("The base directory provided does not contain a recognized bootstrap.conf file");
+        }
+    }
+
+    private PropertiesLoader<ApplicationProperties> getPropertiesLoader(final Path baseDirectory) {
+        if (FileUtilities.isNiFiRegistryConfDirectory(baseDirectory)) {
+            return new NiFiRegistryPropertiesLoader();
+        } else if (FileUtilities.isNiFiConfDirectory(baseDirectory)) {
+            return new NiFiPropertiesLoader();
+        } else {
+            throw new PropertyEncryptorException("The base directory provided does not contain a recognized .properties file");
+        }
+    }
+
+    /**
+     * @param baseDirectory The base directory of a NiFi / NiFi Registry installation that should be encrypted
+     */
+    public int encryptConfigurationFiles(final Path baseDirectory, final PropertyProtectionScheme scheme) {
+        XmlEncryptor encryptor = getXmlEncryptor(scheme);
+        try {
+            encryptConfigurationFiles(configurationFiles, encryptor);
+            outputKeyToBootstrap();
+            logger.info("The Property Encryptor successfully encrypted configuration files in the [{}] directory with the scheme [{}]", baseDirectory, scheme.getPath());
+            return 0;
+        } catch (Exception e) {
+            logger.error("The Property Encryptor failed to encrypt configuration files in the [{}] directory with the scheme [{}]", baseDirectory, scheme.getPath(), e);
+            return 1;
+        }
+    }
+
+    private void outputKeyToBootstrap() throws IOException {
+        final File bootstrapFile = bootstrapLoader.getBootstrapFileWithinConfDirectory(confDirectory);
+        File tempBootstrapFile = FileUtilities.getTemporaryOutputFile("tmp", bootstrapFile);
+        final MutableBootstrapProperties bootstrapProperties = bootstrapLoader.loadMutableBootstrapProperties(bootstrapFile.getPath());
+        bootstrapProperties.setProperty(BootstrapProperties.BootstrapPropertyKey.SENSITIVE_KEY.getKey(), hexKey);
+        PropertiesWriter.writePropertiesFile(new FileInputStream(bootstrapFile), new FileOutputStream(tempBootstrapFile), bootstrapProperties);

Review Comment:
   It looks like these streams should be created in a try-with-resources block.



##########
nifi-toolkit/nifi-property-encryptor-tool/src/main/java/org/apache/nifi/xml/XmlCryptoException.java:
##########
@@ -0,0 +1,27 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.xml;
+
+public class XmlCryptoException extends RuntimeException {

Review Comment:
   This seems a bit more specific than necessary, perhaps `CryptoException`?



##########
nifi-toolkit/nifi-property-encryptor-tool/src/main/java/org/apache/nifi/util/file/FileUtilities.java:
##########
@@ -0,0 +1,102 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.util.file;
+
+import java.io.File;
+import java.io.IOException;
+import java.nio.file.Path;
+import java.util.Arrays;
+
+public class FileUtilities {

Review Comment:
   This class name seems a bit too generic. Perhaps ConfigurationFileUtils?



##########
nifi-toolkit/nifi-property-encryptor-tool/src/main/java/org/apache/nifi/xml/XmlCryptoParser.java:
##########
@@ -0,0 +1,163 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.xml;
+
+import org.apache.nifi.properties.SensitivePropertyProvider;
+import org.apache.nifi.properties.SensitivePropertyProviderFactory;
+import org.apache.nifi.properties.scheme.ProtectionScheme;
+
+import javax.xml.namespace.QName;
+import javax.xml.stream.XMLEventFactory;
+import javax.xml.stream.XMLEventReader;
+import javax.xml.stream.XMLEventWriter;
+import javax.xml.stream.XMLInputFactory;
+import javax.xml.stream.XMLOutputFactory;
+import javax.xml.stream.XMLStreamException;
+import javax.xml.stream.events.Attribute;
+import javax.xml.stream.events.Characters;
+import javax.xml.stream.events.StartElement;
+import javax.xml.stream.events.XMLEvent;
+import java.io.FileNotFoundException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.util.ArrayList;
+import java.util.Iterator;
+import java.util.Objects;
+
+public abstract class XmlCryptoParser {
+
+    protected static final String ENCRYPTION_ATTRIBUTE_NAME = "encryption";
+    protected static final String PARENT_IDENTIFIER = "identifier";
+    protected static final String PROPERTY_ELEMENT = "property";
+
+    protected final SensitivePropertyProvider cryptoProvider;
+    protected SensitivePropertyProviderFactory providerFactory;
+
+    public XmlCryptoParser(final SensitivePropertyProviderFactory providerFactory, final ProtectionScheme scheme) {
+        this.providerFactory = providerFactory;
+        cryptoProvider = providerFactory.getProvider(scheme);
+    }
+
+    protected void cryptographicXmlOperation(final InputStream encryptedXmlContent, final OutputStream decryptedOutputStream) {
+        final XMLOutputFactory factory = XMLOutputFactory.newInstance();
+        factory.setProperty("com.ctc.wstx.outputValidateStructure", false);
+
+        try {
+            final XMLEventReader eventReader = getXMLReader(encryptedXmlContent);
+            final XMLEventWriter xmlWriter = factory.createXMLEventWriter(decryptedOutputStream);
+            String groupIdentifier = "";
+
+            while(eventReader.hasNext()) {
+                XMLEvent event = eventReader.nextEvent();
+
+                if (isGroupIdentifier(event)) {
+                    groupIdentifier = getGroupIdentifier(eventReader.nextEvent());
+                }
+
+                if (isSensitiveElement(event)) {
+                    xmlWriter.add(updateStartElementEncryptionAttribute(event));
+                    xmlWriter.add(cryptoOperationOnCharacters(eventReader.nextEvent(), groupIdentifier, getPropertyName(event)));
+                } else {
+                    try {
+                        xmlWriter.add(event);
+                    } catch (Exception e) {
+                        throw new RuntimeException("Failed operation on XML content", e);

Review Comment:
   It would be helpful to throw a more specific type of exception.



##########
nifi-toolkit/nifi-property-encryptor-tool/src/main/java/org/apache/nifi/xml/XmlCryptoParser.java:
##########
@@ -0,0 +1,163 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.xml;
+
+import org.apache.nifi.properties.SensitivePropertyProvider;
+import org.apache.nifi.properties.SensitivePropertyProviderFactory;
+import org.apache.nifi.properties.scheme.ProtectionScheme;
+
+import javax.xml.namespace.QName;
+import javax.xml.stream.XMLEventFactory;
+import javax.xml.stream.XMLEventReader;
+import javax.xml.stream.XMLEventWriter;
+import javax.xml.stream.XMLInputFactory;
+import javax.xml.stream.XMLOutputFactory;
+import javax.xml.stream.XMLStreamException;
+import javax.xml.stream.events.Attribute;
+import javax.xml.stream.events.Characters;
+import javax.xml.stream.events.StartElement;
+import javax.xml.stream.events.XMLEvent;
+import java.io.FileNotFoundException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.util.ArrayList;
+import java.util.Iterator;
+import java.util.Objects;
+
+public abstract class XmlCryptoParser {
+
+    protected static final String ENCRYPTION_ATTRIBUTE_NAME = "encryption";
+    protected static final String PARENT_IDENTIFIER = "identifier";
+    protected static final String PROPERTY_ELEMENT = "property";
+
+    protected final SensitivePropertyProvider cryptoProvider;
+    protected SensitivePropertyProviderFactory providerFactory;
+
+    public XmlCryptoParser(final SensitivePropertyProviderFactory providerFactory, final ProtectionScheme scheme) {
+        this.providerFactory = providerFactory;
+        cryptoProvider = providerFactory.getProvider(scheme);
+    }
+
+    protected void cryptographicXmlOperation(final InputStream encryptedXmlContent, final OutputStream decryptedOutputStream) {
+        final XMLOutputFactory factory = XMLOutputFactory.newInstance();
+        factory.setProperty("com.ctc.wstx.outputValidateStructure", false);

Review Comment:
   This property presumes the use of Woodstox, which is not clearly implied elsewhere. It looks like this should be removed and re-evaluated.



##########
nifi-toolkit/nifi-property-encryptor-tool/src/main/java/org/apache/nifi/xml/XmlCryptoParser.java:
##########
@@ -0,0 +1,163 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.xml;
+
+import org.apache.nifi.properties.SensitivePropertyProvider;
+import org.apache.nifi.properties.SensitivePropertyProviderFactory;
+import org.apache.nifi.properties.scheme.ProtectionScheme;
+
+import javax.xml.namespace.QName;
+import javax.xml.stream.XMLEventFactory;
+import javax.xml.stream.XMLEventReader;
+import javax.xml.stream.XMLEventWriter;
+import javax.xml.stream.XMLInputFactory;
+import javax.xml.stream.XMLOutputFactory;
+import javax.xml.stream.XMLStreamException;
+import javax.xml.stream.events.Attribute;
+import javax.xml.stream.events.Characters;
+import javax.xml.stream.events.StartElement;
+import javax.xml.stream.events.XMLEvent;
+import java.io.FileNotFoundException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.util.ArrayList;
+import java.util.Iterator;
+import java.util.Objects;
+
+public abstract class XmlCryptoParser {
+
+    protected static final String ENCRYPTION_ATTRIBUTE_NAME = "encryption";
+    protected static final String PARENT_IDENTIFIER = "identifier";
+    protected static final String PROPERTY_ELEMENT = "property";
+
+    protected final SensitivePropertyProvider cryptoProvider;
+    protected SensitivePropertyProviderFactory providerFactory;
+
+    public XmlCryptoParser(final SensitivePropertyProviderFactory providerFactory, final ProtectionScheme scheme) {
+        this.providerFactory = providerFactory;
+        cryptoProvider = providerFactory.getProvider(scheme);
+    }
+
+    protected void cryptographicXmlOperation(final InputStream encryptedXmlContent, final OutputStream decryptedOutputStream) {
+        final XMLOutputFactory factory = XMLOutputFactory.newInstance();
+        factory.setProperty("com.ctc.wstx.outputValidateStructure", false);
+
+        try {
+            final XMLEventReader eventReader = getXMLReader(encryptedXmlContent);
+            final XMLEventWriter xmlWriter = factory.createXMLEventWriter(decryptedOutputStream);
+            String groupIdentifier = "";
+
+            while(eventReader.hasNext()) {
+                XMLEvent event = eventReader.nextEvent();
+
+                if (isGroupIdentifier(event)) {
+                    groupIdentifier = getGroupIdentifier(eventReader.nextEvent());
+                }
+
+                if (isSensitiveElement(event)) {
+                    xmlWriter.add(updateStartElementEncryptionAttribute(event));
+                    xmlWriter.add(cryptoOperationOnCharacters(eventReader.nextEvent(), groupIdentifier, getPropertyName(event)));
+                } else {
+                    try {
+                        xmlWriter.add(event);
+                    } catch (Exception e) {
+                        throw new RuntimeException("Failed operation on XML content", e);
+                    }
+                }
+            }
+
+            eventReader.close();
+            xmlWriter.flush();
+            xmlWriter.close();
+        } catch (Exception e) {
+            throw new RuntimeException("Failed operation on XML content", e);
+        }
+    }
+
+    /**
+     * Update the StartElement 'encryption' attribute for a sensitive value to add or remove the respective encryption details eg. encryption="aes/gcm/128"
+     * @param xmlEvent A 'sensitive' StartElement that contains the 'encryption' tag attribute
+     * @return The updated StartElement
+     */
+    protected abstract StartElement updateStartElementEncryptionAttribute(final XMLEvent xmlEvent);
+
+    /**
+     * Perform an encrypt or decrypt cryptographic operation on a Characters element
+     * @param xmlEvent A Characters XmlEvent
+     * @param groupIdentifier The XML <identifier/> tag
+     * @return The Characters XmlEvent that has been updated by the cryptographic operation
+     */
+    protected abstract Characters cryptoOperationOnCharacters(final XMLEvent xmlEvent, final String groupIdentifier, final String propertyName);
+
+    private String getGroupIdentifier(final XMLEvent xmlEvent) {
+        if (xmlEvent.isCharacters()) {
+            return xmlEvent.asCharacters().getData();
+        } else {
+            return "";
+        }
+    }
+
+    protected String getPropertyName(final XMLEvent xmlEvent) {
+        return xmlEvent.asStartElement().getName().toString();
+    }
+
+    protected boolean isGroupIdentifier(final XMLEvent xmlEvent) {
+        return xmlEvent.isStartElement()
+                && xmlEvent.asStartElement().getName().toString().equals(PARENT_IDENTIFIER);
+    }
+
+    protected boolean isSensitiveElement(final XMLEvent xmlEvent) {
+        return  xmlEvent.isStartElement()
+                && xmlEvent.asStartElement().getName().toString().equals(PROPERTY_ELEMENT)
+                && elementHasEncryptionAttribute(xmlEvent.asStartElement());
+    }
+
+    protected XMLEventReader getXMLReader(final InputStream fileStream) throws XMLStreamException, FileNotFoundException {
+        XMLInputFactory xmlInputFactory = XMLInputFactory.newFactory();
+        xmlInputFactory.setProperty(XMLInputFactory.IS_SUPPORTING_EXTERNAL_ENTITIES, false);
+        xmlInputFactory.setProperty(XMLInputFactory.SUPPORT_DTD, false);
+
+        return xmlInputFactory.createXMLEventReader(fileStream);
+    }

Review Comment:
   This should be replaced with usage of XML components from `nifi-xml-processing` to avoid the possibility of external entity parsing.



##########
nifi-toolkit/nifi-property-encryptor-tool/src/test/resources/login-identity-providers-populated-encrypted.xml:
##########
@@ -0,0 +1,102 @@
+<?xml version='1.0' encoding='UTF-8' standalone='yes'?><!--
+  Licensed to the Apache Software Foundation (ASF) under one or more
+  contributor license agreements.  See the NOTICE file distributed with
+  this work for additional information regarding copyright ownership.
+  The ASF licenses this file to You under the Apache License, Version 2.0
+  (the "License"); you may not use this file except in compliance with
+  the License.  You may obtain a copy of the License at
+      http://www.apache.org/licenses/LICENSE-2.0
+  Unless required by applicable law or agreed to in writing, software
+  distributed under the License is distributed on an "AS IS" BASIS,
+  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+  See the License for the specific language governing permissions and
+  limitations under the License.
+--><!--
+    This file lists the login identity providers to use when running securely. In order
+    to use a specific provider it must be configured here and it's identifier
+    must be specified in the nifi.properties file.
+--><loginIdentityProviders>
+    <!--
+        Identity Provider for users logging in with username/password against an LDAP server.
+
+        'Authentication Strategy' - How the connection to the LDAP server is authenticated. Possible
+            values are ANONYMOUS, SIMPLE, or START_TLS.
+
+        'Manager DN' - The DN of the manager that is used to bind to the LDAP server to search for users.
+        'Manager Password' - The password of the manager that is used to bind to the LDAP server to
+            search for users.
+
+        'TLS - Keystore' - Path to the Keystore that is used when connecting to LDAP using START_TLS.
+        'TLS - Keystore Password' - Password for the Keystore that is used when connecting to LDAP
+            using START_TLS.
+        'TLS - Keystore Type' - Type of the Keystore that is used when connecting to LDAP using
+            START_TLS (i.e. JKS or PKCS12).
+        'TLS - Truststore' - Path to the Truststore that is used when connecting to LDAP using START_TLS.
+        'TLS - Truststore Password' - Password for the Truststore that is used when connecting to
+            LDAP using START_TLS.
+        'TLS - Truststore Type' - Type of the Truststore that is used when connecting to LDAP using
+            START_TLS (i.e. JKS or PKCS12).
+        'TLS - Client Auth' - Client authentication policy when connecting to LDAP using START_TLS.
+            Possible values are REQUIRED, WANT, NONE.
+        'TLS - Protocol' - Protocol to use when connecting to LDAP using START_TLS. (i.e. TLS,
+            TLSv1.1, TLSv1.2, etc).
+        'TLS - Shutdown Gracefully' - Specifies whether the TLS should be shut down gracefully
+            before the target context is closed. Defaults to false.
+
+        'Referral Strategy' - Strategy for handling referrals. Possible values are FOLLOW, IGNORE, THROW.
+        'Connect Timeout' - Duration of connect timeout. (i.e. 10 secs).
+        'Read Timeout' - Duration of read timeout. (i.e. 10 secs).
+
+        'Url' - Url of the LDAP servier (i.e. ldap://<hostname>:<port>).
+        'User Search Base' - Base DN for searching for users (i.e. CN=Users,DC=example,DC=com).
+        'User Search Filter' - Filter for searching for users against the 'User Search Base'.
+            (i.e. sAMAccountName={0}). The user specified name is inserted into '{0}'.
+
+        'Authentication Expiration' - The duration of how long the user authentication is valid
+            for. If the user never logs out, they will be required to log back in following
+            this duration.
+    -->

Review Comment:
   The file contents should be reduced to the minimum, so comments like this can be removed.



##########
nifi-toolkit/nifi-property-encryptor-tool/src/main/java/org/apache/nifi/xml/XmlCryptoParser.java:
##########
@@ -0,0 +1,163 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.xml;
+
+import org.apache.nifi.properties.SensitivePropertyProvider;
+import org.apache.nifi.properties.SensitivePropertyProviderFactory;
+import org.apache.nifi.properties.scheme.ProtectionScheme;
+
+import javax.xml.namespace.QName;
+import javax.xml.stream.XMLEventFactory;
+import javax.xml.stream.XMLEventReader;
+import javax.xml.stream.XMLEventWriter;
+import javax.xml.stream.XMLInputFactory;
+import javax.xml.stream.XMLOutputFactory;
+import javax.xml.stream.XMLStreamException;
+import javax.xml.stream.events.Attribute;
+import javax.xml.stream.events.Characters;
+import javax.xml.stream.events.StartElement;
+import javax.xml.stream.events.XMLEvent;
+import java.io.FileNotFoundException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.util.ArrayList;
+import java.util.Iterator;
+import java.util.Objects;
+
+public abstract class XmlCryptoParser {
+
+    protected static final String ENCRYPTION_ATTRIBUTE_NAME = "encryption";
+    protected static final String PARENT_IDENTIFIER = "identifier";
+    protected static final String PROPERTY_ELEMENT = "property";
+
+    protected final SensitivePropertyProvider cryptoProvider;
+    protected SensitivePropertyProviderFactory providerFactory;
+
+    public XmlCryptoParser(final SensitivePropertyProviderFactory providerFactory, final ProtectionScheme scheme) {
+        this.providerFactory = providerFactory;
+        cryptoProvider = providerFactory.getProvider(scheme);
+    }
+
+    protected void cryptographicXmlOperation(final InputStream encryptedXmlContent, final OutputStream decryptedOutputStream) {
+        final XMLOutputFactory factory = XMLOutputFactory.newInstance();
+        factory.setProperty("com.ctc.wstx.outputValidateStructure", false);
+
+        try {
+            final XMLEventReader eventReader = getXMLReader(encryptedXmlContent);
+            final XMLEventWriter xmlWriter = factory.createXMLEventWriter(decryptedOutputStream);
+            String groupIdentifier = "";
+
+            while(eventReader.hasNext()) {
+                XMLEvent event = eventReader.nextEvent();
+
+                if (isGroupIdentifier(event)) {
+                    groupIdentifier = getGroupIdentifier(eventReader.nextEvent());
+                }
+
+                if (isSensitiveElement(event)) {
+                    xmlWriter.add(updateStartElementEncryptionAttribute(event));
+                    xmlWriter.add(cryptoOperationOnCharacters(eventReader.nextEvent(), groupIdentifier, getPropertyName(event)));
+                } else {
+                    try {
+                        xmlWriter.add(event);
+                    } catch (Exception e) {
+                        throw new RuntimeException("Failed operation on XML content", e);
+                    }
+                }
+            }
+
+            eventReader.close();
+            xmlWriter.flush();
+            xmlWriter.close();
+        } catch (Exception e) {
+            throw new RuntimeException("Failed operation on XML content", e);
+        }
+    }
+
+    /**
+     * Update the StartElement 'encryption' attribute for a sensitive value to add or remove the respective encryption details eg. encryption="aes/gcm/128"
+     * @param xmlEvent A 'sensitive' StartElement that contains the 'encryption' tag attribute
+     * @return The updated StartElement
+     */
+    protected abstract StartElement updateStartElementEncryptionAttribute(final XMLEvent xmlEvent);
+
+    /**
+     * Perform an encrypt or decrypt cryptographic operation on a Characters element
+     * @param xmlEvent A Characters XmlEvent
+     * @param groupIdentifier The XML <identifier/> tag
+     * @return The Characters XmlEvent that has been updated by the cryptographic operation
+     */
+    protected abstract Characters cryptoOperationOnCharacters(final XMLEvent xmlEvent, final String groupIdentifier, final String propertyName);
+
+    private String getGroupIdentifier(final XMLEvent xmlEvent) {
+        if (xmlEvent.isCharacters()) {
+            return xmlEvent.asCharacters().getData();
+        } else {
+            return "";
+        }
+    }
+
+    protected String getPropertyName(final XMLEvent xmlEvent) {
+        return xmlEvent.asStartElement().getName().toString();
+    }
+
+    protected boolean isGroupIdentifier(final XMLEvent xmlEvent) {
+        return xmlEvent.isStartElement()
+                && xmlEvent.asStartElement().getName().toString().equals(PARENT_IDENTIFIER);
+    }
+
+    protected boolean isSensitiveElement(final XMLEvent xmlEvent) {
+        return  xmlEvent.isStartElement()
+                && xmlEvent.asStartElement().getName().toString().equals(PROPERTY_ELEMENT)
+                && elementHasEncryptionAttribute(xmlEvent.asStartElement());
+    }
+
+    protected XMLEventReader getXMLReader(final InputStream fileStream) throws XMLStreamException, FileNotFoundException {
+        XMLInputFactory xmlInputFactory = XMLInputFactory.newFactory();
+        xmlInputFactory.setProperty(XMLInputFactory.IS_SUPPORTING_EXTERNAL_ENTITIES, false);
+        xmlInputFactory.setProperty(XMLInputFactory.SUPPORT_DTD, false);
+
+        return xmlInputFactory.createXMLEventReader(fileStream);
+    }
+
+    private boolean elementHasEncryptionAttribute(final StartElement xmlEvent) {
+        return xmlElementHasAttribute(xmlEvent, ENCRYPTION_ATTRIBUTE_NAME);
+    }
+
+    private boolean xmlElementHasAttribute(final StartElement xmlEvent, final String attributeName) {
+        return !Objects.isNull(xmlEvent.getAttributeByName(new QName(attributeName)));
+    }
+
+    protected StartElement updateElementAttribute(final XMLEvent xmlEvent, final String attributeName, final String attributeValue) {

Review Comment:
   The general order of methods should be public, protected, then private.



##########
nifi-toolkit/nifi-property-encryptor-tool/src/test/resources/login-identity-providers-populated-unecrypted.xml:
##########
@@ -0,0 +1,102 @@
+<?xml version='1.0' encoding='UTF-8' standalone='yes'?><!--
+  Licensed to the Apache Software Foundation (ASF) under one or more
+  contributor license agreements.  See the NOTICE file distributed with
+  this work for additional information regarding copyright ownership.
+  The ASF licenses this file to You under the Apache License, Version 2.0
+  (the "License"); you may not use this file except in compliance with
+  the License.  You may obtain a copy of the License at
+      http://www.apache.org/licenses/LICENSE-2.0
+  Unless required by applicable law or agreed to in writing, software
+  distributed under the License is distributed on an "AS IS" BASIS,
+  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+  See the License for the specific language governing permissions and
+  limitations under the License.
+--><!--
+    This file lists the login identity providers to use when running securely. In order
+    to use a specific provider it must be configured here and it's identifier
+    must be specified in the nifi.properties file.
+--><loginIdentityProviders>
+    <!--
+        Identity Provider for users logging in with username/password against an LDAP server.
+
+        'Authentication Strategy' - How the connection to the LDAP server is authenticated. Possible
+            values are ANONYMOUS, SIMPLE, or START_TLS.
+
+        'Manager DN' - The DN of the manager that is used to bind to the LDAP server to search for users.
+        'Manager Password' - The password of the manager that is used to bind to the LDAP server to
+            search for users.
+
+        'TLS - Keystore' - Path to the Keystore that is used when connecting to LDAP using START_TLS.
+        'TLS - Keystore Password' - Password for the Keystore that is used when connecting to LDAP
+            using START_TLS.
+        'TLS - Keystore Type' - Type of the Keystore that is used when connecting to LDAP using
+            START_TLS (i.e. JKS or PKCS12).
+        'TLS - Truststore' - Path to the Truststore that is used when connecting to LDAP using START_TLS.
+        'TLS - Truststore Password' - Password for the Truststore that is used when connecting to
+            LDAP using START_TLS.
+        'TLS - Truststore Type' - Type of the Truststore that is used when connecting to LDAP using
+            START_TLS (i.e. JKS or PKCS12).
+        'TLS - Client Auth' - Client authentication policy when connecting to LDAP using START_TLS.
+            Possible values are REQUIRED, WANT, NONE.
+        'TLS - Protocol' - Protocol to use when connecting to LDAP using START_TLS. (i.e. TLS,
+            TLSv1.1, TLSv1.2, etc).
+        'TLS - Shutdown Gracefully' - Specifies whether the TLS should be shut down gracefully
+            before the target context is closed. Defaults to false.
+
+        'Referral Strategy' - Strategy for handling referrals. Possible values are FOLLOW, IGNORE, THROW.
+        'Connect Timeout' - Duration of connect timeout. (i.e. 10 secs).
+        'Read Timeout' - Duration of read timeout. (i.e. 10 secs).
+
+        'Url' - Url of the LDAP servier (i.e. ldap://<hostname>:<port>).
+        'User Search Base' - Base DN for searching for users (i.e. CN=Users,DC=example,DC=com).
+        'User Search Filter' - Filter for searching for users against the 'User Search Base'.
+            (i.e. sAMAccountName={0}). The user specified name is inserted into '{0}'.
+
+        'Authentication Expiration' - The duration of how long the user authentication is valid
+            for. If the user never logs out, they will be required to log back in following
+            this duration.
+    -->

Review Comment:
   ```suggestion
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org