mirror of
https://github.com/scm-manager/scm-manager.git
synced 2026-01-30 03:09:13 +01:00
use AES/GCM/NoPadding instead of AES/CTR/PKCS5PADDING to ensure cipher works with newer java versions
Value are now encoded with a prefix to detect the new format. Old values are read with the old algorithm to ensure compatibility with older scm homes.
This commit is contained in:
@@ -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 <a href="https://github.com/scm-manager/scm-manager/issues/1110">Issue 1110</a>
|
||||
*/
|
||||
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;
|
||||
};
|
||||
}
|
||||
|
||||
@@ -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);
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user