Adding AES Walk Encryption support in http://www.jets3t.org/ mode

See previous attempt: https://git.eclipse.org/r/#/c/16674/

Here we preserve as much of JetS3t mode as possible
while allowing to use new Java 8+ PBE algorithms
such as PBEWithHmacSHA512AndAES_256

Summary of changes:
* change pom.xml to control long tests
* add WalkEncryptionTest.launch to run long tests
* add AmazonS3.Keys to to normalize use of constants
* change WalkEncryption to support AES in JetS3t mode
* add WalkEncryptionTest to test remote encryption pipeline
* add support for CI configuration for live Amazon S3 testing
* add log4j based logging for tests in both Eclipse and Maven build

To test locally, check out the review branch, then:
* create amazon test configuration file
* located your home dir: ${user.home}
* named jgit-s3-config.properties
* file format follows AmazonS3 connection settings file:
	accesskey = your-amazon-access-key
	secretkey = your-amazon-secret-key
	test.bucket = your-bucket-for-testing
* finally:
* run in Eclipse: WalkEncryptionTest.launch
* or
* run in Shell: mvn test --define test=WalkEncryptionTest

Change-Id: I6f455fd9fb4eac261ca73d0bec6a4e7dae9f2e91
Signed-off-by: Andrei Pozolotin <andrei.pozolotin@gmail.com>

4 files changed, 157 insertions, 66 deletions

diff --git a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties
index 34bbb415ba..51e44fd778 100644
--- a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties
+++ b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties
@@ -228,6 +228,7 @@ emptyCommit=No changes
 emptyPathNotPermitted=Empty path not permitted.
 emptyRef=Empty ref: {0}
 encryptionError=Encryption error: {0}
+encryptionOnlyPBE=Encryption error: only password-based encryption (PBE) algorithms are supported.
 endOfFileInEscape=End of file in escape
 entryNotFoundByPath=Entry not found by path: {0}
 enumValueNotSupported2=Invalid value: {0}.{1}={2}
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java
index 9067e82954..e39469bd8c 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java
@@ -287,6 +287,7 @@ public class JGitText extends TranslationBundle {
 	/***/ public String emptyPathNotPermitted;
 	/***/ public String emptyRef;
 	/***/ public String encryptionError;
+	/***/ public String encryptionOnlyPBE;
 	/***/ public String endOfFileInEscape;
 	/***/ public String entryNotFoundByPath;
 	/***/ public String enumValueNotSupported2;
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/AmazonS3.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/AmazonS3.java
index d3cdba5bf3..e55066a8bb 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/AmazonS3.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/AmazonS3.java
@@ -56,10 +56,10 @@ import java.net.ProxySelector;
 import java.net.URL;
 import java.net.URLConnection;
 import java.security.DigestOutputStream;
+import java.security.GeneralSecurityException;
 import java.security.InvalidKeyException;
 import java.security.MessageDigest;
 import java.security.NoSuchAlgorithmException;
-import java.security.spec.InvalidKeySpecException;
 import java.text.MessageFormat;
 import java.text.SimpleDateFormat;
 import java.util.ArrayList;
@@ -186,6 +186,19 @@ public class AmazonS3 {
 	/** S3 Bucket Domain. */
 	private final String domain;
 
+	/** Property names used in amazon connection configuration file. */
+	interface Keys {
+		String ACCESS_KEY = "accesskey"; //$NON-NLS-1$
+		String SECRET_KEY = "secretkey"; //$NON-NLS-1$
+		String PASSWORD = "password"; //$NON-NLS-1$
+		String CRYPTO_ALG = "crypto.algorithm"; //$NON-NLS-1$
+		String CRYPTO_VER = "crypto.version"; //$NON-NLS-1$
+		String ACL = "acl"; //$NON-NLS-1$
+		String DOMAIN = "domain"; //$NON-NLS-1$
+		String HTTP_RETRY = "httpclient.retry-max"; //$NON-NLS-1$
+		String TMP_DIR = "tmpdir"; //$NON-NLS-1$
+	}
+
 	/**
 	 * Create a new S3 client for the supplied user information.
 	 * <p>
@@ -219,17 +232,18 @@ public class AmazonS3 {
 	 *
 	 */
 	public AmazonS3(final Properties props) {
-		domain = props.getProperty("domain", "s3.amazonaws.com"); //$NON-NLS-1$ //$NON-NLS-2$
-		publicKey = props.getProperty("accesskey"); //$NON-NLS-1$
+		domain = props.getProperty(Keys.DOMAIN, "s3.amazonaws.com"); //$NON-NLS-1$
+
+		publicKey = props.getProperty(Keys.ACCESS_KEY);
 		if (publicKey == null)
 			throw new IllegalArgumentException(JGitText.get().missingAccesskey);
 
-		final String secret = props.getProperty("secretkey"); //$NON-NLS-1$
+		final String secret = props.getProperty(Keys.SECRET_KEY);
 		if (secret == null)
 			throw new IllegalArgumentException(JGitText.get().missingSecretkey);
 		privateKey = new SecretKeySpec(Constants.encodeASCII(secret), HMAC);
 
-		final String pacl = props.getProperty("acl", "PRIVATE"); //$NON-NLS-1$ //$NON-NLS-2$
+		final String pacl = props.getProperty(Keys.ACL, "PRIVATE"); //$NON-NLS-1$
 		if (StringUtils.equalsIgnoreCase("PRIVATE", pacl)) //$NON-NLS-1$
 			acl = "private"; //$NON-NLS-1$
 		else if (StringUtils.equalsIgnoreCase("PUBLIC", pacl)) //$NON-NLS-1$
@@ -242,26 +256,24 @@ public class AmazonS3 {
 			throw new IllegalArgumentException("Invalid acl: " + pacl); //$NON-NLS-1$
 
 		try {
-			final String cPas = props.getProperty("password"); //$NON-NLS-1$
+			final String cPas = props.getProperty(Keys.PASSWORD);
 			if (cPas != null) {
-				String cAlg = props.getProperty("crypto.algorithm"); //$NON-NLS-1$
+				String cAlg = props.getProperty(Keys.CRYPTO_ALG);
 				if (cAlg == null)
-					cAlg = "PBEWithMD5AndDES"; //$NON-NLS-1$
-				encryption = new WalkEncryption.ObjectEncryptionV2(cAlg, cPas);
+					cAlg = WalkEncryption.ObjectEncryptionJetS3tV2.JETS3T_ALGORITHM;
+				encryption = new WalkEncryption.ObjectEncryptionJetS3tV2(cAlg, cPas);
 			} else {
 				encryption = WalkEncryption.NONE;
 			}
-		} catch (InvalidKeySpecException e) {
-			throw new IllegalArgumentException(JGitText.get().invalidEncryption, e);
-		} catch (NoSuchAlgorithmException e) {
+		} catch (GeneralSecurityException e) {
 			throw new IllegalArgumentException(JGitText.get().invalidEncryption, e);
 		}
 
-		maxAttempts = Integer.parseInt(props.getProperty(
-				"httpclient.retry-max", "3")); //$NON-NLS-1$ //$NON-NLS-2$
+		maxAttempts = Integer
+				.parseInt(props.getProperty(Keys.HTTP_RETRY, "3")); //$NON-NLS-1$
 		proxySelector = ProxySelector.getDefault();
 
-		String tmp = props.getProperty("tmpdir"); //$NON-NLS-1$
+		String tmp = props.getProperty(Keys.TMP_DIR);
 		tmpDir = tmp != null && tmp.length() > 0 ? new File(tmp) : null;
 	}
 
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/WalkEncryption.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/WalkEncryption.java
index e55b984380..e93a2af3ea 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/WalkEncryption.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/WalkEncryption.java
@@ -47,18 +47,16 @@ import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
 import java.net.HttpURLConnection;
-import java.security.InvalidAlgorithmParameterException;
-import java.security.InvalidKeyException;
-import java.security.NoSuchAlgorithmException;
-import java.security.spec.InvalidKeySpecException;
+import java.security.GeneralSecurityException;
+import java.security.spec.AlgorithmParameterSpec;
 import java.text.MessageFormat;
 
 import javax.crypto.Cipher;
 import javax.crypto.CipherInputStream;
 import javax.crypto.CipherOutputStream;
-import javax.crypto.NoSuchPaddingException;
 import javax.crypto.SecretKey;
 import javax.crypto.SecretKeyFactory;
+import javax.crypto.spec.IvParameterSpec;
 import javax.crypto.spec.PBEKeySpec;
 import javax.crypto.spec.PBEParameterSpec;
 
@@ -77,23 +75,29 @@ abstract class WalkEncryption {
 
 	abstract void request(HttpURLConnection u, String prefix);
 
-	abstract void validate(HttpURLConnection u, String p) throws IOException;
+	abstract void validate(HttpURLConnection u, String prefix) throws IOException;
 
-	protected void validateImpl(final HttpURLConnection u, final String p,
+	// TODO mixed ciphers
+	// consider permitting mixed ciphers to facilitate algorithm migration
+	// i.e. user keeps the password, but changes the algorithm
+	// then existing remote entries will still be readable
+	protected void validateImpl(final HttpURLConnection u, final String prefix,
 			final String version, final String name) throws IOException {
 		String v;
 
-		v = u.getHeaderField(p + JETS3T_CRYPTO_VER);
+		v = u.getHeaderField(prefix + JETS3T_CRYPTO_VER);
 		if (v == null)
 			v = ""; //$NON-NLS-1$
 		if (!version.equals(v))
 			throw new IOException(MessageFormat.format(JGitText.get().unsupportedEncryptionVersion, v));
 
-		v = u.getHeaderField(p + JETS3T_CRYPTO_ALG);
+		v = u.getHeaderField(prefix + JETS3T_CRYPTO_ALG);
 		if (v == null)
 			v = ""; //$NON-NLS-1$
-		if (!name.equals(v))
-			throw new IOException(JGitText.get().unsupportedEncryptionAlgorithm + v);
+		// Standard names are not case-sensitive.
+		// http://docs.oracle.com/javase/8/docs/technotes/guides/security/StandardNames.html
+		if (!name.equalsIgnoreCase(v))
+			throw new IOException(MessageFormat.format(JGitText.get().unsupportedEncryptionAlgorithm, v));
 	}
 
 	IOException error(final Throwable why) {
@@ -110,9 +114,9 @@ abstract class WalkEncryption {
 		}
 
 		@Override
-		void validate(final HttpURLConnection u, final String p)
+		void validate(final HttpURLConnection u, final String prefix)
 				throws IOException {
-			validateImpl(u, p, "", ""); //$NON-NLS-1$ //$NON-NLS-2$
+			validateImpl(u, prefix, "", ""); //$NON-NLS-1$ //$NON-NLS-2$
 		}
 
 		@Override
@@ -126,53 +130,132 @@ abstract class WalkEncryption {
 		}
 	}
 
-	static class ObjectEncryptionV2 extends WalkEncryption {
-		private static int ITERATION_COUNT = 5000;
+	// PBEParameterSpec factory for Java (version <= 7).
+	// Does not support AlgorithmParameterSpec.
+	static PBEParameterSpec java7PBEParameterSpec(byte[] salt,
+			int iterationCount) {
+		return new PBEParameterSpec(salt, iterationCount);
+	}
+
+	// PBEParameterSpec factory for Java (version >= 8).
+	// Adds support for AlgorithmParameterSpec.
+	static PBEParameterSpec java8PBEParameterSpec(byte[] salt,
+			int iterationCount, AlgorithmParameterSpec paramSpec) {
+		try {
+			@SuppressWarnings("boxing")
+			PBEParameterSpec instance = PBEParameterSpec.class
+					.getConstructor(byte[].class, int.class,
+							AlgorithmParameterSpec.class)
+					.newInstance(salt, iterationCount, paramSpec);
+			return instance;
+		} catch (Exception e) {
+			throw new RuntimeException(e);
+		}
+	}
+
+	// Current runtime version.
+	// https://docs.oracle.com/javase/7/docs/technotes/guides/versioning/spec/versioning2.html
+	static double javaVersion() {
+		return Double.parseDouble(System.getProperty("java.specification.version")); //$NON-NLS-1$
+	}
+
+	/**
+	 * JetS3t compatibility reference: <a href=
+	 * "https://bitbucket.org/jmurty/jets3t/src/156c00eb160598c2e9937fd6873f00d3190e28ca/src/org/jets3t/service/security/EncryptionUtil.java">
+	 * EncryptionUtil.java</a>
+	 * <p>
+	 * Note: EncryptionUtil is inadequate:
+	 * <li>EncryptionUtil.isCipherAvailableForUse checks encryption only which
+	 * "always works", but in JetS3t both encryption and decryption use non-IV
+	 * aware algorithm parameters for all PBE specs, which breaks in case of AES
+	 * <li>that means that only non-IV algorithms will work round trip in
+	 * JetS3t, such as PBEWithMD5AndDES and PBEWithSHAAndTwofish-CBC
+	 * <li>any AES based algorithms such as "PBE...With...And...AES" will not
+	 * work, since they need proper IV setup
+	 */
+	static class ObjectEncryptionJetS3tV2 extends WalkEncryption {
+
+		static final String JETS3T_VERSION = "2"; //$NON-NLS-1$
+
+		static final String JETS3T_ALGORITHM = "PBEWithMD5AndDES"; //$NON-NLS-1$
+
+		static final int JETS3T_ITERATIONS = 5000;
+
+		static final int JETS3T_KEY_SIZE = 32;
+
+		static final byte[] JETS3T_SALT = { //
+				(byte) 0xA4, (byte) 0x0B, (byte) 0xC8, (byte) 0x34, //
+				(byte) 0xD6, (byte) 0x95, (byte) 0xF3, (byte) 0x13 //
+		};
+
+		// Size 16, see com.sun.crypto.provider.AESConstants.AES_BLOCK_SIZE
+		static final byte[] ZERO_AES_IV = new byte[16];
 
-		private static byte[] salt = { (byte) 0xA4, (byte) 0x0B, (byte) 0xC8,
-				(byte) 0x34, (byte) 0xD6, (byte) 0x95, (byte) 0xF3, (byte) 0x13 };
+		private final String cryptoVer = JETS3T_VERSION;
 
-		private final String algorithmName;
+		private final String cryptoAlg;
 
-		private final SecretKey skey;
+		private final SecretKey secretKey;
 
-		private final PBEParameterSpec aspec;
+		private final AlgorithmParameterSpec paramSpec;
 
-		ObjectEncryptionV2(final String algo, final String key)
-				throws InvalidKeySpecException, NoSuchAlgorithmException {
-			algorithmName = algo;
+		ObjectEncryptionJetS3tV2(final String algo, final String key)
+				throws GeneralSecurityException {
+			cryptoAlg = algo;
+
+			// Standard names are not case-sensitive.
+			// http://docs.oracle.com/javase/8/docs/technotes/guides/security/StandardNames.html
+			String cryptoName = cryptoAlg.toUpperCase();
+
+			if (!cryptoName.startsWith("PBE")) //$NON-NLS-1$
+				throw new GeneralSecurityException(JGitText.get().encryptionOnlyPBE);
+
+			PBEKeySpec keySpec = new PBEKeySpec(key.toCharArray(), JETS3T_SALT, JETS3T_ITERATIONS, JETS3T_KEY_SIZE);
+			secretKey = SecretKeyFactory.getInstance(algo).generateSecret(keySpec);
+
+			// Detect algorithms which require initialization vector.
+			boolean useIV = cryptoName.contains("AES"); //$NON-NLS-1$
+
+			// PBEParameterSpec algorithm parameters are supported from Java 8.
+			boolean isJava8 = javaVersion() >= 1.8;
+
+			if (useIV && isJava8) {
+				// Support IV where possible:
+				// * since JCE provider uses random IV for PBE/AES
+				// * and there is no place to store dynamic IV in JetS3t V2
+				// * we use static IV, and tolerate increased security risk
+				// TODO back port this change to JetS3t V2
+				// See:
+				// https://bitbucket.org/jmurty/jets3t/raw/156c00eb160598c2e9937fd6873f00d3190e28ca/src/org/jets3t/service/security/EncryptionUtil.java
+				// http://cr.openjdk.java.net/~mullan/webrevs/ascarpin/webrev.00/raw_files/new/src/share/classes/com/sun/crypto/provider/PBES2Core.java
+				IvParameterSpec paramIV = new IvParameterSpec(ZERO_AES_IV);
+				paramSpec = java8PBEParameterSpec(JETS3T_SALT, JETS3T_ITERATIONS, paramIV);
+			} else {
+				// Strict legacy JetS3t V2 compatibility, with no IV support.
+				paramSpec = java7PBEParameterSpec(JETS3T_SALT, JETS3T_ITERATIONS);
+			}
 
-			final PBEKeySpec s;
-			s = new PBEKeySpec(key.toCharArray(), salt, ITERATION_COUNT, 32);
-			skey = SecretKeyFactory.getInstance(algo).generateSecret(s);
-			aspec = new PBEParameterSpec(salt, ITERATION_COUNT);
 		}
 
 		@Override
 		void request(final HttpURLConnection u, final String prefix) {
-			u.setRequestProperty(prefix + JETS3T_CRYPTO_VER, "2"); //$NON-NLS-1$
-			u.setRequestProperty(prefix + JETS3T_CRYPTO_ALG, algorithmName);
+			u.setRequestProperty(prefix + JETS3T_CRYPTO_VER, cryptoVer);
+			u.setRequestProperty(prefix + JETS3T_CRYPTO_ALG, cryptoAlg);
 		}
 
 		@Override
-		void validate(final HttpURLConnection u, final String p)
+		void validate(final HttpURLConnection u, final String prefix)
 				throws IOException {
-			validateImpl(u, p, "2", algorithmName); //$NON-NLS-1$
+			validateImpl(u, prefix, cryptoVer, cryptoAlg);
 		}
 
 		@Override
 		OutputStream encrypt(final OutputStream os) throws IOException {
 			try {
-				final Cipher c = Cipher.getInstance(algorithmName);
-				c.init(Cipher.ENCRYPT_MODE, skey, aspec);
-				return new CipherOutputStream(os, c);
-			} catch (NoSuchAlgorithmException e) {
-				throw error(e);
-			} catch (NoSuchPaddingException e) {
-				throw error(e);
-			} catch (InvalidKeyException e) {
-				throw error(e);
-			} catch (InvalidAlgorithmParameterException e) {
+				final Cipher cipher = Cipher.getInstance(cryptoAlg);
+				cipher.init(Cipher.ENCRYPT_MODE, secretKey, paramSpec);
+				return new CipherOutputStream(os, cipher);
+			} catch (GeneralSecurityException e) {
 				throw error(e);
 			}
 		}
@@ -180,16 +263,10 @@ abstract class WalkEncryption {
 		@Override
 		InputStream decrypt(final InputStream in) throws IOException {
 			try {
-				final Cipher c = Cipher.getInstance(algorithmName);
-				c.init(Cipher.DECRYPT_MODE, skey, aspec);
-				return new CipherInputStream(in, c);
-			} catch (NoSuchAlgorithmException e) {
-				throw error(e);
-			} catch (NoSuchPaddingException e) {
-				throw error(e);
-			} catch (InvalidKeyException e) {
-				throw error(e);
-			} catch (InvalidAlgorithmParameterException e) {
+				final Cipher cipher = Cipher.getInstance(cryptoAlg);
+				cipher.init(Cipher.DECRYPT_MODE, secretKey, paramSpec);
+				return new CipherInputStream(in, cipher);
+			} catch (GeneralSecurityException e) {
 				throw error(e);
 			}
 		}