[GitHub] milleruntime commented on a change in pull request #560: Provide new Crypto interface & impl
milleruntime commented on a change in pull request #560: Provide new Crypto interface & impl URL: https://github.com/apache/accumulo/pull/560#discussion_r208028977 ## File path: core/src/main/java/org/apache/accumulo/core/security/crypto/CryptoServiceFactory.java ## @@ -16,30 +16,30 @@ */ package org.apache.accumulo.core.security.crypto; +import java.util.concurrent.atomic.AtomicReference; + import org.apache.accumulo.core.conf.AccumuloConfiguration; import org.apache.accumulo.core.conf.Property; import org.apache.accumulo.core.spi.crypto.CryptoService; import org.apache.accumulo.core.spi.crypto.CryptoService.CryptoException; import org.apache.accumulo.start.classloader.vfs.AccumuloVFSClassLoader; public class CryptoServiceFactory { - private static CryptoService singleton = null; + private static AtomicReference singleton = new AtomicReference<>(); /** * Load the singleton class configured in {@link Property#INSTANCE_CRYPTO_SERVICE} */ public static CryptoService getConfigured(AccumuloConfiguration conf) { +CryptoService cyptoService = singleton.get(); String configuredClass = conf.get(Property.INSTANCE_CRYPTO_SERVICE.getKey()); -if (singleton == null) { - singleton = loadCryptoService(configuredClass); - singleton.init(conf.getAllPropertiesWithPrefix(Property.INSTANCE_CRYPTO_PREFIX)); -} else { - if (!singleton.getClass().getName().equals(configuredClass)) { -singleton = loadCryptoService(configuredClass); - singleton.init(conf.getAllPropertiesWithPrefix(Property.INSTANCE_CRYPTO_PREFIX)); - } +if (cyptoService == null || !cyptoService.getClass().getName().equals(configuredClass)) { + CryptoService newCryptoService = loadCryptoService(configuredClass); + newCryptoService.init(conf.getAllPropertiesWithPrefix(Property.INSTANCE_CRYPTO_PREFIX)); + singleton.compareAndSet(cyptoService, newCryptoService); + return singleton.get(); Review comment: This still doesn't prevent concurrency issues though does it? There is still the possibility of a race condition where CryptoService.init() gets called twice. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] milleruntime commented on a change in pull request #560: Provide new Crypto interface & impl
milleruntime commented on a change in pull request #560: Provide new Crypto interface & impl URL: https://github.com/apache/accumulo/pull/560#discussion_r208013492 ## File path: core/src/main/java/org/apache/accumulo/core/security/crypto/CryptoServiceFactory.java ## @@ -16,30 +16,30 @@ */ package org.apache.accumulo.core.security.crypto; +import java.util.concurrent.atomic.AtomicReference; + import org.apache.accumulo.core.conf.AccumuloConfiguration; import org.apache.accumulo.core.conf.Property; import org.apache.accumulo.core.spi.crypto.CryptoService; import org.apache.accumulo.core.spi.crypto.CryptoService.CryptoException; import org.apache.accumulo.start.classloader.vfs.AccumuloVFSClassLoader; public class CryptoServiceFactory { - private static CryptoService singleton = null; + private static AtomicReference singleton = new AtomicReference<>(); /** * Load the singleton class configured in {@link Property#INSTANCE_CRYPTO_SERVICE} */ public static CryptoService getConfigured(AccumuloConfiguration conf) { +CryptoService cyptoService = singleton.get(); String configuredClass = conf.get(Property.INSTANCE_CRYPTO_SERVICE.getKey()); -if (singleton == null) { - singleton = loadCryptoService(configuredClass); - singleton.init(conf.getAllPropertiesWithPrefix(Property.INSTANCE_CRYPTO_PREFIX)); -} else { - if (!singleton.getClass().getName().equals(configuredClass)) { -singleton = loadCryptoService(configuredClass); - singleton.init(conf.getAllPropertiesWithPrefix(Property.INSTANCE_CRYPTO_PREFIX)); - } +if (cyptoService == null || !cyptoService.getClass().getName().equals(configuredClass)) { + CryptoService newCryptoService = loadCryptoService(configuredClass); + newCryptoService.init(conf.getAllPropertiesWithPrefix(Property.INSTANCE_CRYPTO_PREFIX)); + singleton.compareAndSet(cyptoService, newCryptoService); + return singleton.get(); Review comment: Wondering if I should check the compareAndSet result. And if so what should I do if it fails? Is it worth logging a warning? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] milleruntime commented on a change in pull request #560: Provide new Crypto interface & impl
milleruntime commented on a change in pull request #560: Provide new Crypto interface & impl URL: https://github.com/apache/accumulo/pull/560#discussion_r208011274 ## File path: core/src/main/java/org/apache/accumulo/core/security/crypto/CryptoServiceFactory.java ## @@ -16,30 +16,30 @@ */ package org.apache.accumulo.core.security.crypto; +import java.util.concurrent.atomic.AtomicReference; + import org.apache.accumulo.core.conf.AccumuloConfiguration; import org.apache.accumulo.core.conf.Property; import org.apache.accumulo.core.spi.crypto.CryptoService; import org.apache.accumulo.core.spi.crypto.CryptoService.CryptoException; import org.apache.accumulo.start.classloader.vfs.AccumuloVFSClassLoader; public class CryptoServiceFactory { - private static CryptoService singleton = null; + private static AtomicReference singleton = new AtomicReference<>(); /** * Load the singleton class configured in {@link Property#INSTANCE_CRYPTO_SERVICE} */ public static CryptoService getConfigured(AccumuloConfiguration conf) { +CryptoService cyptoService = singleton.get(); String configuredClass = conf.get(Property.INSTANCE_CRYPTO_SERVICE.getKey()); -if (singleton == null) { - singleton = loadCryptoService(configuredClass); - singleton.init(conf.getAllPropertiesWithPrefix(Property.INSTANCE_CRYPTO_PREFIX)); -} else { - if (!singleton.getClass().getName().equals(configuredClass)) { -singleton = loadCryptoService(configuredClass); - singleton.init(conf.getAllPropertiesWithPrefix(Property.INSTANCE_CRYPTO_PREFIX)); - } +if (cyptoService == null || !cyptoService.getClass().getName().equals(configuredClass)) { + CryptoService newCryptoService = loadCryptoService(configuredClass); + newCryptoService.init(conf.getAllPropertiesWithPrefix(Property.INSTANCE_CRYPTO_PREFIX)); + singleton.compareAndSet(cyptoService, newCryptoService); + return singleton.get(); Review comment: Hmm OK I hadn't thought of that. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] milleruntime commented on a change in pull request #560: Provide new Crypto interface & impl
milleruntime commented on a change in pull request #560: Provide new Crypto interface & impl URL: https://github.com/apache/accumulo/pull/560#discussion_r207392392 ## File path: core/src/main/java/org/apache/accumulo/core/security/crypto/CryptoServiceFactory.java ## @@ -0,0 +1,54 @@ +/* + * 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.accumulo.core.security.crypto; + +import org.apache.accumulo.core.conf.AccumuloConfiguration; +import org.apache.accumulo.core.conf.Property; +import org.apache.accumulo.core.spi.crypto.CryptoService; +import org.apache.accumulo.core.spi.crypto.CryptoService.CryptoException; +import org.apache.accumulo.start.classloader.vfs.AccumuloVFSClassLoader; + +public class CryptoServiceFactory { + private static CryptoService singleton = null; + + /** + * Load the singleton class configured in {@link Property#INSTANCE_CRYPTO_SERVICE} + */ + public static CryptoService getConfigured(AccumuloConfiguration conf) { Review comment: Where in TabletServer would cryptoService get created just once? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] milleruntime commented on a change in pull request #560: Provide new Crypto interface & impl
milleruntime commented on a change in pull request #560: Provide new Crypto interface & impl URL: https://github.com/apache/accumulo/pull/560#discussion_r207378931 ## File path: core/src/main/java/org/apache/accumulo/core/security/crypto/impl/AESCryptoService.java ## @@ -0,0 +1,518 @@ +/* + * 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.accumulo.core.security.crypto.impl; + +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.io.DataInputStream; +import java.io.DataOutputStream; +import java.io.IOException; +import java.io.InputStream; +import java.io.OutputStream; +import java.security.InvalidAlgorithmParameterException; +import java.security.InvalidKeyException; +import java.security.Key; +import java.security.NoSuchAlgorithmException; +import java.security.NoSuchProviderException; +import java.security.SecureRandom; +import java.util.Arrays; +import java.util.HashMap; +import java.util.Map; +import java.util.Objects; + +import javax.crypto.Cipher; +import javax.crypto.CipherInputStream; +import javax.crypto.CipherOutputStream; +import javax.crypto.NoSuchPaddingException; +import javax.crypto.spec.GCMParameterSpec; +import javax.crypto.spec.IvParameterSpec; + +import org.apache.accumulo.core.security.crypto.CryptoUtils; +import org.apache.accumulo.core.security.crypto.streams.BlockedInputStream; +import org.apache.accumulo.core.security.crypto.streams.BlockedOutputStream; +import org.apache.accumulo.core.security.crypto.streams.DiscardCloseOutputStream; +import org.apache.accumulo.core.security.crypto.streams.RFileCipherOutputStream; +import org.apache.accumulo.core.spi.crypto.CryptoEnvironment; +import org.apache.accumulo.core.spi.crypto.CryptoService; +import org.apache.accumulo.core.spi.crypto.FileDecrypter; +import org.apache.accumulo.core.spi.crypto.FileEncrypter; + +/** + * Example implementation of AES encryption for Accumulo + */ +public class AESCryptoService implements CryptoService { + + // Hard coded NoCryptoService.VERSION - this permits the removal of NoCryptoService from the + // core jar, allowing use of only one crypto service + private static final String NO_CRYPTO_VERSION = "U+1F47B"; + + private Key encryptingKek = null; + private String encryptingKekId = null; + private String encryptingKeyManager = null; + // Lets just load keks for reading once + private static HashMap decryptingKeys = new HashMap(); + + @Override + public void init(Map conf) throws CryptoException { +String kekId = conf.get("instance.crypto.opts.kekId"); +String keyMgr = conf.get("instance.crypto.opts.keyManager"); +Objects.requireNonNull(kekId, "Config property table.crypto.opts.kekId is required."); +Objects.requireNonNull(keyMgr, "Config property table.crypto.opts.keyManager is required."); +switch (keyMgr) { + case KeyManager.URI: +this.encryptingKeyManager = keyMgr; +this.encryptingKekId = kekId; +this.encryptingKek = KeyManager.loadKekFromUri(kekId); +break; + default: +throw new CryptoException("Unrecognized key manager"); +} + + } + + @Override + public FileEncrypter getFileEncrypter(CryptoEnvironment environment) { +CryptoModule cm; +switch (environment.getScope()) { + case WAL: +cm = new AESCBCCryptoModule(this.encryptingKek, this.encryptingKekId, +this.encryptingKeyManager); +return cm.getEncrypter(); + + case RFILE: +cm = new AESGCMCryptoModule(this.encryptingKek, this.encryptingKekId, +this.encryptingKeyManager); +return cm.getEncrypter(); + + default: +throw new CryptoException("Unknown scope: " + environment.getScope()); +} + } + + @Override + public FileDecrypter getFileDecrypter(CryptoEnvironment environment) { +CryptoModule cm; +ParsedCryptoParameters parsed = parseCryptoParameters(environment.getDecryptionParams()); +Key kek = loadDecryptionKek(parsed); +Key fek = KeyManager.unwrapKey(parsed.getEncFek(), kek); +switch (parsed.getCryptoServiceVersion()) { + case AESCBCCryptoModule.VERSION: +cm = new AESCBCCryptoModule(this.encryptingKek,
[GitHub] milleruntime commented on a change in pull request #560: Provide new Crypto interface & impl
milleruntime commented on a change in pull request #560: Provide new Crypto interface & impl URL: https://github.com/apache/accumulo/pull/560#discussion_r207358840 ## File path: core/src/main/java/org/apache/accumulo/core/security/crypto/impl/AESCryptoService.java ## @@ -0,0 +1,518 @@ +/* + * 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.accumulo.core.security.crypto.impl; + +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.io.DataInputStream; +import java.io.DataOutputStream; +import java.io.IOException; +import java.io.InputStream; +import java.io.OutputStream; +import java.security.InvalidAlgorithmParameterException; +import java.security.InvalidKeyException; +import java.security.Key; +import java.security.NoSuchAlgorithmException; +import java.security.NoSuchProviderException; +import java.security.SecureRandom; +import java.util.Arrays; +import java.util.HashMap; +import java.util.Map; +import java.util.Objects; + +import javax.crypto.Cipher; +import javax.crypto.CipherInputStream; +import javax.crypto.CipherOutputStream; +import javax.crypto.NoSuchPaddingException; +import javax.crypto.spec.GCMParameterSpec; +import javax.crypto.spec.IvParameterSpec; + +import org.apache.accumulo.core.security.crypto.CryptoUtils; +import org.apache.accumulo.core.security.crypto.streams.BlockedInputStream; +import org.apache.accumulo.core.security.crypto.streams.BlockedOutputStream; +import org.apache.accumulo.core.security.crypto.streams.DiscardCloseOutputStream; +import org.apache.accumulo.core.security.crypto.streams.RFileCipherOutputStream; +import org.apache.accumulo.core.spi.crypto.CryptoEnvironment; +import org.apache.accumulo.core.spi.crypto.CryptoService; +import org.apache.accumulo.core.spi.crypto.FileDecrypter; +import org.apache.accumulo.core.spi.crypto.FileEncrypter; + +/** + * Example implementation of AES encryption for Accumulo + */ +public class AESCryptoService implements CryptoService { + + // Hard coded NoCryptoService.VERSION - this permits the removal of NoCryptoService from the + // core jar, allowing use of only one crypto service + private static final String NO_CRYPTO_VERSION = "U+1F47B"; + + private Key encryptingKek = null; + private String encryptingKekId = null; + private String encryptingKeyManager = null; + // Lets just load keks for reading once + private static HashMap decryptingKeys = new HashMap(); + + @Override + public void init(Map conf) throws CryptoException { +String kekId = conf.get("instance.crypto.opts.kekId"); +String keyMgr = conf.get("instance.crypto.opts.keyManager"); +Objects.requireNonNull(kekId, "Config property table.crypto.opts.kekId is required."); +Objects.requireNonNull(keyMgr, "Config property table.crypto.opts.keyManager is required."); +switch (keyMgr) { + case KeyManager.URI: +this.encryptingKeyManager = keyMgr; +this.encryptingKekId = kekId; +this.encryptingKek = KeyManager.loadKekFromUri(kekId); +break; + default: +throw new CryptoException("Unrecognized key manager"); +} + + } + + @Override + public FileEncrypter getFileEncrypter(CryptoEnvironment environment) { +CryptoModule cm; +switch (environment.getScope()) { + case WAL: +cm = new AESCBCCryptoModule(this.encryptingKek, this.encryptingKekId, +this.encryptingKeyManager); +return cm.getEncrypter(); + + case RFILE: +cm = new AESGCMCryptoModule(this.encryptingKek, this.encryptingKekId, +this.encryptingKeyManager); +return cm.getEncrypter(); + + default: +throw new CryptoException("Unknown scope: " + environment.getScope()); +} + } + + @Override + public FileDecrypter getFileDecrypter(CryptoEnvironment environment) { +CryptoModule cm; +ParsedCryptoParameters parsed = parseCryptoParameters(environment.getDecryptionParams()); +Key kek = loadDecryptionKek(parsed); +Key fek = KeyManager.unwrapKey(parsed.getEncFek(), kek); +switch (parsed.getCryptoServiceVersion()) { + case AESCBCCryptoModule.VERSION: +cm = new AESCBCCryptoModule(this.encryptingKek,
[GitHub] milleruntime commented on a change in pull request #560: Provide new Crypto interface & impl
milleruntime commented on a change in pull request #560: Provide new Crypto interface & impl URL: https://github.com/apache/accumulo/pull/560#discussion_r207356731 ## File path: core/src/main/java/org/apache/accumulo/core/security/crypto/impl/AESCryptoService.java ## @@ -0,0 +1,518 @@ +/* + * 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.accumulo.core.security.crypto.impl; + +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.io.DataInputStream; +import java.io.DataOutputStream; +import java.io.IOException; +import java.io.InputStream; +import java.io.OutputStream; +import java.security.InvalidAlgorithmParameterException; +import java.security.InvalidKeyException; +import java.security.Key; +import java.security.NoSuchAlgorithmException; +import java.security.NoSuchProviderException; +import java.security.SecureRandom; +import java.util.Arrays; +import java.util.HashMap; +import java.util.Map; +import java.util.Objects; + +import javax.crypto.Cipher; +import javax.crypto.CipherInputStream; +import javax.crypto.CipherOutputStream; +import javax.crypto.NoSuchPaddingException; +import javax.crypto.spec.GCMParameterSpec; +import javax.crypto.spec.IvParameterSpec; + +import org.apache.accumulo.core.security.crypto.CryptoUtils; +import org.apache.accumulo.core.security.crypto.streams.BlockedInputStream; +import org.apache.accumulo.core.security.crypto.streams.BlockedOutputStream; +import org.apache.accumulo.core.security.crypto.streams.DiscardCloseOutputStream; +import org.apache.accumulo.core.security.crypto.streams.RFileCipherOutputStream; +import org.apache.accumulo.core.spi.crypto.CryptoEnvironment; +import org.apache.accumulo.core.spi.crypto.CryptoService; +import org.apache.accumulo.core.spi.crypto.FileDecrypter; +import org.apache.accumulo.core.spi.crypto.FileEncrypter; + +/** + * Example implementation of AES encryption for Accumulo + */ +public class AESCryptoService implements CryptoService { + + // Hard coded NoCryptoService.VERSION - this permits the removal of NoCryptoService from the + // core jar, allowing use of only one crypto service + private static final String NO_CRYPTO_VERSION = "U+1F47B"; + + private Key encryptingKek = null; + private String encryptingKekId = null; + private String encryptingKeyManager = null; + // Lets just load keks for reading once + private static HashMap decryptingKeys = new HashMap(); + + @Override + public void init(Map conf) throws CryptoException { +String kekId = conf.get("instance.crypto.opts.kekId"); +String keyMgr = conf.get("instance.crypto.opts.keyManager"); +Objects.requireNonNull(kekId, "Config property table.crypto.opts.kekId is required."); +Objects.requireNonNull(keyMgr, "Config property table.crypto.opts.keyManager is required."); +switch (keyMgr) { + case KeyManager.URI: +this.encryptingKeyManager = keyMgr; +this.encryptingKekId = kekId; +this.encryptingKek = KeyManager.loadKekFromUri(kekId); +break; + default: +throw new CryptoException("Unrecognized key manager"); +} + + } + + @Override + public FileEncrypter getFileEncrypter(CryptoEnvironment environment) { +CryptoModule cm; +switch (environment.getScope()) { + case WAL: +cm = new AESCBCCryptoModule(this.encryptingKek, this.encryptingKekId, +this.encryptingKeyManager); +return cm.getEncrypter(); + + case RFILE: +cm = new AESGCMCryptoModule(this.encryptingKek, this.encryptingKekId, +this.encryptingKeyManager); +return cm.getEncrypter(); + + default: +throw new CryptoException("Unknown scope: " + environment.getScope()); +} + } + + @Override + public FileDecrypter getFileDecrypter(CryptoEnvironment environment) { +CryptoModule cm; +ParsedCryptoParameters parsed = parseCryptoParameters(environment.getDecryptionParams()); +Key kek = loadDecryptionKek(parsed); +Key fek = KeyManager.unwrapKey(parsed.getEncFek(), kek); +switch (parsed.getCryptoServiceVersion()) { + case AESCBCCryptoModule.VERSION: +cm = new AESCBCCryptoModule(this.encryptingKek,
[GitHub] milleruntime commented on a change in pull request #560: Provide new Crypto interface & impl
milleruntime commented on a change in pull request #560: Provide new Crypto interface & impl URL: https://github.com/apache/accumulo/pull/560#discussion_r207281193 ## File path: core/src/main/java/org/apache/accumulo/core/security/crypto/CryptoServiceFactory.java ## @@ -0,0 +1,54 @@ +/* + * 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.accumulo.core.security.crypto; + +import org.apache.accumulo.core.conf.AccumuloConfiguration; +import org.apache.accumulo.core.conf.Property; +import org.apache.accumulo.core.spi.crypto.CryptoService; +import org.apache.accumulo.core.spi.crypto.CryptoService.CryptoException; +import org.apache.accumulo.start.classloader.vfs.AccumuloVFSClassLoader; + +public class CryptoServiceFactory { + private static CryptoService singleton = null; + + /** + * Load the singleton class configured in {@link Property#INSTANCE_CRYPTO_SERVICE} + */ + public static CryptoService getConfigured(AccumuloConfiguration conf) { Review comment: I guess the issue with thread safety is with calling CryptoService.init(), since you only want that to ever be called once. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] milleruntime commented on a change in pull request #560: Provide new Crypto interface & impl
milleruntime commented on a change in pull request #560: Provide new Crypto interface & impl URL: https://github.com/apache/accumulo/pull/560#discussion_r207269106 ## File path: core/src/main/java/org/apache/accumulo/core/security/crypto/impl/AESCryptoService.java ## @@ -0,0 +1,518 @@ +/* + * 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.accumulo.core.security.crypto.impl; + +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.io.DataInputStream; +import java.io.DataOutputStream; +import java.io.IOException; +import java.io.InputStream; +import java.io.OutputStream; +import java.security.InvalidAlgorithmParameterException; +import java.security.InvalidKeyException; +import java.security.Key; +import java.security.NoSuchAlgorithmException; +import java.security.NoSuchProviderException; +import java.security.SecureRandom; +import java.util.Arrays; +import java.util.HashMap; +import java.util.Map; +import java.util.Objects; + +import javax.crypto.Cipher; +import javax.crypto.CipherInputStream; +import javax.crypto.CipherOutputStream; +import javax.crypto.NoSuchPaddingException; +import javax.crypto.spec.GCMParameterSpec; +import javax.crypto.spec.IvParameterSpec; + +import org.apache.accumulo.core.security.crypto.CryptoUtils; +import org.apache.accumulo.core.security.crypto.streams.BlockedInputStream; +import org.apache.accumulo.core.security.crypto.streams.BlockedOutputStream; +import org.apache.accumulo.core.security.crypto.streams.DiscardCloseOutputStream; +import org.apache.accumulo.core.security.crypto.streams.RFileCipherOutputStream; +import org.apache.accumulo.core.spi.crypto.CryptoEnvironment; +import org.apache.accumulo.core.spi.crypto.CryptoService; +import org.apache.accumulo.core.spi.crypto.FileDecrypter; +import org.apache.accumulo.core.spi.crypto.FileEncrypter; + +/** + * Example implementation of AES encryption for Accumulo + */ +public class AESCryptoService implements CryptoService { + + // Hard coded NoCryptoService.VERSION - this permits the removal of NoCryptoService from the + // core jar, allowing use of only one crypto service + private static final String NO_CRYPTO_VERSION = "U+1F47B"; + + private Key encryptingKek = null; + private String encryptingKekId = null; + private String encryptingKeyManager = null; + // Lets just load keks for reading once + private static HashMap decryptingKeys = new HashMap(); + + @Override + public void init(Map conf) throws CryptoException { +String kekId = conf.get("instance.crypto.opts.kekId"); +String keyMgr = conf.get("instance.crypto.opts.keyManager"); +Objects.requireNonNull(kekId, "Config property table.crypto.opts.kekId is required."); +Objects.requireNonNull(keyMgr, "Config property table.crypto.opts.keyManager is required."); +switch (keyMgr) { + case KeyManager.URI: +this.encryptingKeyManager = keyMgr; +this.encryptingKekId = kekId; +this.encryptingKek = KeyManager.loadKekFromUri(kekId); +break; + default: +throw new CryptoException("Unrecognized key manager"); +} + + } + + @Override + public FileEncrypter getFileEncrypter(CryptoEnvironment environment) { +CryptoModule cm; +switch (environment.getScope()) { + case WAL: +cm = new AESCBCCryptoModule(this.encryptingKek, this.encryptingKekId, +this.encryptingKeyManager); +return cm.getEncrypter(); + + case RFILE: +cm = new AESGCMCryptoModule(this.encryptingKek, this.encryptingKekId, +this.encryptingKeyManager); +return cm.getEncrypter(); + + default: +throw new CryptoException("Unknown scope: " + environment.getScope()); +} + } + + @Override + public FileDecrypter getFileDecrypter(CryptoEnvironment environment) { +CryptoModule cm; +ParsedCryptoParameters parsed = parseCryptoParameters(environment.getDecryptionParams()); +Key kek = loadDecryptionKek(parsed); +Key fek = KeyManager.unwrapKey(parsed.getEncFek(), kek); +switch (parsed.getCryptoServiceVersion()) { + case AESCBCCryptoModule.VERSION: +cm = new AESCBCCryptoModule(this.encryptingKek,
[GitHub] milleruntime commented on a change in pull request #560: Provide new Crypto interface & impl
milleruntime commented on a change in pull request #560: Provide new Crypto interface & impl URL: https://github.com/apache/accumulo/pull/560#discussion_r207261393 ## File path: core/src/main/java/org/apache/accumulo/core/security/crypto/CryptoServiceFactory.java ## @@ -0,0 +1,54 @@ +/* + * 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.accumulo.core.security.crypto; + +import org.apache.accumulo.core.conf.AccumuloConfiguration; +import org.apache.accumulo.core.conf.Property; +import org.apache.accumulo.core.spi.crypto.CryptoService; +import org.apache.accumulo.core.spi.crypto.CryptoService.CryptoException; +import org.apache.accumulo.start.classloader.vfs.AccumuloVFSClassLoader; + +public class CryptoServiceFactory { + private static CryptoService singleton = null; + + /** + * Load the singleton class configured in {@link Property#INSTANCE_CRYPTO_SERVICE} + */ + public static CryptoService getConfigured(AccumuloConfiguration conf) { Review comment: Does it need to be thread safe? Tservers shouldn't have different configuration and should fail the instance start up so the class shouldn't be different. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] milleruntime commented on a change in pull request #560: Provide new Crypto interface & impl
milleruntime commented on a change in pull request #560: Provide new Crypto interface & impl URL: https://github.com/apache/accumulo/pull/560#discussion_r206698438 ## File path: core/src/main/java/org/apache/accumulo/core/security/crypto/impl/AESCryptoService.java ## @@ -0,0 +1,530 @@ +/* + * 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.accumulo.core.security.crypto.impl; + +import java.io.IOException; +import java.io.InputStream; +import java.io.OutputStream; +import java.nio.charset.Charset; +import java.security.InvalidAlgorithmParameterException; +import java.security.InvalidKeyException; +import java.security.Key; +import java.security.NoSuchAlgorithmException; +import java.security.NoSuchProviderException; +import java.security.SecureRandom; +import java.util.Arrays; +import java.util.HashMap; +import java.util.Map; +import java.util.Objects; +import java.util.StringJoiner; + +import javax.crypto.Cipher; +import javax.crypto.CipherInputStream; +import javax.crypto.CipherOutputStream; +import javax.crypto.NoSuchPaddingException; +import javax.crypto.spec.GCMParameterSpec; +import javax.crypto.spec.IvParameterSpec; + +import org.apache.accumulo.core.security.crypto.BlockedInputStream; +import org.apache.accumulo.core.security.crypto.BlockedOutputStream; +import org.apache.accumulo.core.security.crypto.CryptoEnvironment; +import org.apache.accumulo.core.security.crypto.CryptoService; +import org.apache.accumulo.core.security.crypto.CryptoUtils; +import org.apache.accumulo.core.security.crypto.DiscardCloseOutputStream; +import org.apache.accumulo.core.security.crypto.FileDecrypter; +import org.apache.accumulo.core.security.crypto.FileEncrypter; + +/** + * Example implementation of AES encryption for Accumulo + */ +public class AESCryptoService implements CryptoService { Review comment: Created a follow on issue for this: https://github.com/apache/accumulo-website/issues/101 This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] milleruntime commented on a change in pull request #560: Provide new Crypto interface & impl
milleruntime commented on a change in pull request #560: Provide new Crypto interface & impl URL: https://github.com/apache/accumulo/pull/560#discussion_r206691938 ## File path: core/src/main/java/org/apache/accumulo/core/security/crypto/impl/AESCryptoService.java ## @@ -0,0 +1,530 @@ +/* + * 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.accumulo.core.security.crypto.impl; + +import java.io.IOException; +import java.io.InputStream; +import java.io.OutputStream; +import java.nio.charset.Charset; +import java.security.InvalidAlgorithmParameterException; +import java.security.InvalidKeyException; +import java.security.Key; +import java.security.NoSuchAlgorithmException; +import java.security.NoSuchProviderException; +import java.security.SecureRandom; +import java.util.Arrays; +import java.util.HashMap; +import java.util.Map; +import java.util.Objects; +import java.util.StringJoiner; + +import javax.crypto.Cipher; +import javax.crypto.CipherInputStream; +import javax.crypto.CipherOutputStream; +import javax.crypto.NoSuchPaddingException; +import javax.crypto.spec.GCMParameterSpec; +import javax.crypto.spec.IvParameterSpec; + +import org.apache.accumulo.core.security.crypto.BlockedInputStream; +import org.apache.accumulo.core.security.crypto.BlockedOutputStream; +import org.apache.accumulo.core.security.crypto.CryptoEnvironment; +import org.apache.accumulo.core.security.crypto.CryptoService; +import org.apache.accumulo.core.security.crypto.CryptoUtils; +import org.apache.accumulo.core.security.crypto.DiscardCloseOutputStream; +import org.apache.accumulo.core.security.crypto.FileDecrypter; +import org.apache.accumulo.core.security.crypto.FileEncrypter; + +/** + * Example implementation of AES encryption for Accumulo + */ +public class AESCryptoService implements CryptoService { Review comment: Nevermind, I think I found it: https://github.com/apache/accumulo-website/tree/master/_docs-2-0 This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] milleruntime commented on a change in pull request #560: Provide new Crypto interface & impl
milleruntime commented on a change in pull request #560: Provide new Crypto interface & impl URL: https://github.com/apache/accumulo/pull/560#discussion_r206686648 ## File path: core/src/main/java/org/apache/accumulo/core/security/crypto/impl/AESCryptoService.java ## @@ -0,0 +1,530 @@ +/* + * 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.accumulo.core.security.crypto.impl; + +import java.io.IOException; +import java.io.InputStream; +import java.io.OutputStream; +import java.nio.charset.Charset; +import java.security.InvalidAlgorithmParameterException; +import java.security.InvalidKeyException; +import java.security.Key; +import java.security.NoSuchAlgorithmException; +import java.security.NoSuchProviderException; +import java.security.SecureRandom; +import java.util.Arrays; +import java.util.HashMap; +import java.util.Map; +import java.util.Objects; +import java.util.StringJoiner; + +import javax.crypto.Cipher; +import javax.crypto.CipherInputStream; +import javax.crypto.CipherOutputStream; +import javax.crypto.NoSuchPaddingException; +import javax.crypto.spec.GCMParameterSpec; +import javax.crypto.spec.IvParameterSpec; + +import org.apache.accumulo.core.security.crypto.BlockedInputStream; +import org.apache.accumulo.core.security.crypto.BlockedOutputStream; +import org.apache.accumulo.core.security.crypto.CryptoEnvironment; +import org.apache.accumulo.core.security.crypto.CryptoService; +import org.apache.accumulo.core.security.crypto.CryptoUtils; +import org.apache.accumulo.core.security.crypto.DiscardCloseOutputStream; +import org.apache.accumulo.core.security.crypto.FileDecrypter; +import org.apache.accumulo.core.security.crypto.FileEncrypter; + +/** + * Example implementation of AES encryption for Accumulo + */ +public class AESCryptoService implements CryptoService { Review comment: Where does this exist? I can at least open a ticket to have this done once this stuff gets merged. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] milleruntime commented on a change in pull request #560: Provide new Crypto interface & impl
milleruntime commented on a change in pull request #560: Provide new Crypto interface & impl URL: https://github.com/apache/accumulo/pull/560#discussion_r206630967 ## File path: core/src/main/java/org/apache/accumulo/core/security/crypto/impl/AESCryptoService.java ## @@ -0,0 +1,530 @@ +/* + * 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.accumulo.core.security.crypto.impl; + +import java.io.IOException; +import java.io.InputStream; +import java.io.OutputStream; +import java.nio.charset.Charset; +import java.security.InvalidAlgorithmParameterException; +import java.security.InvalidKeyException; +import java.security.Key; +import java.security.NoSuchAlgorithmException; +import java.security.NoSuchProviderException; +import java.security.SecureRandom; +import java.util.Arrays; +import java.util.HashMap; +import java.util.Map; +import java.util.Objects; +import java.util.StringJoiner; + +import javax.crypto.Cipher; +import javax.crypto.CipherInputStream; +import javax.crypto.CipherOutputStream; +import javax.crypto.NoSuchPaddingException; +import javax.crypto.spec.GCMParameterSpec; +import javax.crypto.spec.IvParameterSpec; + +import org.apache.accumulo.core.security.crypto.BlockedInputStream; +import org.apache.accumulo.core.security.crypto.BlockedOutputStream; +import org.apache.accumulo.core.security.crypto.CryptoEnvironment; +import org.apache.accumulo.core.security.crypto.CryptoService; +import org.apache.accumulo.core.security.crypto.CryptoUtils; +import org.apache.accumulo.core.security.crypto.DiscardCloseOutputStream; +import org.apache.accumulo.core.security.crypto.FileDecrypter; +import org.apache.accumulo.core.security.crypto.FileEncrypter; + +/** + * Example implementation of AES encryption for Accumulo + */ +public class AESCryptoService implements CryptoService { + + private Key encryptingKek = null; + private String encryptingKekId = null; + private String encryptingKeyManager = null; + // Lets just load keks for reading once + private static HashMap decryptingKeys = new HashMap(); + + // Each byte has a valid unicode representation + private static final String unicodeCharset = "ISO_8859_1"; + + @Override + public void init(Map conf) throws CryptoException { +String kekId = conf.get("table.crypto.opts.kekId"); +String keyMgr = conf.get("table.crypto.opts.keyManager"); +Objects.requireNonNull(kekId, "Config property table.crypto.opts.kekId is required."); +Objects.requireNonNull(keyMgr, "Config property table.crypto.opts.keyManager is required."); +switch (keyMgr) { + case KeyManager.URI: +this.encryptingKeyManager = keyMgr; +this.encryptingKekId = kekId; +this.encryptingKek = KeyManager.loadKekFromUri(kekId); +break; + default: +throw new CryptoException("Unrecognized key manager"); +} + + } + + @Override + public FileEncrypter getFileEncrypter(CryptoEnvironment environment) { +CryptoModule cm; +switch (environment.getScope()) { + case WAL: +cm = new AESCBCCryptoModule(this.encryptingKek, this.encryptingKekId, +this.encryptingKeyManager); +return cm.getEncrypter(); + + case RFILE: +cm = new AESGCMCryptoModule(this.encryptingKek, this.encryptingKekId, +this.encryptingKeyManager); +return cm.getEncrypter(); + + default: +throw new CryptoException("Unknown scope: " + environment.getScope()); +} + } + + @Override + public FileDecrypter getFileDecrypter(CryptoEnvironment environment) { +CryptoModule cm; +ParsedCryptoParameters parsed = parseCryptoParameters(environment.getParameters()); +Key kek = loadDecryptionKek(parsed); +Key fek = KeyManager.unwrapKey(parsed.getEncFek(), kek); +switch (parsed.getCryptoServiceVersion()) { + case AESCBCCryptoModule.VERSION: +cm = new AESCBCCryptoModule(this.encryptingKek, this.encryptingKekId, +this.encryptingKeyManager); +return (cm.getDecrypter(fek)); + case AESGCMCryptoModule.VERSION: +cm = new AESGCMCryptoModule(this.encryptingKek, this.encryptingKekId, +this.encryptingKeyManager); +return
[GitHub] milleruntime commented on a change in pull request #560: Provide new Crypto interface & impl
milleruntime commented on a change in pull request #560: Provide new Crypto interface & impl URL: https://github.com/apache/accumulo/pull/560#discussion_r206622449 ## File path: core/src/main/java/org/apache/accumulo/core/security/crypto/CryptoService.java ## @@ -0,0 +1,74 @@ +/* + * 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.accumulo.core.security.crypto; + +import java.util.Map; + +/** + * Self contained cryptographic service. All on disk encryption and decryption will take place + * through this interface. Each implementation must implement a {@link FileEncrypter} for encryption + * and a {@link FileDecrypter} for decryption. + * + * @since 2.0 + */ +public interface CryptoService { + + /** + * Initialize CryptoService. This is called once at Tablet Server startup. + * + * @since 2.0 + */ + void init(Map conf) throws CryptoException; Review comment: I modified CryptoEnvironment to be an interface with only two methods to get scope and decryptionParams (the new name to params). Does this make more sense with init? Or do you think I should mention something about configuration in the javadoc. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] milleruntime commented on a change in pull request #560: Provide new Crypto interface & impl
milleruntime commented on a change in pull request #560: Provide new Crypto interface & impl URL: https://github.com/apache/accumulo/pull/560#discussion_r206619514 ## File path: core/src/main/java/org/apache/accumulo/core/security/crypto/FileEncrypter.java ## @@ -0,0 +1,43 @@ +/* + * 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.accumulo.core.security.crypto; + +import java.io.OutputStream; + +public interface FileEncrypter { + /** + * Encrypt the OutputStream. + * + * @since 2.0 Review comment: Ok cool. Will do. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services