Skip to content

Commit

Permalink
Updates the default Cipher mode to GCM in AesCipherService
Browse files Browse the repository at this point in the history 
Adds tests for each mode
Fix issue where GCM needs a different AlgorithmParameterSpec implementation (previously IvParameterSpec was used)
  • Loading branch information
@bdemers
bdemers committed Nov 6, 2019
1 parent 92bfac6 commit a801878
Show file tree
Hide file tree
Showing 5 changed files with 179 additions and 46 deletions.
7 changes: 7 additions & 0 deletions crypto/cipher/pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,13 @@
<groupId>org.apache.shiro</groupId>
<artifactId>shiro-crypto-core</artifactId>
</dependency>

<dependency>
<groupId>org.bouncycastle</groupId>
<artifactId>bcprov-jdk15on</artifactId>
<version>1.64</version>
<scope>test</scope>
</dependency>
</dependencies>

</project>
23 changes: 19 additions & 4 deletions crypto/cipher/src/main/java/org/apache/shiro/crypto/AesCipherService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,16 @@
*/
package org.apache.shiro.crypto;

import javax.crypto.spec.GCMParameterSpec;
import java.security.spec.AlgorithmParameterSpec;

/**
* {@code CipherService} using the {@code AES} cipher algorithm for all encryption, decryption, and key operations.
* <p/>
* The AES algorithm can support key sizes of {@code 128}, {@code 192} and {@code 256} bits<b>*</b>. This implementation
* defaults to 128 bits.
* <p/>
* Note that this class retains the parent class's default {@link OperationMode#CBC CBC} mode of operation
* Note that this class retains changes the parent class's default {@link OperationMode#CBC CBC} mode to {@link OperationMode#GCM GCM} of operation
* instead of the typical JDK default of {@link OperationMode#ECB ECB}. {@code ECB} should not be used in
* security-sensitive environments because {@code ECB} does not allow for initialization vectors, which are
* considered necessary for strong encryption. See the {@link DefaultBlockCipherService parent class}'s JavaDoc and the
Expand Down Expand Up @@ -59,7 +62,7 @@ public class AesCipherService extends DefaultBlockCipherService {
* </tr>
* <tr>
* <td>{@link #setMode mode}</td>
* <td>{@link OperationMode#CBC CBC}<b>*</b></td>
* <td>{@link OperationMode#GCM GCM}<b>*</b></td>
* </tr>
* <tr>
* <td>{@link #setPaddingScheme paddingScheme}</td>
Expand All @@ -75,16 +78,28 @@ public class AesCipherService extends DefaultBlockCipherService {
* </tr>
* </table>
* <p/>
* <b>*</b> The {@link OperationMode#CBC CBC} operation mode is used instead of the JDK default {@code ECB} to
* <b>*</b> The {@link OperationMode#GCM GCM} operation mode is used instead of the JDK default {@code ECB} to
* ensure strong encryption. {@code ECB} should not be used in security-sensitive environments - see the
* {@link DefaultBlockCipherService DefaultBlockCipherService} class JavaDoc's &quot;Operation Mode&quot; section
* for more.
* <p/>
* <b>**</b>In conjunction with the default {@code CBC} operation mode, initialization vectors are generated by
* <b>**</b>In conjunction with the default {@code GCM} operation mode, initialization vectors are generated by
* default to ensure strong encryption. See the {@link JcaCipherService JcaCipherService} class JavaDoc for more.
*/
public AesCipherService() {
super(ALGORITHM_NAME);
setMode(OperationMode.GCM);
setStreamingMode(OperationMode.GCM);
}

@Override
protected AlgorithmParameterSpec createParameterSpec(byte[] iv, boolean streaming) {

if ((streaming && OperationMode.GCM.name().equals(getStreamingModeName()))
|| (!streaming && OperationMode.GCM.name().equals(getModeName()))) {
return new GCMParameterSpec(getKeySize(), iv);
}

return super.createParameterSpec(iv, streaming);
}
}
9 changes: 7 additions & 2 deletions crypto/cipher/src/main/java/org/apache/shiro/crypto/JcaCipherService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -590,13 +590,18 @@ private javax.crypto.Cipher initNewCipher(int jcaCipherMode, byte[] key, byte[]

javax.crypto.Cipher cipher = newCipherInstance(streaming);
java.security.Key jdkKey = new SecretKeySpec(key, getAlgorithmName());
IvParameterSpec ivSpec = null;
AlgorithmParameterSpec ivSpec = null;

if (iv != null && iv.length > 0) {
ivSpec = new IvParameterSpec(iv);
ivSpec = createParameterSpec(iv, streaming);
}

init(cipher, jcaCipherMode, jdkKey, ivSpec, getSecureRandom());

return cipher;
}

protected AlgorithmParameterSpec createParameterSpec(byte[] iv, boolean streaming) {
return new IvParameterSpec(iv);
}
}
158 changes: 132 additions & 26 deletions crypto/cipher/src/test/groovy/org/apache/shiro/crypto/AesCipherServiceTest.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,12 @@
*/
package org.apache.shiro.crypto

import org.bouncycastle.jce.provider.BouncyCastleProvider

import java.security.Security

import static org.junit.Assert.*;

import org.apache.shiro.codec.CodecSupport
import org.apache.shiro.util.ByteSource
import org.junit.Test
Expand All @@ -29,46 +35,146 @@ import static junit.framework.Assert.*
*
* @since 1.0
*/
public class AesCipherServiceTest {
class AesCipherServiceTest {

private static final String[] PLAINTEXTS = [
"Hello, this is a test.",
"Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua."
];
]

AesCipherServiceTest() {
Security.addProvider(new BouncyCastleProvider())
}

@Test
public void testBlockOperations() {
AesCipherService aes = new AesCipherService();
void testBlockOperations() {
AesCipherService cipher = new AesCipherService()
assertBlock(cipher)
}

byte[] key = aes.generateNewKey().getEncoded();
@Test
void testStreamingOperations() {
AesCipherService cipher = new AesCipherService()
assertStreaming(cipher)
}

for (String plain : PLAINTEXTS) {
byte[] plaintext = CodecSupport.toBytes(plain);
ByteSource ciphertext = aes.encrypt(plaintext, key);
ByteSource decrypted = aes.decrypt(ciphertext.getBytes(), key);
assertTrue(Arrays.equals(plaintext, decrypted.getBytes()));
}
@Test
void testAesGcm() {
assertBlock(OperationMode.GCM)
assertStreaming(OperationMode.GCM)
}

@Test
public void testStreamingOperations() {
void testCcm() {
assertBlock(OperationMode.CCM, PaddingScheme.NONE, 13 * 8) // 13 bytes
assertStreaming(OperationMode.CCM)
}

AesCipherService cipher = new AesCipherService();
byte[] key = cipher.generateNewKey().getEncoded();
@Test
void testCfb() {
assertBlock(OperationMode.CFB)
assertStreaming(OperationMode.CFB)
}

@Test
void testCtr() {
assertBlock(OperationMode.CTR)
assertStreaming(OperationMode.CTR)
}

@Test
void testEax() {
assertBlock(OperationMode.EAX)
assertStreaming(OperationMode.EAX)
}

@Test
void testEcb() {
assertBlock(OperationMode.ECB, PaddingScheme.PKCS5)
}

@Test
void testNone() {
assertBlock((OperationMode) null, null)
}

@Test
void testOcb() {
assertBlock(OperationMode.OCB, PaddingScheme.NONE, 15 * 8) // 15 bytes
assertStreaming(OperationMode.OCB, PaddingScheme.NONE, 16 * 8) // 16 bytes
}

@Test
void testOfb() {
assertBlock(OperationMode.OFB)
assertStreaming(OperationMode.OFB)
}

@Test
void testPcbc() {
assertBlock(OperationMode.PCBC, PaddingScheme.PKCS5)
assertStreaming(OperationMode.PCBC, PaddingScheme.PKCS5)
}

private static assertBlock(OperationMode mode, PaddingScheme scheme = PaddingScheme.NONE, int ivSize = JcaCipherService.DEFAULT_KEY_SIZE) {
AesCipherService cipher = new AesCipherService()
cipher.setInitializationVectorSize(ivSize)

if (mode == null) {
cipher.setModeName(null)
} else {
cipher.setMode(mode)
}

if (scheme == null) {
cipher.setPaddingSchemeName(null)
} else {
cipher.setPaddingScheme(scheme)
}
assertBlock(cipher)
}

private static assertStreaming(OperationMode mode, PaddingScheme scheme = PaddingScheme.NONE, int ivSize = JcaCipherService.DEFAULT_KEY_SIZE) {
AesCipherService cipher = new AesCipherService()
cipher.setInitializationVectorSize(ivSize)

if (mode == null) {
cipher.setStreamingModeName(null)
} else {
cipher.setStreamingMode(mode)
}

if (scheme == null) {
cipher.setStreamingPaddingScheme(null)
} else {
cipher.setStreamingPaddingScheme(scheme)
}
assertBlock(cipher)
}

private static assertBlock(AesCipherService cipher, byte[] key = cipher.generateNewKey().getEncoded()) {
for (String plain : PLAINTEXTS) {
byte[] plaintext = CodecSupport.toBytes(plain);
InputStream plainIn = new ByteArrayInputStream(plaintext);
ByteArrayOutputStream cipherOut = new ByteArrayOutputStream();
cipher.encrypt(plainIn, cipherOut, key);

byte[] ciphertext = cipherOut.toByteArray();
InputStream cipherIn = new ByteArrayInputStream(ciphertext);
ByteArrayOutputStream plainOut = new ByteArrayOutputStream();
cipher.decrypt(cipherIn, plainOut, key);

byte[] decrypted = plainOut.toByteArray();
assertTrue(Arrays.equals(plaintext, decrypted));
byte[] plaintext = CodecSupport.toBytes(plain)
ByteSource ciphertext = cipher.encrypt(plaintext, key)
ByteSource decrypted = cipher.decrypt(ciphertext.getBytes(), key)
assertTrue(Arrays.equals(plaintext, decrypted.getBytes()))
}
}

private static assertStreaming(AesCipherService cipher, byte[] key = cipher.generateNewKey().getEncoded()) {
for (String plain : PLAINTEXTS) {
byte[] plaintext = CodecSupport.toBytes(plain)
InputStream plainIn = new ByteArrayInputStream(plaintext)
ByteArrayOutputStream cipherOut = new ByteArrayOutputStream()
cipher.encrypt(plainIn, cipherOut, key)

byte[] ciphertext = cipherOut.toByteArray()
InputStream cipherIn = new ByteArrayInputStream(ciphertext)
ByteArrayOutputStream plainOut = new ByteArrayOutputStream()
cipher.decrypt(cipherIn, plainOut, key)

byte[] decrypted = plainOut.toByteArray()
assertTrue(Arrays.equals(plaintext, decrypted))
}
}
}
28 changes: 14 additions & 14 deletions web/src/test/java/org/apache/shiro/web/mgt/CookieRememberMeManagerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -125,13 +125,13 @@ public void getRememberedPrincipals() {

//The following base64 string was determined from the log output of the above 'onSuccessfulLogin' test.
//This will have to change any time the PrincipalCollection implementation changes:
final String userPCAesBase64 = "WlD5MLzzZznN3dQ1lPJO/eScSuY245k29aECNmjUs31o7Yu478hWhaM5Sj" +
"jmoe900/72JNu3hcJaPG6Q17Vuz4F8x0kBjbFnPVx4PqzsZYT6yreeS2jwO6OwfI+efqXOKyB2a5KPtnr" +
"7jt5kZsyH38XJISb81cf6xqTGUru8zC+kNqJFz7E5RpO0kraBofS5jhMm45gDVjDRkjgPJAzocVWMtrza" +
"zy67P8eb+kMSBCqGI251JTNAGboVgQ28KjfaAJ/6LXRJUj7kB7CGia7mgRk+hxzEJGDs81at5VOPqODJr" +
"xb8tcIdemFUFIkiYVP9bGs4dP3ECtmw7aNrCzv+84sx3vRFUrd5DbDYpEuE12hF2Y9owDK9sxStbXoF0y" +
"A32dhfGDIqS+agsass0sWn8WX2TM9i8SxrUjiFbxqyIG49HbqGrZp5QLM9IuIwO+TzGfF1FzumQGdwmWT" +
"xkVapw5UESl34YvA615cb+82ue1I=";
final String userPCAesBase64 = "0o6DCfePYTjK4q579qzUFEfkeGRvbBOdKHp2y8/nGAltt1Vz8uW0Z8igeO" +
"Tq/yBmcw25f3Q0ui/Leg3x0iQZWhw9Bbu0mFHmHsGxEd6mPwtUpSegIjyX5c/kZpqnb7QLdajPWiczX8P" +
"Oc2Eku5+8ye1u38Y8uKlklHxcYCPh0pRiDSBxfjPsLaDfOpGbmPjZd4SVg68i/++TvUjqBNJyb+pDix3f" +
"PeuPvReWGcE50iovezVZrEfDOAQ0cZYW35ShypMWOmE9yZnb+p8++StDyAUegryyuIa4pjuRzfMh9D+sN" +
"F9tm/EnDC1VCer2S/a0AGlWAQiM7jrWt1sNinZcKIrvShaWI21tONJt8WhozNS2H72lk4p92rfLNHeglT" +
"xObxIYxLfTI9KiToSe1nYmpQmbBO8x1wWDkWBG//EqRvhgbIfQVqJp12T0fJC1nFuZuVhw/ZanaAZGDk8" +
"7aLMiw3T6FBZtWaspgvfH+0TJrTD8Ra386ekNXNN8JW8=";

Cookie[] cookies = new Cookie[]{
new Cookie(CookieRememberMeManager.DEFAULT_REMEMBER_ME_COOKIE_NAME, userPCAesBase64)
Expand Down Expand Up @@ -165,13 +165,13 @@ public void getRememberedPrincipalsNoMoreDefaultCipher() {

//The following base64 string was determined from the log output of the above 'onSuccessfulLogin' test.
//This will have to change any time the PrincipalCollection implementation changes:
final String userPCAesBase64 = "WlD5MLzzZznN3dQ1lPJO/eScSuY245k29aECNmjUs31o7Yu478hWhaM5Sj" +
"jmoe900/72JNu3hcJaPG6Q17Vuz4F8x0kBjbFnPVx4PqzsZYT6yreeS2jwO6OwfI+efqXOKyB2a5KPtnr" +
"7jt5kZsyH38XJISb81cf6xqTGUru8zC+kNqJFz7E5RpO0kraBofS5jhMm45gDVjDRkjgPJAzocVWMtrza" +
"zy67P8eb+kMSBCqGI251JTNAGboVgQ28KjfaAJ/6LXRJUj7kB7CGia7mgRk+hxzEJGDs81at5VOPqODJr" +
"xb8tcIdemFUFIkiYVP9bGs4dP3ECtmw7aNrCzv+84sx3vRFUrd5DbDYpEuE12hF2Y9owDK9sxStbXoF0y" +
"A32dhfGDIqS+agsass0sWn8WX2TM9i8SxrUjiFbxqyIG49HbqGrZp5QLM9IuIwO+TzGfF1FzumQGdwmWT" +
"xkVapw5UESl34YvA615cb+82ue1I=";
final String userPCAesBase64 = "0o6DCfePYTjK4q579qzUFEfkeGRvbBOdKHp2y8/nGAltt1Vz8uW0Z8igeO" +
"Tq/yBmcw25f3Q0ui/Leg3x0iQZWhw9Bbu0mFHmHsGxEd6mPwtUpSegIjyX5c/kZpqnb7QLdajPWiczX8P" +
"Oc2Eku5+8ye1u38Y8uKlklHxcYCPh0pRiDSBxfjPsLaDfOpGbmPjZd4SVg68i/++TvUjqBNJyb+pDix3f" +
"PeuPvReWGcE50iovezVZrEfDOAQ0cZYW35ShypMWOmE9yZnb+p8++StDyAUegryyuIa4pjuRzfMh9D+sN" +
"F9tm/EnDC1VCer2S/a0AGlWAQiM7jrWt1sNinZcKIrvShaWI21tONJt8WhozNS2H72lk4p92rfLNHeglT" +
"xObxIYxLfTI9KiToSe1nYmpQmbBO8x1wWDkWBG//EqRvhgbIfQVqJp12T0fJC1nFuZuVhw/ZanaAZGDk8" +
"7aLMiw3T6FBZtWaspgvfH+0TJrTD8Ra386ekNXNN8JW8=";

Cookie[] cookies = new Cookie[]{
new Cookie(CookieRememberMeManager.DEFAULT_REMEMBER_ME_COOKIE_NAME, userPCAesBase64)
Expand Down

0 comments on commit a801878

Please sign in to comment.