NIFI-8511 Added KeyStore implementation of KeyProvider

[exceptionfactory]

Description of PR

NIFI-8511 Adds an encrypted repository KeyProvider implementation using a java.security.KeyStore instance containing javax.crypto.SecretKey entries. The implementation supports PKCS12 or BCFKS keystores.

The Bouncy Castle implementation of PKCS12 does not support SecretKey entries, so KeyStoreUtils includes additional mappings and methods for reading keystore files containing SecretKey entries. Changes also include refactoring the KeyProvider interface and implementations into a separate nifi-security-kms module to streamline and clarify dependencies. The User Guide includes additional sections with example nifi.properties sections to configure the KeyStoreKeyProvider.

In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:

For all changes:

• Is there a JIRA ticket associated with this PR? Is it referenced
  in the commit message?

• Does your PR title start with NIFI-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.

• Has your PR been rebased against the latest commit within the target branch (typically main)?

• Is your initial contribution a single, squashed commit? Additional commits in response to PR reviewer feedback should be made on this branch and pushed to allow change tracking. Do not squash or use --force when pushing to allow for clean monitoring of changes.

For code changes:

• Have you ensured that the full suite of tests is executed via mvn -Pcontrib-check clean install at the root nifi folder?
• Have you written or updated unit tests to verify your changes?
• Have you verified that the full build is successful on JDK 8?
• Have you verified that the full build is successful on JDK 11?
• If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under ASF 2.0?
• If applicable, have you updated the LICENSE file, including the main LICENSE file under nifi-assembly?
• If applicable, have you updated the NOTICE file, including the main NOTICE file found under nifi-assembly?
• If adding new Properties, have you added .displayName in addition to .name (programmatic access) for each of the new properties?

For documentation related changes:

• Have you ensured that format looks appropriate for the output in which it is rendered?

Note:

Please ensure that once the PR is submitted, you check GitHub Actions CI for build issues and submit an update to your PR as soon as possible.

[Lehel44]

I've got an idea to avoid downcasting and branching here. The KeyProviderConfiguration interface could contain a new method:
KeyProvider getKeyProvider(final KeyProviderCreator keyProviderCreator);

A key provider creator class could handle creating the different providers based on different configurations:

public class KeyProviderCreator {

private static final String SECRET_KEY_ALGORITHM = "AES";

public KeyProvider getKeyProvider(final StaticKeyProviderConfiguration configuration) {
    final Map<String, SecretKey> secretKeys;
    try {
        secretKeys = getSecretKeys(configuration.getKeys());
        return new StaticKeyProvider(secretKeys);
    } catch (final DecoderException e) {
        throw new KeyReaderException("Decoding Hexadecimal Secret Keys failed", e);
    }
}

public KeyProvider getKeyProvider(final FileBasedKeyProviderConfiguration configuration) {
    final Path keyProviderPath = Paths.get(configuration.getLocation());
    return new FileBasedKeyProvider(keyProviderPath, configuration.getRootKey());
}

public KeyProvider getKeyProvider(final KeyStoreKeyProviderConfiguration configuration) {
    final KeyStore keyStore = configuration.getKeyStore();
    return new KeyStoreKeyProvider(keyStore, configuration.getKeyPassword());
}

private Map<String, SecretKey> getSecretKeys(final Map<String, String> keys) throws DecoderException {
    final Map<String, SecretKey> secretKeys = new HashMap<>();

    for (final Map.Entry<String, String> keyEntry : keys.entrySet()) {
        final byte[] encodedSecretKey = Hex.decodeHex(keyEntry.getValue());
        final SecretKey secretKey = new SecretKeySpec(encodedSecretKey, SECRET_KEY_ALGORITHM);
        secretKeys.put(keyEntry.getKey(), secretKey);
    }

    return secretKeys;
}

}`

The configuration classes can utilize the creator class to make providers:

@Override public KeyProvider getKeyProvider(KeyProviderCreator keyProviderCreator) { return keyProviderCreator.getKeyProvider(this); }

And then in the factory, there's no branching remaining:

`public class KeyProviderFactory {

private static final KeyProviderCreator creator = new KeyProviderCreator();

public static KeyProvider getKeyProvider(final KeyProviderConfiguration<?> configuration) {
    return configuration.getKeyProvider(creator);
}

}`

What do you think of this approach? It adds a bit of extra complexity to other classes but withdraws some from the factory. Also, this way if anyone adds a new configuration, they will be obliged to implement the provider method.

[exceptionfactory]

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.

[exceptionfactory]

Thanks for the helpful reviews and feedback @Lehel44 and @greyp9! I pushed an update incorporating changes.

[exceptionfactory]

Rebased to resolve merge conflicts.

[exceptionfactory]

@greyp9 and @Lehel44, do you have any additional feedback following recent updates?

[thenatog]

Tested this out with a pkcs12 keystore containing a secret key (keytool -genseckey -alias primary-key -keyalg AES -keysize 256 -keystore repository.p12 -storetype PKCS12) and setting it to be used in nifi.properties:

nifi.provenance.repository.implementation=org.apache.nifi.provenance.EncryptedWriteAheadProvenanceRepository nifi.provenance.repository.encryption.key.provider.implementation=org.apache.nifi.security.kms.KeyStoreKeyProvider nifi.provenance.repository.encryption.key.provider.location=./conf/repository.p12 nifi.provenance.repository.encryption.key.provider.password=password nifi.provenance.repository.encryption.key.id=primary-key

and verified that the data is encrypted in the provenance repo data in ./provenance_repository. Thought there was an issue with querying which turned out to be an authZ issue. I also ran into the below exception when opening the provenance UI:

2021-07-09 00:02:10,594 ERROR [Provenance Repository Maintenance-1] o.a.n.p.index.lucene.LuceneEventIndex Failed to perform background maintenance procedures
java.lang.ClassCastException: org.apache.nifi.provenance.EventIdFirstSchemaRecordReader cannot be cast to org.apache.nifi.provenance.EncryptedSchemaRecordReader
at org.apache.nifi.provenance.EncryptedWriteAheadProvenanceRepository.lambda$initialize$1(EncryptedWriteAheadProvenanceRepository.java:115)
at org.apache.nifi.provenance.store.iterator.SequentialRecordReaderEventIterator.rotateReader(SequentialRecordReaderEventIterator.java:109)
at org.apache.nifi.provenance.store.iterator.SequentialRecordReaderEventIterator.nextEvent(SequentialRecordReaderEventIterator.java:65)
at org.apache.nifi.provenance.store.iterator.AuthorizingEventIterator.nextEvent(AuthorizingEventIterator.java:47)
at org.apache.nifi.provenance.store.PartitionedEventStore.getEvents(PartitionedEventStore.java:193)
at org.apache.nifi.provenance.store.PartitionedEventStore.getEvents(PartitionedEventStore.java:159)
at org.apache.nifi.provenance.store.PartitionedEventStore.getEvents(PartitionedEventStore.java:149)
at org.apache.nifi.provenance.index.lucene.LuceneEventIndex.performMaintenance(LuceneEventIndex.java:824)
at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
at java.util.concurrent.FutureTask.runAndReset(FutureTask.java:308)
at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.access$301(ScheduledThreadPoolExecutor.java:180)
at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:294)
at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
at java.lang.Thread.run(Thread.java:748)

which was a result of having an existing unencrypted provenance database. I stopped NiFi, deleted ./provenance_repository/* and started back up and provenance was working once I fixed the authz issue.

+1, will merge.