exceptionfactory commented on a change in pull request #5110:
URL: https://github.com/apache/nifi/pull/5110#discussion_r644440450
##########
File path: nifi-docs/src/main/asciidoc/user-guide.adoc
##########
@@ -2922,6 +2922,34 @@
key5=c6FzfnKm7UR7xqI2NFpZ+fEKBfSU7+1NvRw+XWQ9U39MONWqk5gvoyOCdFR1kUgeg46jrN5dGXk
Each line defines a key ID and then the Base64-encoded cipher text of a 16
byte IV and wrapped AES-128, AES-192, or AES-256 key depending on the JCE
policies available. The individual keys are wrapped by AES/GCM encryption using
the **root key** defined by `nifi.bootstrap.sensitive.key` in
_conf/bootstrap.conf_.
+===== KeyStoreKeyProvider
+The `KeyStoreKeyProvider` implementation reads from an encrypted keystore
using the configured password to load AES Secret Key entries.
+
+The provider supports the following Keystore Types:
+
+* BCFKS
+* PKCS12
Review comment:
JKS does not support storage of Secret Key entries, so attempting to run
`keytool -genseckey -storetype JKS` throws a KeyStoreException. PKCS12 is also
the default KeyStore Type starting in Java 9.
##########
File path:
nifi-commons/nifi-security-kms/src/main/java/org/apache/nifi/security/kms/KeyProviderFactory.java
##########
@@ -0,0 +1,85 @@
+/*
+ * 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.security.kms;
+
+import org.apache.commons.codec.DecoderException;
+import
org.apache.nifi.security.kms.configuration.FileBasedKeyProviderConfiguration;
+import org.apache.nifi.security.kms.configuration.KeyProviderConfiguration;
+import
org.apache.nifi.security.kms.configuration.KeyStoreKeyProviderConfiguration;
+import
org.apache.nifi.security.kms.configuration.StaticKeyProviderConfiguration;
+import org.apache.commons.codec.binary.Hex;
+import org.apache.nifi.security.kms.reader.KeyReaderException;
+
+import javax.crypto.SecretKey;
+import javax.crypto.spec.SecretKeySpec;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.security.KeyStore;
+import java.util.HashMap;
+import java.util.Map;
+
+/**
+ * Key Provider Factory
+ */
+public class KeyProviderFactory {
+ private static final String SECRET_KEY_ALGORITHM = "AES";
Review comment:
That's a good question, unfortunately `nifi-security-kms` is probably
not the right place to place a shared reference. Perhaps
`nifi-security-utils-api` would be a good option. Thanks for the suggestion,
it seems best suited to a follow-on PR.
##########
File path:
nifi-commons/nifi-security-kms/src/main/java/org/apache/nifi/security/kms/reader/StandardFileBasedKeyReader.java
##########
@@ -0,0 +1,115 @@
+/*
+ * 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.security.kms.reader;
+
+import javax.crypto.BadPaddingException;
+import javax.crypto.Cipher;
+import javax.crypto.IllegalBlockSizeException;
+import javax.crypto.NoSuchPaddingException;
+import javax.crypto.SecretKey;
+import javax.crypto.spec.GCMParameterSpec;
+import javax.crypto.spec.SecretKeySpec;
+import java.io.FileInputStream;
+import java.io.IOException;
+import java.nio.file.Path;
+import java.security.InvalidAlgorithmParameterException;
+import java.security.InvalidKeyException;
+import java.security.NoSuchAlgorithmException;
+import java.util.Arrays;
+import java.util.Base64;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Properties;
+
+/**
+ * Standard File Based Key Reader reads Secret Keys from Properties files
encrypted using AES-GCM with Tag Size of 128
+ */
+public class StandardFileBasedKeyReader implements FileBasedKeyReader {
+ protected static final String CIPHER_ALGORITHM = "AES/GCM/NoPadding";
+
+ protected static final int IV_LENGTH = 16;
+
+ protected static final int TAG_SIZE = 128;
+
+ private static final Base64.Decoder DECODER = Base64.getDecoder();
+
+ private static final String SECRET_KEY_ALGORITHM = "AES";
+
+ /**
+ * Read Secret Keys using provided Root Secret Key
+ *
+ * @param path File Path contains a properties file with Key Identifier
and Base64-encoded encrypted values
+ * @param rootKey Root Secret Key
+ * @return Map of Key Identifier to decrypted Secret Key
+ */
+ @Override
+ public Map<String, SecretKey> readSecretKeys(final Path path, final
SecretKey rootKey) {
+ Objects.requireNonNull(path, "Path required");
+ Objects.requireNonNull(rootKey, "Root Key required");
+ final Map<String, SecretKey> secretKeys = new HashMap<>();
+
+ final Properties properties = getProperties(path);
+ for (final String keyId : properties.stringPropertyNames()) {
+ final String encodedProperty = properties.getProperty(keyId);
+ final SecretKey secretKey = readSecretKey(keyId, encodedProperty,
rootKey);
+ secretKeys.put(keyId, secretKey);
+ }
+ return secretKeys;
+ }
+
+ private Properties getProperties(final Path path) {
+ final Properties properties = new Properties();
+ try (final FileInputStream inputStream = new
FileInputStream(path.toFile())) {
+ properties.load(inputStream);
+ } catch (final IOException e) {
+ throw new KeyReaderException(String.format("Reading Secret Keys
Failed [%s]", path), e);
+ }
+ return properties;
+ }
+
+ private SecretKey readSecretKey(final String keyId, final String
encodedProperty, final SecretKey rootKey) {
Review comment:
This current effort refactored existing `KeyProvider` implementations to
streamline the approach for `KeyStoreKeyProvider`, so the general purpose is
reading keys as opposed to writing. However, given that the
`FileBasedKeyProvider` uses a very specific format, it probably makes sense to
implement a writer in this same module to help maintain alignment between
reading and writing.
##########
File path:
nifi-commons/nifi-security-kms/src/main/java/org/apache/nifi/security/kms/KeyProviderFactory.java
##########
@@ -0,0 +1,85 @@
+/*
+ * 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.security.kms;
+
+import org.apache.commons.codec.DecoderException;
+import
org.apache.nifi.security.kms.configuration.FileBasedKeyProviderConfiguration;
+import org.apache.nifi.security.kms.configuration.KeyProviderConfiguration;
+import
org.apache.nifi.security.kms.configuration.KeyStoreKeyProviderConfiguration;
+import
org.apache.nifi.security.kms.configuration.StaticKeyProviderConfiguration;
+import org.apache.commons.codec.binary.Hex;
+import org.apache.nifi.security.kms.reader.KeyReaderException;
+
+import javax.crypto.SecretKey;
+import javax.crypto.spec.SecretKeySpec;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.security.KeyStore;
+import java.util.HashMap;
+import java.util.Map;
+
+/**
+ * Key Provider Factory
+ */
+public class KeyProviderFactory {
+ private static final String SECRET_KEY_ALGORITHM = "AES";
+
+ /**
+ * Get Key Provider based on Configuration
+ *
+ * @param configuration Key Provider Configuration
+ * @return Key Provider
+ */
+ public static KeyProvider getKeyProvider(final KeyProviderConfiguration<?>
configuration) {
Review comment:
Thanks for the suggestion, that is an interesting idea. The purpose of
the `KeyProviderFactory` is to abstract the details of instantiating a
`KeyProvider`, so going in the direction of `KeyProviderCreator` seems to
create another level of abstraction. The basic purpose of the
`KeyProviderConfiguration` classes is to describe the properties necessary to
create the associated `KeyProvider`. A different approach could be to have one
`KeyProviderConfiguration` with all possible properties, but it would still
require branching to determine the required properties. There are probably
other options for streamlining and abstracting `KeyProvider` creation, but the
current implementation is a variation on the previous approach.
##########
File path:
nifi-commons/nifi-security-kms/src/test/java/org/apache/nifi/security/kms/util/SecretKeyUtils.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.security.kms.util;
+
+import javax.crypto.Cipher;
+import javax.crypto.SecretKey;
+import javax.crypto.spec.GCMParameterSpec;
+import javax.crypto.spec.SecretKeySpec;
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.UncheckedIOException;
+import java.security.GeneralSecurityException;
+import java.security.SecureRandom;
+import java.util.Base64;
+import java.util.Map;
+import java.util.Properties;
+
+public class SecretKeyUtils {
+ private static final SecureRandom SECURE_RANDOM = new SecureRandom();
Review comment:
This class is only used for unit tests, so it has a short lifespan.
##########
File path:
nifi-commons/nifi-security-kms/src/test/java/org/apache/nifi/security/kms/util/SecretKeyUtils.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.security.kms.util;
+
+import javax.crypto.Cipher;
+import javax.crypto.SecretKey;
+import javax.crypto.spec.GCMParameterSpec;
+import javax.crypto.spec.SecretKeySpec;
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.UncheckedIOException;
+import java.security.GeneralSecurityException;
+import java.security.SecureRandom;
+import java.util.Base64;
+import java.util.Map;
+import java.util.Properties;
+
+public class SecretKeyUtils {
+ private static final SecureRandom SECURE_RANDOM = new SecureRandom();
+
+ private static final Base64.Encoder ENCODER = Base64.getEncoder();
+
+ private static final String CIPHER_ALGORITHM = "AES/GCM/NoPadding";
+
+ private static final String KEY_ALGORITHM = "AES";
+
+ private static final int KEY_LENGTH = 32;
+
+ private static final int IV_LENGTH = 16;
+
+ private static final int TAG_LENGTH = 128;
+
+ /**
+ * Get Encrypted Secret Keys as Properties
+ *
+ * @param rootKey Root Key used to encrypt Secret Keys
+ * @param secretKeys Map of Key Identifier to Secret Key
+ * @return Properties containing encrypted Secret Keys
+ * @throws GeneralSecurityException Thrown on getEncryptedSecretKey()
+ */
+ public static Properties getEncryptedSecretKeys(final SecretKey rootKey,
final Map<String, SecretKey> secretKeys) throws GeneralSecurityException {
+ final Properties properties = new Properties();
+ for (final Map.Entry<String, SecretKey> secretKeyEntry :
secretKeys.entrySet()) {
+ final SecretKey secretKey = secretKeyEntry.getValue();
+ final String encryptedSecretKey = getEncryptedSecretKey(rootKey,
secretKey);
+ properties.setProperty(secretKeyEntry.getKey(),
encryptedSecretKey);
+ }
+ return properties;
+ }
+
+ /**
+ * Get Random AES Secret Key
+ *
+ * @return Secret Key
+ */
+ public static SecretKey getSecretKey() {
+ final byte[] encodedKey = new byte[KEY_LENGTH];
+ SECURE_RANDOM.nextBytes(encodedKey);
+ return new SecretKeySpec(encodedKey, KEY_ALGORITHM);
+ }
+
+ /**
+ * Get Encrypted Secret Key using AES-GCM with Base64 encoded string
prefixed with initialization vector
+ *
+ * @param rootKey Root Key used to encrypt Secret Key
+ * @param secretKey Secret Key to be encrypted
+ * @return Base64 encoded and encrypted Secret Key
+ * @throws GeneralSecurityException Thrown when unable to encrypt Secret
Key
+ */
+ private static String getEncryptedSecretKey(final SecretKey rootKey, final
SecretKey secretKey) throws GeneralSecurityException {
+ final Cipher cipher = Cipher.getInstance(CIPHER_ALGORITHM);
+
+ final byte[] initializationVector = new byte[IV_LENGTH];
+ SECURE_RANDOM.nextBytes(initializationVector);
+ cipher.init(Cipher.ENCRYPT_MODE, rootKey, new
GCMParameterSpec(TAG_LENGTH, initializationVector));
+ final byte[] encryptedSecretKey =
cipher.doFinal(secretKey.getEncoded());
+
+ final ByteArrayOutputStream outputStream = new ByteArrayOutputStream();
+ try {
+ outputStream.write(initializationVector);
+ outputStream.write(encryptedSecretKey);
+ } catch (final IOException e) {
+ throw new UncheckedIOException(e);
Review comment:
The OutputStream is a ByteArrayOutputStream, so it is highly unlikely to
throw an IOException. In addition to this class being used only in unit tests,
the UncheckedIOException seemed like the best approach.
##########
File path:
nifi-commons/nifi-security-utils/src/main/java/org/apache/nifi/security/util/KeyStoreUtils.java
##########
@@ -143,6 +169,27 @@ public static KeyStore loadKeyStore(String keystorePath,
char[] keystorePassword
}
}
+ /**
+ * Load {@link KeyStore} containing Secret Key entries using configured
Security Provider
+ *
+ * @param keystorePath File path to KeyStore
+ * @param keystorePassword Password for loading KeyStore
+ * @param keystoreTypeName Keystore Type Name
+ * @return KeyStore loaded using specified configuration
+ * @throws TlsException Thrown when unable to load KeyStore or unsupported
Keystore Type
+ */
+ public static KeyStore loadSecretKeyStore(final String keystorePath, final
char[] keystorePassword, final String keystoreTypeName) throws TlsException {
+ try {
+ final KeyStore keyStore = getSecretKeyStore(keystoreTypeName);
+ try (final InputStream keyStoreStream = new
FileInputStream(keystorePath)) {
+ keyStore.load(keyStoreStream, keystorePassword);
+ }
+ return keyStore;
+ } catch (final GeneralSecurityException|IOException e) {
+ throw new TlsException(String.format("Loading Secret Keystore [%s]
Type [%s] Failed", keystorePath, keystoreTypeName), e);
Review comment:
Although `TlsException` is probably not the best name, it follows the
pattern of other methods in KeyStoreUtils. Since `KeyStoreException` is a
subclass of `GeneralSecurityException`, it could also be somewhat confusing.
Refactoring the checked exceptions from multiple KeyStoreUtils methods to use a
different name is probably worth doing separately.
--
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.
For queries about this service, please contact Infrastructure at:
[email protected]
