Implementation of CryptoStore

[morph166955]

trafficstars

Implements (#3289)

Creates the basis for a centralized crypto storage mechanism. Also creates several functions that are used across addon's to simplify addon creation.

Basic usage expectation:

CryptoStore myCryptoStore = new CryptoStore(); Key cryptoStoreKey = CryptoStore.generateEncryptionKey();

cryptoStoreKey is an AES encryption key used to encrypt the private key while in memory. This must be passed to any function which handles the private key.

Option 1) User has private key and certificate to provide (e.g. something provided by a device) myCryptoStore.setPrivKey(String myPrivKey, cryptoStoreKey); myCryptoStore.setCert(String cert); OR .setCert(Certificate cert);

Option 2) User needs a new keypair created. myCryptoStore.generateNewKeyPair(cryptoStoreKey);

Option 3) User already has a java KeyStore stored on disk and imports it. myCryptoStore.loadFromKeyStore(String keystoreFileName, String keystorePassword, cryptoStoreKey);

User can request a KeyStore via getKeyStore(String keystorePassword, cryptoStoreKey);

User can save the CryptoStore to a java KeyStore for persistence via loadFromKeyStore(String keystoreFileName, String keystorePassword, cryptoStoreKey);

Several other supporting functions exist and will be documented prior to merge.

This is the first part of the centralized crypto pieces. Part two will be the creation of a centralized storage location. Centralized crypto will store the private key in it's encrypted format as well as the certificate. It will also store an encrypted copy of the cryptoStoreKey. Both the store and key will be recovered by the add-on as needed for operation.

Users can also call encrypt(String data, Key key) or decrypt(String encryptedData, Key key) if they want to simply encrypt/decrypt strings for use. Key can be the cryptoStoreKey or a separate key as needed.

[openhab-bot]

This pull request has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/openhab-4-0-wishlist/142388/304

[morph166955]

pom.xml conflict should be fixed. This was the addon/binding change.

[morph166955]

I'm down to just a few changes at this point. Code is very stable on the androidtv binding. I need to add a few extra functions to handle CA certs.

[morph166955]

Just so this doesn't go stale, this is pending additional testing once the androidtv binding is reviewed and merged.

[morph166955]

@openhab/core-maintainers This is ready for review. I'm NOT expecting this to make it in for 4.0.0. I highly doubt any binding developer will be able to adapt their binding in the 6 days between now and the code freeze. I see this as a 4.1.0 feature. Also note, this is part 1 of 2. Part 2 will be the centralized crypto storage as discussed in #3289. In theory, CryptoStore replaces and enhances the need for a KeyStore in the bindings (or anywhere else that's relevant). CryptoStore implements several features such as key generation, encryption/decryption of strings, and self signed certificate creation. Code is all based off of the AndroidTV binding implementation which seems to work very well. Once this is locked down I'll work on part 2 with the goal of having both available for 4.1.0.

As a note, I considered adding in some things like TrustManagers to this, but I think that may be better separated out into a separate library. If the consensus is to add it here, I can easily add the functions.

[morph166955]

The builds are failing due to the automation integration tests, not due to any specific issue with this PR. This builds cleanly against 4.1.0 on my local system when just compiling the bundle.

[morph166955]

@openhab/core-maintainers Respectfully requesting a review on this. Code is very stable on AndroidTV binding. I really want to start working on the second part of this to try and get both into 4.1. Thank you!

[openhab-bot]

This pull request has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/io-jetty-certificate-embeds-bouncy-castle-crypto-classes-resulting-in-classloader-error/153020/4

[holgerfriedrich]

@morph166955 I think it would be helpful to integrate bcprov-jdk18on as part of core. I tried to pull in bouncy castle into KNX addon for a proof of concept for TPMs openhab/openhab-addons#15326 - resulting in +5500 class files and +7MB size of my addon jar. The same thing seems to happen with androidtv you mentioned.

[morph166955]

Fully agreed and this PR should do that already. My ultimate goal is to really pull as much crypto as we can into a (very) reviewed and hardened implementation to make sure we don't have security leaks/holes in bindings. bcpkix-jdk18on, bcprov-jdk18on, and bcutil-jdk18on are all added to bom/compile/pom.xml as part of this PR.

In respect to TPM usage, my is to do a second PR which will create a central storage location for crypto (certs, keys, passwords, etc). TPM would definitely be a good addition to that once it works. There was some discussion on this in #3289 but it definitely needs further thought.

[morph166955]

@pacive Thank you for the review! I believe I've replied to all of your notes above. Absolutely happy to keep talking about any if you don't feel the answer is solid. As I mentioned above, the goal here is two fold.

1. To create a central set of functions to deal with crypto so that the binding developers are not required to work that (especially since crypto can be confusing to those who don't work on it). This also helps the overall security posture of the platform by reducing the risk of a bad/improper/mistake in those implementations.

2. To prepare for a key warehouseing function to go into the core. Several of the functions are designed with the concept of shared storage in mind and how to secure the bindings across (e.g. encryption of the keys). The goal would be for the warehouse to hand a CryptoStore and a key to the binding when it asks for it (which would likely be in a recall manner as opposed to a "give me new" which CryptoStore would handle directly).

[holgerfriedrich]

I played around a bit with your PR and wrote a simple test as well. For most of the methods the crypto algorithms seem to be provided by the JDK - not BC. If I explicitly add “BC” to the getter, it does not find provider BC. So I think we should make sure to add the provider BC - as you did for generating keys. This could be done in the CTOR - and maybe using an Activate tag to ensure BC is properly added during start of the OSGI bundle. Not sure if we should play with the precedence or query explicitly for “BC”.

[J-N-K]

We could add a new interface CredentialConsumer which is automatically added to a CredentialSupplierServlet that allows entering credentials on a web-servlet. The CredentialConsumer could provide a description what is needed (a password, a private/public key, ...) and after reception store it in the crypto store. If it is bound to a thing, it could also implement ThingHandlerService which creates an OSGi component and injects the ThingHandler.

[J-N-K]

@morph166955 I'll wait with a review until you discussions with @dbadia are finished. I'm not an expert on the crypto-parts, so it's probably better to discuss those issues first.