[GitHub] [nifi] thenatog commented on a diff in pull request #6273: NIFI-9953 - The config encryption tool is too complicated to use and can be simplified
thenatog commented on code in PR #6273:
URL: https://github.com/apache/nifi/pull/6273#discussion_r1033694205
##
nifi-toolkit/nifi-property-encryptor-tool/src/main/java/org/apache/nifi/util/console/utils/SchemeCandidates.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.util.console.utils;
+
+import org.apache.nifi.properties.scheme.StandardProtectionSchemeResolver;
+
+import java.util.ArrayList;
+
+public class SchemeCandidates extends ArrayList {
+SchemeCandidates() {
+super(new
StandardProtectionSchemeResolver().getSupportedProtectionSchemes());
Review Comment:
Is there an existing way to enumerate all the possible ProtectionScheme
paths available at run time? This SchemeCandidates class is used to present all
available options to the user in the CLI and does validation checking on the
user selection of the scheme
--
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-unsubscr...@nifi.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [nifi] thenatog commented on a diff in pull request #6273: NIFI-9953 - The config encryption tool is too complicated to use and can be simplified
thenatog commented on code in PR #6273:
URL: https://github.com/apache/nifi/pull/6273#discussion_r967495476
##
nifi-toolkit/nifi-property-encryptor-tool/src/main/java/org/apache/nifi/serde/StandardPropertiesWriter.java:
##
@@ -0,0 +1,52 @@
+/*
+ * 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 StandardPropertiesWriter implements PropertiesWriter {
+
+private static final String DELIMITER = "=";
+private static final String PROPERTY_FORMAT = "%s=%s";
+
+public void writePropertiesFile(final InputStream inputStream, final
OutputStream outputStream, final ReadableProperties properties) throws
IOException {
+try (BufferedReader reader = new BufferedReader(new
InputStreamReader(inputStream));
+ BufferedWriter writer = new BufferedWriter(new
OutputStreamWriter(outputStream))) {
+String line;
+while ((line = reader.readLine()) != null) {
+Set keys = properties.getPropertyKeys();
+for (final String key : keys) {
+if (line.split(DELIMITER)[0].matches(key)) {
Review Comment:
Updated this, not sure if I did this quite the way you described, let me
know if it should change
--
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-unsubscr...@nifi.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [nifi] thenatog commented on a diff in pull request #6273: NIFI-9953 - The config encryption tool is too complicated to use and can be simplified
thenatog commented on code in PR #6273:
URL: https://github.com/apache/nifi/pull/6273#discussion_r960075378
##
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:
I added a SchemeCandidates class which utilises the
StandardProtectionSchemeResolver instead of directly using the
PropertyProtectionScheme enum. Does this resolve the issue at hand? There may
still be future work to allow providing any extra required parameters to handle
these other schemes.
--
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-unsubscr...@nifi.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [nifi] thenatog commented on a diff in pull request #6273: NIFI-9953 - The config encryption tool is too complicated to use and can be simplified
thenatog commented on code in PR #6273:
URL: https://github.com/apache/nifi/pull/6273#discussion_r960075378
##
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:
I added a SchemeCandidates class which utilises the
StandardProtectionSchemeResolver instead of directly using the
PropertyProtectionScheme enum. Does this resolve the issue at hand?
--
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-unsubscr...@nifi.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [nifi] thenatog commented on a diff in pull request #6273: NIFI-9953 - The config encryption tool is too complicated to use and can be simplified
thenatog commented on code in PR #6273:
URL: https://github.com/apache/nifi/pull/6273#discussion_r959786011
##
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:
I'm wondering if for this PR we might only support AES and worry about other
schemes later?
--
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-unsubscr...@nifi.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [nifi] thenatog commented on a diff in pull request #6273: NIFI-9953 - The config encryption tool is too complicated to use and can be simplified
thenatog commented on code in PR #6273:
URL: https://github.com/apache/nifi/pull/6273#discussion_r959785072
##
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:
There was some reason I set this, and it looked like woodstox was being used
which led to this property. I'll re-check why and if it can be removed.
--
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-unsubscr...@nifi.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [nifi] thenatog commented on a diff in pull request #6273: NIFI-9953 - The config encryption tool is too complicated to use and can be simplified
thenatog commented on code in PR #6273:
URL: https://github.com/apache/nifi/pull/6273#discussion_r959741034
##
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:
What would be a good way to handle some of the common methods across the
different commands like retrieving/storing keys, getting the properties loader
and configuration file resolver etc?
--
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-unsubscr...@nifi.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [nifi] thenatog commented on a diff in pull request #6273: NIFI-9953 - The config encryption tool is too complicated to use and can be simplified
thenatog commented on code in PR #6273:
URL: https://github.com/apache/nifi/pull/6273#discussion_r955317722
##
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:
Does this mean an interface that the XmlCryptoParser would implement, or an
interface that the XmlDecryptor/XmlEncryptor classes would implement? My main
goal here was to reuse as much code as possible as initially I had very similar
code between an encryptor and decryptor class which was unnecessary.
--
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-unsubscr...@nifi.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [nifi] thenatog commented on a diff in pull request #6273: NIFI-9953 - The config encryption tool is too complicated to use and can be simplified
thenatog commented on code in PR #6273:
URL: https://github.com/apache/nifi/pull/6273#discussion_r953140358
##
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 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);
