Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added support for encryption algorithms for symmetric keys #17209

Merged
merged 13 commits into from
Nov 13, 2020
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -346,6 +346,10 @@
<!-- InvalidKeyException is not a runtime exception, issue link: https://github.com/Azure/azure-sdk-for-java/issues/5178 -->
<suppress checks="com.azure.tools.checkstyle.checks.ThrowFromClientLogger"
files="com.azure.security.keyvault.keys.cryptography.AesCbc.java"/>
<suppress checks="com.azure.tools.checkstyle.checks.ThrowFromClientLogger"
files="com.azure.security.keyvault.keys.cryptography.AesCbcPad.java"/>
<suppress checks="com.azure.tools.checkstyle.checks.ThrowFromClientLogger"
files="com.azure.security.keyvault.keys.cryptography.AesGcm.java"/>

<!-- MSAL extension temporarily living in our package. Will not take dependency on azure-core once migrated to MSAL repo -->
<suppress checks="com.azure.tools.checkstyle.checks.ThrowFromClientLogger"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2383,7 +2383,7 @@
<Method name="getUserName" />
<Bug pattern="NM_CONFUSING" />
</Match>

<!-- Disabling false positives in azure-core -->
<!-- This Issue has been resolved as per spotbugs's recommended solution but the static checker still flags it, its a known issue with this rule. -->
<Match>
Expand Down Expand Up @@ -2414,4 +2414,13 @@
<Method name="~(get|post)" />
<Bug pattern="RCN_REDUNDANT_NULLCHECK_WOULD_HAVE_BEEN_A_NPE" />
</Match>

<!-- Service accepts null byte arrays as input parameters -->
<Match>
<Or>
<Class name="com.azure.security.keyvault.keys.cryptography.options.DecryptOptions" />
<Class name="com.azure.security.keyvault.keys.cryptography.options.EncryptOptions" />
</Or>
<Bug pattern="PZLA_PREFER_ZERO_LENGTH_ARRAYS" />
</Match>
</FindBugsFilter>
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

package com.azure.security.keyvault.keys.cryptography;

class Aes128CbcPad extends AesCbcPad {
private static final int KEY_SIZE = 128;
public static final String ALGORITHM_NAME = "A128CBCPAD";

Aes128CbcPad() {
super(ALGORITHM_NAME, KEY_SIZE);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

package com.azure.security.keyvault.keys.cryptography;

class Aes128Gcm extends AesGcm {
private static final int KEY_SIZE = 128;
public static final String ALGORITHM_NAME = "A128GCM";

Aes128Gcm() {
super(ALGORITHM_NAME, KEY_SIZE);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,13 @@
import java.security.Provider;
import java.util.Arrays;

class AesKw128 extends AesKw {
class Aes128Kw extends AesKw {

public static final String ALGORITHM_NAME = "A128KW";

static final int KEY_SIZE_IN_BYTES = 128 >> 3;

AesKw128() {
Aes128Kw() {
super(ALGORITHM_NAME);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

package com.azure.security.keyvault.keys.cryptography;

class Aes192CbcPad extends AesCbcPad {
private static final int KEY_SIZE = 192;
public static final String ALGORITHM_NAME = "A192CBCPAD";

Aes192CbcPad() {
super(ALGORITHM_NAME, KEY_SIZE);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

package com.azure.security.keyvault.keys.cryptography;

class Aes192Gcm extends AesGcm {
private static final int KEY_SIZE = 192;
public static final String ALGORITHM_NAME = "A192GCM";

Aes192Gcm() {
super(ALGORITHM_NAME, KEY_SIZE);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,13 @@
import java.security.Provider;
import java.util.Arrays;

class AesKw192 extends AesKw {
class Aes192Kw extends AesKw {

public static final String ALGORITHM_NAME = "A192KW";

static final int KEY_SIZE_IN_BYTES = 192 >> 3;

AesKw192() {
Aes192Kw() {
super(ALGORITHM_NAME);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

package com.azure.security.keyvault.keys.cryptography;

class Aes256CbcPad extends AesCbcPad {
private static final int KEY_SIZE = 256;
public static final String ALGORITHM_NAME = "A256CBCPAD";

Aes256CbcPad() {
super(ALGORITHM_NAME, KEY_SIZE);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

package com.azure.security.keyvault.keys.cryptography;

class Aes256Gcm extends AesGcm {
private static final int KEY_SIZE = 256;
public static final String ALGORITHM_NAME = "A256GCM";

Aes256Gcm() {
super(ALGORITHM_NAME, KEY_SIZE);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,13 @@
import java.security.Provider;
import java.util.Arrays;

class AesKw256 extends AesKw {
class Aes256Kw extends AesKw {

public static final String ALGORITHM_NAME = "A256KW";

static final int KEY_SIZE_IN_BYTES = 256 >> 3;

AesKw256() {
Aes256Kw() {
super(ALGORITHM_NAME);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,25 +16,30 @@
import java.util.Arrays;

abstract class AesCbc extends SymmetricEncryptionAlgorithm {

final int keySizeInBytes;
final int keySize;
static class AesCbcDecryptor implements ICryptoTransform {

protected AesCbc(String name, int size) {
super(name);

keySize = size;
keySizeInBytes = size >> 3;
}

static class AesCbcEncryptor implements ICryptoTransform {
private final Cipher cipher;

AesCbcDecryptor(byte[] key, byte[] iv, Provider provider)
throws NoSuchAlgorithmException, NoSuchPaddingException, InvalidKeyException,
InvalidAlgorithmParameterException {
AesCbcEncryptor(byte[] key, byte[] iv, Provider provider) throws NoSuchAlgorithmException,
NoSuchPaddingException, InvalidKeyException, InvalidAlgorithmParameterException {

// Create the cipher using the Provider if specified
if (provider == null) {
cipher = Cipher.getInstance("AES/CBC/PKCS5Padding");
cipher = Cipher.getInstance("AES/CBC/NoPadding");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lusitanian should this be zero-padding? When we spoke about .NET, you said zero-padding was what the service was using.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

...if this is right, I'll have to change .NET's implementation to NoPadding as well.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Going from the name only it made sense to me that we should not use padding for AES-CBC and use padding for AES-CBC-PAD. Is that not the case?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I asked MHSM about it, the reply was that zero-padding seems to be closer. I'm honestly not sure. If you're writing tests, maybe try it against the service and see what it does with CBC vs CBCPAD.

} else {
cipher = Cipher.getInstance("AES/CBC/PKCS5Padding", provider);
cipher = Cipher.getInstance("AES/CBC/NoPadding", provider);
}

cipher.init(Cipher.DECRYPT_MODE, new SecretKeySpec(key, "AES"), new IvParameterSpec(iv));
cipher.init(Cipher.ENCRYPT_MODE, new SecretKeySpec(key, "AES"), new IvParameterSpec(iv));
}

@Override
Expand All @@ -43,22 +48,20 @@ public byte[] doFinal(byte[] plaintext) throws IllegalBlockSizeException, BadPad
}
}

static class AesCbcEncryptor implements ICryptoTransform {

static class AesCbcDecryptor implements ICryptoTransform {
private final Cipher cipher;

AesCbcEncryptor(byte[] key, byte[] iv, Provider provider)
throws NoSuchAlgorithmException, NoSuchPaddingException, InvalidKeyException,
InvalidAlgorithmParameterException {
AesCbcDecryptor(byte[] key, byte[] iv, Provider provider) throws NoSuchAlgorithmException,
NoSuchPaddingException, InvalidKeyException, InvalidAlgorithmParameterException {

// Create the cipher using the Provider if specified
if (provider == null) {
cipher = Cipher.getInstance("AES/CBC/PKCS5Padding");
cipher = Cipher.getInstance("AES/CBC/NoPadding");
} else {
cipher = Cipher.getInstance("AES/CBC/PKCS5Padding", provider);
cipher = Cipher.getInstance("AES/CBC/NoPadding", provider);
}

cipher.init(Cipher.ENCRYPT_MODE, new SecretKeySpec(key, "AES"), new IvParameterSpec(iv));
cipher.init(Cipher.DECRYPT_MODE, new SecretKeySpec(key, "AES"), new IvParameterSpec(iv));
}

@Override
Expand All @@ -67,22 +70,12 @@ public byte[] doFinal(byte[] plaintext) throws IllegalBlockSizeException, BadPad
}
}

protected AesCbc(String name, int size) {
super(name);
keySize = size;
keySizeInBytes = size >> 3;
}

@Override
public ICryptoTransform createEncryptor(byte[] key, byte[] iv, byte[] authenticationData)
throws InvalidKeyException, NoSuchAlgorithmException, NoSuchPaddingException,
InvalidAlgorithmParameterException {

if (key == null || key.length < keySizeInBytes) {
throw new InvalidKeyException("key must be at least " + keySize + " bits in length");
}

return new AesCbcEncryptor(Arrays.copyOfRange(key, 0, keySizeInBytes), iv, null);
return createEncryptor(key, iv, authenticationData, null);
vcolin7 marked this conversation as resolved.
Show resolved Hide resolved
}

@Override
Expand All @@ -91,7 +84,7 @@ public ICryptoTransform createEncryptor(byte[] key, byte[] iv, byte[] authentica
InvalidAlgorithmParameterException {

if (key == null || key.length < keySizeInBytes) {
throw new InvalidKeyException("key must be at least " + keySize + " bits in length");
throw new InvalidKeyException("Key must be at least " + keySize + " bits in length.");
}

return new AesCbcEncryptor(Arrays.copyOfRange(key, 0, keySizeInBytes), iv, provider);
Expand All @@ -102,11 +95,7 @@ public ICryptoTransform createDecryptor(byte[] key, byte[] iv, byte[] authentica
throws InvalidKeyException, NoSuchAlgorithmException, NoSuchPaddingException,
InvalidAlgorithmParameterException {

if (key == null || key.length < keySizeInBytes) {
throw new InvalidKeyException("key must be at least " + keySize + " bits in length");
}

return new AesCbcDecryptor(Arrays.copyOfRange(key, 0, keySizeInBytes), iv, null);
return createDecryptor(key, iv, authenticationData, authenticationTag, null);
}

@Override
Expand All @@ -116,7 +105,7 @@ public ICryptoTransform createDecryptor(byte[] key, byte[] iv, byte[] authentica
InvalidAlgorithmParameterException {

if (key == null || key.length < keySizeInBytes) {
throw new InvalidKeyException("key must be at least " + keySize + " bits in length");
throw new InvalidKeyException("Key must be at least " + keySize + " bits in length.");
}

return new AesCbcDecryptor(Arrays.copyOfRange(key, 0, keySizeInBytes), iv, provider);
Expand Down
Loading