diff --git a/scm-core/src/main/java/sonia/scm/security/DefaultCipherHandler.java b/scm-core/src/main/java/sonia/scm/security/DefaultCipherHandler.java index 8250c91a44..77e08ac0d5 100644 --- a/scm-core/src/main/java/sonia/scm/security/DefaultCipherHandler.java +++ b/scm-core/src/main/java/sonia/scm/security/DefaultCipherHandler.java @@ -21,7 +21,7 @@ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE * SOFTWARE. */ - + package sonia.scm.security; //~--- non-JDK imports -------------------------------------------------------- @@ -42,6 +42,8 @@ import java.io.FileNotFoundException; import java.io.FileReader; import java.io.IOException; import java.io.PrintWriter; +import java.nio.charset.Charset; +import java.nio.charset.StandardCharsets; import java.security.GeneralSecurityException; import java.security.MessageDigest; @@ -51,27 +53,38 @@ import java.security.SecureRandom; import java.util.Arrays; import java.util.Base64; +import javax.crypto.Cipher; import javax.crypto.SecretKey; +import javax.crypto.spec.GCMParameterSpec; import javax.crypto.spec.IvParameterSpec; import javax.crypto.spec.SecretKeySpec; /** - * Default implementation of the {@link CipherHandler}, which uses AES for + * Default implementation of the {@link CipherHandler}, which uses AES for * encryption and decryption. - * + * * @author Sebastian Sdorra * @since 1.7 */ public class DefaultCipherHandler implements CipherHandler { - /** used cipher type */ - public static final String CIPHER_TYPE = "AES/CTR/PKCS5PADDING"; + /** + * Cipher type used before v2. + * @see Issue 1110 + */ + public static final String OLD_CIPHER_TYPE = "AES/CTR/PKCS5PADDING"; + + /** used cipher type for format v2 */ + public static final String CIPHER_TYPE = "AES/GCM/NoPadding"; + + /** prefix to detect new format */ + public static final String PREFIX_FORMAT_V2 = "v2:"; /** digest type for key generation */ public static final String DIGEST_TYPE = "SHA-512"; /** string encoding */ - public static final String ENCODING = "UTF-8"; + public static final Charset ENCODING = StandardCharsets.UTF_8; /** default key length */ public static final int KEY_LENGTH = 16; @@ -92,12 +105,12 @@ public class DefaultCipherHandler implements CipherHandler { private static final String KEY_TYPE = "AES"; /** the logger for DefaultCipherHandler */ - private static final Logger logger = LoggerFactory.getLogger(DefaultCipherHandler.class); - + private static final Logger LOG = LoggerFactory.getLogger(DefaultCipherHandler.class); + private final SecureRandom random = new SecureRandom(); - + private final char[] key; - + /** * Constructs a new DefaultCipherHandler. Note this constructor is only for * unit tests. @@ -112,7 +125,7 @@ public class DefaultCipherHandler implements CipherHandler { } /** - * Constructs a new instance and reads the default key from the scm home directory, + * Constructs a new instance and reads the default key from the scm home directory, * if the key file does not exists it will be generated with the {@link KeyGenerator}. * * @param context SCM-Manager context provider @@ -161,25 +174,26 @@ public class DefaultCipherHandler implements CipherHandler { } private String decode(char[] plainKey, String value, Base64.Decoder decoder) { + CipherFactory cipherFactory = oldCipherFactor; + if (value.startsWith(PREFIX_FORMAT_V2)) { + cipherFactory = v2CipherFactor; + value = value.substring(PREFIX_FORMAT_V2.length()); + } else { + LOG.warn("found encrypted data in old format, the data should be stored again to ensure the new format is used"); + } + try { byte[] encodedInput = decoder.decode(value); byte[] salt = new byte[SALT_LENGTH]; byte[] encoded = new byte[encodedInput.length - SALT_LENGTH]; System.arraycopy(encodedInput, 0, salt, 0, SALT_LENGTH); - System.arraycopy(encodedInput, SALT_LENGTH, encoded, 0, - encodedInput.length - SALT_LENGTH); - - IvParameterSpec iv = new IvParameterSpec(salt); - SecretKey secretKey = buildSecretKey(plainKey); - javax.crypto.Cipher cipher = javax.crypto.Cipher.getInstance(CIPHER_TYPE); - - cipher.init(javax.crypto.Cipher.DECRYPT_MODE, secretKey, iv); + System.arraycopy(encodedInput, SALT_LENGTH, encoded, 0, encodedInput.length - SALT_LENGTH); + Cipher cipher = cipherFactory.create(plainKey, salt, Cipher.DECRYPT_MODE); byte[] decoded = cipher.doFinal(encoded); - return new String(decoded, ENCODING); - } catch (IOException | GeneralSecurityException ex) { + } catch (GeneralSecurityException ex) { throw new CipherException("could not decode string", ex); } } @@ -204,11 +218,7 @@ public class DefaultCipherHandler implements CipherHandler { random.nextBytes(salt); - IvParameterSpec iv = new IvParameterSpec(salt); - SecretKey secretKey = buildSecretKey(plainKey); - javax.crypto.Cipher cipher = javax.crypto.Cipher.getInstance(CIPHER_TYPE); - - cipher.init(javax.crypto.Cipher.ENCRYPT_MODE, secretKey, iv); + Cipher cipher = v2CipherFactor.create(plainKey, salt, Cipher.ENCRYPT_MODE); byte[] inputBytes = value.getBytes(ENCODING); byte[] encodedInput = cipher.doFinal(inputBytes); @@ -217,15 +227,15 @@ public class DefaultCipherHandler implements CipherHandler { System.arraycopy(salt, 0, result, 0, SALT_LENGTH); System.arraycopy(encodedInput, 0, result, SALT_LENGTH, result.length - SALT_LENGTH); - res = new String(Base64.getUrlEncoder().encode(result), ENCODING); - } catch (IOException | GeneralSecurityException ex) { + res = PREFIX_FORMAT_V2 + new String(Base64.getUrlEncoder().encode(result), ENCODING); + } catch (GeneralSecurityException ex) { throw new CipherException("could not encode string", ex); } return res; } - private SecretKey buildSecretKey(char[] plainKey) throws IOException, NoSuchAlgorithmException { + private SecretKey buildSecretKey(char[] plainKey) throws NoSuchAlgorithmException { byte[] raw = new String(plainKey).getBytes(ENCODING); MessageDigest digest = MessageDigest.getInstance(DIGEST_TYPE); @@ -237,16 +247,50 @@ public class DefaultCipherHandler implements CipherHandler { private char[] loadKey(File cipherKeyFile) throws IOException { try (BufferedReader reader = new BufferedReader(new FileReader(cipherKeyFile))) { - String line = reader.readLine(); + String encodedKey = reader.readLine(); - return decode(KEY_BASE, line).toCharArray(); + char[] decodedKey = decode(KEY_BASE, encodedKey).toCharArray(); + + // rewrite key in new format, if the stored key uses the old format + if (!encodedKey.startsWith(PREFIX_FORMAT_V2)) { + LOG.info("found default key in old format, rewrite with new format"); + storeKey(cipherKeyFile, decodedKey); + } + + return decodedKey; } } private void storeKey(File cipherKeyFile) throws FileNotFoundException { + storeKey(cipherKeyFile, key); + } + + private void storeKey(File cipherKeyFile, char[] key) throws FileNotFoundException { String storeKey = encode(KEY_BASE, new String(key)); try (PrintWriter output = new PrintWriter(cipherKeyFile)) { output.write(storeKey); } } + + @FunctionalInterface + private interface CipherFactory { + Cipher create(char[] plainKey, byte[] salt, int mode) throws GeneralSecurityException; + + } + + private final CipherFactory v2CipherFactor = (char[] plainKey, byte[] salt, int mode) -> { + Cipher cipher = Cipher.getInstance(CIPHER_TYPE); + SecretKey secretKey = buildSecretKey(plainKey); + GCMParameterSpec parameterSpec = new GCMParameterSpec(128, salt); + cipher.init(mode, secretKey, parameterSpec); + return cipher; + }; + + private final CipherFactory oldCipherFactor = (char[] plainKey, byte[] salt, int mode) -> { + Cipher cipher = Cipher.getInstance(OLD_CIPHER_TYPE); + SecretKey secretKey = buildSecretKey(plainKey); + IvParameterSpec iv = new IvParameterSpec(salt); + cipher.init(Cipher.DECRYPT_MODE, secretKey, iv); + return cipher; + }; } diff --git a/scm-core/src/test/java/sonia/scm/security/DefaultCipherHandlerTest.java b/scm-core/src/test/java/sonia/scm/security/DefaultCipherHandlerTest.java index 249daa0368..985e8eb1a8 100644 --- a/scm-core/src/test/java/sonia/scm/security/DefaultCipherHandlerTest.java +++ b/scm-core/src/test/java/sonia/scm/security/DefaultCipherHandlerTest.java @@ -24,6 +24,7 @@ package sonia.scm.security; +import com.google.common.io.Files; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.junitpioneer.jupiter.TempDirectory; @@ -32,6 +33,8 @@ import org.mockito.junit.jupiter.MockitoExtension; import sonia.scm.SCMContextProvider; import java.io.File; +import java.io.IOException; +import java.nio.charset.StandardCharsets; import java.nio.file.Path; import static org.assertj.core.api.Assertions.assertThat; @@ -55,7 +58,7 @@ public class DefaultCipherHandlerTest { * Tests loading and storing default key. */ @Test - void shouldLoadAndStoreDefaultKey(@TempDirectory.TempDir Path tempDir) { + void shouldLoadAndStoreDefaultKey(@TempDirectory.TempDir Path tempDir) throws IOException { File baseDirectory = tempDir.toFile(); when(context.getBaseDirectory()).thenReturn(baseDirectory); @@ -79,6 +82,30 @@ public class DefaultCipherHandlerTest { assertThat(cipher.decode(encrypted)).isEqualTo(plain); } + @Test + @SuppressWarnings("UnstableApiUsage") // is ok for unit test + void shouldReEncodeOldFormattedDefaultKey(@TempDirectory.TempDir Path tempDir) throws IOException { + String oldKey = "17eXopruTtX3S4dJ9KTEmbZ-vfZztw=="; + String encryptedValue = "A11kQF7wytpWCkjPflxJB-zUWJ1CVKU3qhwhRFq4Pvl6XqiS9V2w-gqNktqMX6YNDw=="; + String plainValue = "Marvin The Paranoid Android - RAM"; + + File baseDirectory = tempDir.toFile(); + + when(context.getBaseDirectory()).thenReturn(baseDirectory); + + File configDirectory = new File(baseDirectory, "config"); + configDirectory.mkdirs(); + File defaultKeyFile = new File(configDirectory, DefaultCipherHandler.CIPHERKEY_FILENAME); + Files.write(oldKey.getBytes(StandardCharsets.UTF_8), defaultKeyFile); + + + DefaultCipherHandler cipher = new DefaultCipherHandler(context, keyGenerator); + + String newKey = Files.readLines(defaultKeyFile, StandardCharsets.UTF_8).get(0); + assertThat(newKey).startsWith(DefaultCipherHandler.PREFIX_FORMAT_V2); + assertThat(cipher.decode(encryptedValue)).isEqualTo(plainValue); + } + /** * Test encode and decode method with a separate key. */ @@ -98,4 +125,18 @@ public class DefaultCipherHandlerTest { assertThat(cipher.decode(cipher.encode("hallo123"))).isEqualTo("hallo123"); } + @Test + void shouldDecodeOldCipherFormat() { + DefaultCipherHandler cipher = new DefaultCipherHandler("hitchhiker-secrets"); + String oldFormat = "zhoCMoApolM3cMPRqXHjcGBd-gDQN0JHwWBxvyh3xnCWzj5V"; + assertThat(cipher.decode(oldFormat)).isEqualTo("Arthur Dent's Secret"); + } + + @Test + void shouldAddPrefixToEncodedValue() { + DefaultCipherHandler cipher = new DefaultCipherHandler("hitchhiker-secrets"); + String encoded = cipher.encode("Trillian's Secret Dairy"); + assertThat(encoded).startsWith(DefaultCipherHandler.PREFIX_FORMAT_V2); + } + }