honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Internal: Simplify CryptoService api 

The CryptoService currently has a lot of variations of methods that are
unused. It really only uses sign/unsign, encrypt/decrypt. This change
trims the api down to those needed methods.

Original commit: elastic/x-pack-elasticsearch@92e83efeb7
This commit is contained in:
Ryan Ernst 2016-07-10 14:47:48 -07:00
parent f67b758b47
commit 1c10efc60f
8 changed files with 28 additions and 224 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/SecurityFeatureSet.java
View File

@ -143,7 +143,7 @@ public class SecurityFeatureSet implements XPackFeatureSet {
static boolean systemKeyUsage(CryptoService cryptoService) {
// we can piggy back on the encryption enabled method as it is only enabled if there is a system key
return cryptoService != null && cryptoService.encryptionEnabled();
return cryptoService != null && cryptoService.isEncryptionEnabled();
}
static class Usage extends XPackFeatureSet.Usage {

2
elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/action/filter/SecurityActionFilter.java
View File

@ -191,7 +191,7 @@ public class SecurityActionFilter extends AbstractComponent implements ActionFil
if (response instanceof SearchResponse) {
SearchResponse searchResponse = (SearchResponse) response;
String scrollId = searchResponse.getScrollId();
if (scrollId != null && !cryptoService.signed(scrollId)) {
if (scrollId != null && !cryptoService.isSigned(scrollId)) {
searchResponse.scrollId(cryptoService.sign(scrollId));
}
return response;

61
elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/crypto/CryptoService.java
View File

@ -5,7 +5,6 @@
*/
package org.elasticsearch.xpack.security.crypto;
import javax.crypto.SecretKey;
import java.io.IOException;
/**
@ -26,27 +25,10 @@ public interface CryptoService {
*/
String unsignAndVerify(String text);
/**
* Signs the given text and returns the signed text (original text + signature)
* @param text the string to sign
* @param key the key to sign the text with
* @param systemKey the system key. This is optional and if the key != systemKey then the format of the
* message will change
*/
String sign(String text, SecretKey key, SecretKey systemKey) throws IOException;
/**
* Unsigns the given signed text, verifies the original text with the attached signature and if valid returns
* the unsigned (original) text. If signature verification fails a {@link IllegalArgumentException} is thrown.
* @param text the string to unsign and verify
* @param key the key to unsign the text with
*/
String unsignAndVerify(String text, SecretKey key);
/**
* Checks whether the given text is signed.
*/
boolean signed(String text);
boolean isSigned(String text);
/**
* Encrypts the provided char array and returns the encrypted values in a char array
@ -55,13 +37,6 @@ public interface CryptoService {
*/
char[] encrypt(char[] chars);
/**
* Encrypts the provided byte array and returns the encrypted value
* @param bytes the data to encrypt
* @return encrypted data
*/
byte[] encrypt(byte[] bytes);
/**
* Decrypts the provided char array and returns the plain-text chars
* @param chars the data to decrypt
@ -69,46 +44,16 @@ public interface CryptoService {
*/
char[] decrypt(char[] chars);
/**
* Decrypts the provided char array and returns the plain-text chars
* @param chars the data to decrypt
* @param key the key to decrypt the data with
* @return plaintext chars
*/
char[] decrypt(char[] chars, SecretKey key);
/**
* Decrypts the provided byte array and returns the unencrypted bytes
* @param bytes the bytes to decrypt
* @return plaintext bytes
*/
byte[] decrypt(byte[] bytes);
/**
* Decrypts the provided byte array and returns the unencrypted bytes
* @param bytes the bytes to decrypt
* @param key the key to decrypt the data with
* @return plaintext bytes
*/
byte[] decrypt(byte[] bytes, SecretKey key);
/**
* Checks whether the given chars are encrypted
* @param chars the chars to check if they are encrypted
* @return true is data is encrypted
*/
boolean encrypted(char[] chars);
/**
* Checks whether the given bytes are encrypted
* @param bytes the chars to check if they are encrypted
* @return true is data is encrypted
*/
boolean encrypted(byte[] bytes);
boolean isEncrypted(char[] chars);
/**
* Flag for callers to determine if values will actually be encrypted or returned plaintext
* @return true if values will be encrypted
*/
boolean encryptionEnabled();
boolean isEncryptionEnabled();
}

85
elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/crypto/InternalCryptoService.java
View File

@ -157,23 +157,12 @@ public class InternalCryptoService extends AbstractComponent implements CryptoSe
@Override
public String sign(String text) throws IOException {
return sign(text, this.signingKey, this.systemKey);
}
@Override
public String sign(String text, SecretKey signingKey, @Nullable SecretKey systemKey) throws IOException {
assert signingKey != null;
String sigStr = signInternal(text, signingKey);
return "$$" + sigStr.length() + "$$" + (systemKey == signingKey ? "" : randomKeyBase64) + "$$" + sigStr + text;
}
@Override
public String unsignAndVerify(String signedText) {
return unsignAndVerify(signedText, this.systemKey);
}
@Override
public String unsignAndVerify(String signedText, SecretKey systemKey) {
if (!signedText.startsWith("$$") || signedText.length() < 2) {
throw new IllegalArgumentException("tampered signed text");
}
@ -239,7 +228,7 @@ public class InternalCryptoService extends AbstractComponent implements CryptoSe
}
@Override
public boolean signed(String text) {
public boolean isSigned(String text) {
return SIG_PATTERN.matcher(text).matches();
}
@ -257,33 +246,13 @@ public class InternalCryptoService extends AbstractComponent implements CryptoSe
return ENCRYPTED_TEXT_PREFIX.concat(base64).toCharArray();
}
@Override
public byte[] encrypt(byte[] bytes) {
SecretKey key = this.encryptionKey;
if (key == null) {
logger.warn("encrypt called without a key, returning plain text. run syskeygen and copy same key to all nodes to enable " +
"encryption");
return bytes;
}
byte[] encrypted = encryptInternal(bytes, key);
byte[] prefixed = new byte[ENCRYPTED_BYTE_PREFIX.length + encrypted.length];
System.arraycopy(ENCRYPTED_BYTE_PREFIX, 0, prefixed, 0, ENCRYPTED_BYTE_PREFIX.length);
System.arraycopy(encrypted, 0, prefixed, ENCRYPTED_BYTE_PREFIX.length, encrypted.length);
return prefixed;
}
@Override
public char[] decrypt(char[] chars) {
return decrypt(chars, this.encryptionKey);
}
@Override
public char[] decrypt(char[] chars, SecretKey key) {
if (key == null) {
if (encryptionKey == null) {
return chars;
}
if (!encrypted(chars)) {
if (!isEncrypted(chars)) {
// Not encrypted
return chars;
}
@ -296,41 +265,17 @@ public class InternalCryptoService extends AbstractComponent implements CryptoSe
throw new ElasticsearchException("unable to decode encrypted data", e);
}
byte[] decrypted = decryptInternal(bytes, key);
byte[] decrypted = decryptInternal(bytes, encryptionKey);
return CharArrays.utf8BytesToChars(decrypted);
}
@Override
public byte[] decrypt(byte[] bytes) {
return decrypt(bytes, this.encryptionKey);
}
@Override
public byte[] decrypt(byte[] bytes, SecretKey key) {
if (key == null) {
return bytes;
}
if (!encrypted(bytes)) {
return bytes;
}
byte[] encrypted = Arrays.copyOfRange(bytes, ENCRYPTED_BYTE_PREFIX.length, bytes.length);
return decryptInternal(encrypted, key);
}
@Override
public boolean encrypted(char[] chars) {
public boolean isEncrypted(char[] chars) {
return CharArrays.charsBeginsWith(ENCRYPTED_TEXT_PREFIX, chars);
}
@Override
public boolean encrypted(byte[] bytes) {
return bytesBeginsWith(ENCRYPTED_BYTE_PREFIX, bytes);
}
@Override
public boolean encryptionEnabled() {
public boolean isEncryptionEnabled() {
return this.encryptionKey != null;
}
@ -417,24 +362,6 @@ public class InternalCryptoService extends AbstractComponent implements CryptoSe
return new SecretKeySpec(truncatedDigest, algorithm);
}
private static boolean bytesBeginsWith(byte[] prefix, byte[] bytes) {
if (bytes == null || prefix == null) {
return false;
}
if (prefix.length > bytes.length) {
return false;
}
for (int i = 0; i < prefix.length; i++) {
if (bytes[i] != prefix[i]) {
return false;
}
}
return true;
}
/**
* Provider class for the HmacSHA1 {@link Mac} that provides an optimization by using a thread local instead of calling
* Mac#getInstance and obtaining a lock (in the internals)

2
elasticsearch/x-pack/security/src/test/java/org/elasticsearch/integration/ScrollIdSigningTests.java
View File

@ -108,6 +108,6 @@ public class ScrollIdSigningTests extends SecurityIntegTestCase {
private void assertSigned(String scrollId) {
CryptoService cryptoService = internalCluster().getDataNodeInstance(InternalCryptoService.class);
String message = String.format(Locale.ROOT, "Expected scrollId [%s] to be signed, but was not", scrollId);
assertThat(message, cryptoService.signed(scrollId), is(true));
assertThat(message, cryptoService.isSigned(scrollId), is(true));
}
}

4
elasticsearch/x-pack/security/src/test/java/org/elasticsearch/xpack/security/SecurityFeatureSetTests.java
View File

@ -96,7 +96,7 @@ public class SecurityFeatureSetTests extends ESTestCase {
public void testSystemKeyUsageEnabledByCryptoService() {
final boolean enabled = randomBoolean();
when(cryptoService.encryptionEnabled()).thenReturn(enabled);
when(cryptoService.isEncryptionEnabled()).thenReturn(enabled);
assertThat(SecurityFeatureSet.systemKeyUsage(cryptoService), is(enabled));
}
@ -143,7 +143,7 @@ public class SecurityFeatureSetTests extends ESTestCase {
when(rolesStore.usageStats()).thenReturn(Collections.emptyMap());
}
final boolean useSystemKey = randomBoolean();
when(cryptoService.encryptionEnabled()).thenReturn(useSystemKey);
when(cryptoService.isEncryptionEnabled()).thenReturn(useSystemKey);
List<Realm> realmsList= new ArrayList<>();

4
elasticsearch/x-pack/security/src/test/java/org/elasticsearch/xpack/security/action/filter/SecurityActionFilterTests.java
View File

@ -105,7 +105,7 @@ public class SecurityActionFilterTests extends ESTestCase {
Task task = mock(Task.class);
Authentication authentication = new Authentication(user, new RealmRef("test", "test", "foo"), null);
when(authcService.authenticate("_action", request, SystemUser.INSTANCE)).thenReturn(authentication);
when(cryptoService.signed("signed_scroll_id")).thenReturn(true);
when(cryptoService.isSigned("signed_scroll_id")).thenReturn(true);
when(cryptoService.unsignAndVerify("signed_scroll_id")).thenReturn("scroll_id");
filter.apply(task, "_action", request, listener, chain);
assertThat(request.scrollId(), equalTo("scroll_id"));
@ -122,7 +122,7 @@ public class SecurityActionFilterTests extends ESTestCase {
Task task = mock(Task.class);
Authentication authentication = new Authentication(user, new RealmRef("test", "test", "foo"), null);
when(authcService.authenticate("_action", request, SystemUser.INSTANCE)).thenReturn(authentication);
when(cryptoService.signed("scroll_id")).thenReturn(true);
when(cryptoService.isSigned("scroll_id")).thenReturn(true);
doThrow(sigException).when(cryptoService).unsignAndVerify("scroll_id");
filter.apply(task, "_action", request, listener, chain);
verify(listener).onFailure(isA(ElasticsearchSecurityException.class));

92
elasticsearch/x-pack/security/src/test/java/org/elasticsearch/xpack/security/crypto/InternalCryptoServiceTests.java
View File

@ -7,7 +7,6 @@ package org.elasticsearch.xpack.security.crypto;
import javax.crypto.SecretKey;
import javax.crypto.spec.SecretKeySpec;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.Arrays;
@ -46,7 +45,7 @@ public class InternalCryptoServiceTests extends ESTestCase {
InternalCryptoService service = new InternalCryptoService(settings, env);
String text = randomAsciiOfLength(10);
String signed = service.sign(text);
assertThat(service.signed(signed), is(true));
assertThat(service.isSigned(signed), is(true));
}
public void testSignAndUnsign() throws Exception {
@ -133,7 +132,7 @@ public class InternalCryptoServiceTests extends ESTestCase {
public void testEncryptionAndDecryptionChars() throws Exception {
InternalCryptoService service = new InternalCryptoService(settings, env);
assertThat(service.encryptionEnabled(), is(true));
assertThat(service.isEncryptionEnabled(), is(true));
final char[] chars = randomAsciiOfLengthBetween(0, 1000).toCharArray();
final char[] encrypted = service.encrypt(chars);
assertThat(encrypted, notNullValue());
@ -143,21 +142,9 @@ public class InternalCryptoServiceTests extends ESTestCase {
assertThat(Arrays.equals(chars, decrypted), is(true));
}
public void testEncryptionAndDecryptionBytes() throws Exception {
InternalCryptoService service = new InternalCryptoService(settings, env);
assertThat(service.encryptionEnabled(), is(true));
final byte[] bytes = randomByteArray();
final byte[] encrypted = service.encrypt(bytes);
assertThat(encrypted, notNullValue());
assertThat(Arrays.equals(encrypted, bytes), is(false));
final byte[] decrypted = service.decrypt(encrypted);
assertThat(Arrays.equals(bytes, decrypted), is(true));
}
public void testEncryptionAndDecryptionCharsWithoutKey() throws Exception {
InternalCryptoService service = new InternalCryptoService(Settings.EMPTY, env);
assertThat(service.encryptionEnabled(), is(false));
assertThat(service.isEncryptionEnabled(), is(false));
final char[] chars = randomAsciiOfLengthBetween(0, 1000).toCharArray();
final char[] encryptedChars = service.encrypt(chars);
final char[] decryptedChars = service.decrypt(encryptedChars);
@ -165,68 +152,26 @@ public class InternalCryptoServiceTests extends ESTestCase {
assertThat(chars, equalTo(decryptedChars));
}
public void testEncryptionAndDecryptionBytesWithoutKey() throws Exception {
InternalCryptoService service = new InternalCryptoService(Settings.EMPTY, env);
assertThat(service.encryptionEnabled(), is(false));
final byte[] bytes = randomByteArray();
final byte[] encryptedBytes = service.encrypt(bytes);
final byte[] decryptedBytes = service.decrypt(bytes);
assertThat(bytes, equalTo(encryptedBytes));
assertThat(decryptedBytes, equalTo(encryptedBytes));
}
public void testEncryptionEnabledWithKey() throws Exception {
InternalCryptoService service = new InternalCryptoService(settings, env);
assertThat(service.encryptionEnabled(), is(true));
assertThat(service.isEncryptionEnabled(), is(true));
}
public void testEncryptionEnabledWithoutKey() throws Exception {
InternalCryptoService service = new InternalCryptoService(Settings.EMPTY, env);
assertThat(service.encryptionEnabled(), is(false));
}
public void testChangingAByte() throws Exception {
InternalCryptoService service = new InternalCryptoService(settings, env);
assertThat(service.encryptionEnabled(), is(true));
// We need at least one byte to test changing a byte, otherwise output is always the same
final byte[] bytes = randomByteArray(1);
final byte[] encrypted = service.encrypt(bytes);
assertThat(encrypted, notNullValue());
assertThat(Arrays.equals(encrypted, bytes), is(false));
int tamperedIndex = randomIntBetween(InternalCryptoService.ENCRYPTED_BYTE_PREFIX.length, encrypted.length - 1);
final byte untamperedByte = encrypted[tamperedIndex];
byte tamperedByte = randomByte();
while (tamperedByte == untamperedByte) {
tamperedByte = randomByte();
}
encrypted[tamperedIndex] = tamperedByte;
final byte[] decrypted = service.decrypt(encrypted);
assertThat(Arrays.equals(bytes, decrypted), is(false));
assertThat(service.isEncryptionEnabled(), is(false));
}
public void testEncryptedChar() throws Exception {
InternalCryptoService service = new InternalCryptoService(settings, env);
assertThat(service.encryptionEnabled(), is(true));
assertThat(service.isEncryptionEnabled(), is(true));
assertThat(service.encrypted((char[]) null), is(false));
assertThat(service.encrypted(new char[0]), is(false));
assertThat(service.encrypted(new char[InternalCryptoService.ENCRYPTED_TEXT_PREFIX.length()]), is(false));
assertThat(service.encrypted(InternalCryptoService.ENCRYPTED_TEXT_PREFIX.toCharArray()), is(true));
assertThat(service.encrypted(randomAsciiOfLengthBetween(0, 100).toCharArray()), is(false));
assertThat(service.encrypted(service.encrypt(randomAsciiOfLength(10).toCharArray())), is(true));
}
public void testEncryptedByte() throws Exception {
InternalCryptoService service = new InternalCryptoService(settings, env);
assertThat(service.encryptionEnabled(), is(true));
assertThat(service.encrypted((byte[]) null), is(false));
assertThat(service.encrypted(new byte[0]), is(false));
assertThat(service.encrypted(new byte[InternalCryptoService.ENCRYPTED_BYTE_PREFIX.length]), is(false));
assertThat(service.encrypted(InternalCryptoService.ENCRYPTED_BYTE_PREFIX), is(true));
assertThat(service.encrypted(randomAsciiOfLengthBetween(0, 100).getBytes(StandardCharsets.UTF_8)), is(false));
assertThat(service.encrypted(service.encrypt(randomAsciiOfLength(10).getBytes(StandardCharsets.UTF_8))), is(true));
assertThat(service.isEncrypted((char[]) null), is(false));
assertThat(service.isEncrypted(new char[0]), is(false));
assertThat(service.isEncrypted(new char[InternalCryptoService.ENCRYPTED_TEXT_PREFIX.length()]), is(false));
assertThat(service.isEncrypted(InternalCryptoService.ENCRYPTED_TEXT_PREFIX.toCharArray()), is(true));
assertThat(service.isEncrypted(randomAsciiOfLengthBetween(0, 100).toCharArray()), is(false));
assertThat(service.isEncrypted(service.encrypt(randomAsciiOfLength(10).toCharArray())), is(true));
}
public void testSigningKeyCanBeRecomputedConsistently() {
@ -239,17 +184,4 @@ public class InternalCryptoServiceTests extends ESTestCase {
assertThat(regenerated, equalTo(signingKey));
}
}
private static byte[] randomByteArray() {
return randomByteArray(0);
}
private static byte[] randomByteArray(int min) {
int count = randomIntBetween(min, 1000);
byte[] bytes = new byte[count];
for (int i = 0; i < count; i++) {
bytes[i] = randomByte();
}
return bytes;
}
}