-
Notifications
You must be signed in to change notification settings - Fork 24.9k
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
Packet-based enc/dec cipher streams #49896
Merged
albertzaharovits
merged 63 commits into
elastic:repository-encrypted-client-side
from
albertzaharovits:packet-based-cipherstream-2
Jan 6, 2020
Merged
Changes from 62 commits
Commits
Show all changes
63 commits
Select commit
Hold shift + click to select a range
92a3b34
Polished main
albertzaharovits d07c05f
First successful tests
albertzaharovits de603a7
More tests
albertzaharovits 7cc62e0
More tests
albertzaharovits cbd3c50
BufferOnMarkInputStreamBug
albertzaharovits 9eb9bcf
A few bugs...
albertzaharovits 5919a11
BufferOnMark bug
albertzaharovits 47f6aea
More more more bugs!
albertzaharovits 8263062
Mad tests
albertzaharovits cf97ba2
Manic testing
albertzaharovits 3c82ba9
BufferOnMarkInputStreamTests completed
albertzaharovits 76d8271
Checkstyle
albertzaharovits 6b30902
Merge branch 'repository-encrypted-client-side' into packet-based-cip…
albertzaharovits 4e9778e
BufferOnMarkInputStream javadocs
albertzaharovits 24d6d27
merge fallout
albertzaharovits 3cd79bd
PrefixInputStream tests
albertzaharovits c610fe8
WIP
albertzaharovits e4f8564
CountingInputStreamTests
albertzaharovits c816c45
Renaming and more javadocs
albertzaharovits 29c484b
Refactor ChainingInputStream
albertzaharovits db5f58e
Scarce EncryptionPacketsInputStream javadocs
albertzaharovits d6dc875
ChainingInputStream polishing and tests
albertzaharovits 7cb48f6
ChainingInputStreamTests
albertzaharovits 83e028b
ChainingInputStreamTests without mark/reset
albertzaharovits 26a624f
ChainingInputStreamTests mark/reset
albertzaharovits aeb6698
More tests
albertzaharovits 2939289
WIP
albertzaharovits 76678a6
DecryptionPacketsInputStream tests
albertzaharovits f44b97c
Tests done!
albertzaharovits 016164a
More javadocs
albertzaharovits 5b26ff6
Merge branch 'repository-encrypted-client-side' into packet-based-cip…
albertzaharovits a0751c6
Update x-pack/plugin/repository-encrypted/src/main/java/org/elasticse…
albertzaharovits 92e177f
Update x-pack/plugin/repository-encrypted/src/main/java/org/elasticse…
albertzaharovits 5d9321a
Update x-pack/plugin/repository-encrypted/src/main/java/org/elasticse…
albertzaharovits c231486
Tim's review WIP
albertzaharovits bda96b6
ChainingInputStream javadocs
albertzaharovits cb6006d
Logging on component close
albertzaharovits fad9eb4
Merge branch 'repository-encrypted-client-side' into packet-based-cip…
albertzaharovits 65f0adb
Nit
albertzaharovits b40d999
No Randomness in ChainingInputStreamTests
albertzaharovits cb7bc1c
Update x-pack/plugin/repository-encrypted/src/test/java/org/elasticse…
albertzaharovits 7acaf54
Tim's review WIP before mark/reset review
albertzaharovits dfeea83
Almost WIP
albertzaharovits da29e2f
Review complete
albertzaharovits 5e7269b
Update x-pack/plugin/repository-encrypted/src/main/java/org/elasticse…
albertzaharovits 14ee4aa
IV position
albertzaharovits 7ee63c0
Update x-pack/plugin/repository-encrypted/src/main/java/org/elasticse…
albertzaharovits 74f38b2
move mark supported before mark/reset implementations
albertzaharovits fee2d79
Package-protected instead of protected for final classes
albertzaharovits eda1fe0
RemainingPrefixByteCount
albertzaharovits e85aefe
Update x-pack/plugin/repository-encrypted/src/main/java/org/elasticse…
albertzaharovits 8a0773a
no iv instance variable
albertzaharovits 07d7ac8
Nit
albertzaharovits fd10914
Exception messages
albertzaharovits 2e41d4f
Fix tests with exception names
albertzaharovits 97f5917
Test for reader of fewer bytes
albertzaharovits 4fd6dcc
Adjust counting input stream docs
albertzaharovits 3d1daf4
RingBuffer
albertzaharovits 4fcd49d
WIP
albertzaharovits 9ef136e
WIP
albertzaharovits cb966b2
More javadoc to the ring buffer inner
albertzaharovits 0f9f77c
Small test polishing
albertzaharovits 08fb26c
Merge branch 'repository-encrypted-client-side' into packet-based-cip…
elasticmachine File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
evaluationDependsOn(xpackModule('core')) | ||
|
||
apply plugin: 'elasticsearch.esplugin' | ||
esplugin { | ||
name 'repository-encrypted' | ||
description 'Elasticsearch Expanded Pack Plugin - client-side encrypted repositories.' | ||
classname 'org.elasticsearch.repositories.encrypted.EncryptedRepositoryPlugin' | ||
extendedPlugins = ['x-pack-core'] | ||
} | ||
|
||
integTest.enabled = false |
546 changes: 546 additions & 0 deletions
546
...ypted/src/main/java/org/elasticsearch/repositories/encrypted/BufferOnMarkInputStream.java
Large diffs are not rendered by default.
Oops, something went wrong.
375 changes: 375 additions & 0 deletions
375
...encrypted/src/main/java/org/elasticsearch/repositories/encrypted/ChainingInputStream.java
Large diffs are not rendered by default.
Oops, something went wrong.
115 changes: 115 additions & 0 deletions
115
...encrypted/src/main/java/org/elasticsearch/repositories/encrypted/CountingInputStream.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,115 @@ | ||
/* | ||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
* or more contributor license agreements. Licensed under the Elastic License; | ||
* you may not use this file except in compliance with the Elastic License. | ||
*/ | ||
package org.elasticsearch.repositories.encrypted; | ||
|
||
import java.io.IOException; | ||
import java.io.InputStream; | ||
import java.util.Objects; | ||
|
||
/** | ||
* A {@code CountingInputStream} wraps another input stream and counts the number of bytes | ||
* that have been read or skipped. | ||
* <p> | ||
* This input stream does no buffering on its own and only supports {@code mark} and | ||
* {@code reset} if the underlying wrapped stream supports it. | ||
* <p> | ||
* If the stream supports {@code mark} and {@code reset} the byte count is also reset to the | ||
* value that it had on the last {@code mark} call, thereby not counting the same bytes twice. | ||
* <p> | ||
* If the {@code closeSource} constructor argument is {@code true}, closing this | ||
* stream will also close the wrapped input stream. Apart from closing the wrapped | ||
* stream in this case, the {@code close} method does nothing else. | ||
*/ | ||
public final class CountingInputStream extends InputStream { | ||
|
||
private final InputStream source; | ||
private final boolean closeSource; | ||
long count; // package-protected for tests | ||
long mark; // package-protected for tests | ||
boolean closed; // package-protected for tests | ||
|
||
/** | ||
* Wraps another input stream, counting the number of bytes read. | ||
* | ||
* @param source the input stream to be wrapped | ||
* @param closeSource {@code true} if closing this stream will also close the wrapped stream | ||
*/ | ||
public CountingInputStream(InputStream source, boolean closeSource) { | ||
this.source = Objects.requireNonNull(source); | ||
this.closeSource = closeSource; | ||
this.count = 0L; | ||
this.mark = -1L; | ||
this.closed = false; | ||
} | ||
|
||
/** Returns the number of bytes read. */ | ||
public long getCount() { | ||
return count; | ||
} | ||
|
||
@Override | ||
public int read() throws IOException { | ||
int result = source.read(); | ||
if (result != -1) { | ||
count++; | ||
} | ||
return result; | ||
} | ||
|
||
@Override | ||
public int read(byte[] b, int off, int len) throws IOException { | ||
int result = source.read(b, off, len); | ||
if (result != -1) { | ||
count += result; | ||
} | ||
return result; | ||
} | ||
|
||
@Override | ||
public long skip(long n) throws IOException { | ||
long result = source.skip(n); | ||
count += result; | ||
return result; | ||
} | ||
|
||
@Override | ||
public int available() throws IOException { | ||
return source.available(); | ||
} | ||
|
||
@Override | ||
public boolean markSupported() { | ||
return source.markSupported(); | ||
} | ||
|
||
@Override | ||
public synchronized void mark(int readlimit) { | ||
source.mark(readlimit); | ||
mark = count; | ||
} | ||
|
||
@Override | ||
public synchronized void reset() throws IOException { | ||
if (false == source.markSupported()) { | ||
throw new IOException("Mark not supported"); | ||
} | ||
if (mark == -1L) { | ||
throw new IOException("Mark not set"); | ||
} | ||
count = mark; | ||
source.reset(); | ||
} | ||
|
||
@Override | ||
public void close() throws IOException { | ||
if (false == closed) { | ||
closed = true; | ||
if (closeSource) { | ||
source.close(); | ||
} | ||
} | ||
} | ||
} |
173 changes: 173 additions & 0 deletions
173
.../src/main/java/org/elasticsearch/repositories/encrypted/DecryptionPacketsInputStream.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,173 @@ | ||
/* | ||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
* or more contributor license agreements. Licensed under the Elastic License; | ||
* you may not use this file except in compliance with the Elastic License. | ||
*/ | ||
package org.elasticsearch.repositories.encrypted; | ||
|
||
import javax.crypto.BadPaddingException; | ||
import javax.crypto.Cipher; | ||
import javax.crypto.IllegalBlockSizeException; | ||
import javax.crypto.NoSuchPaddingException; | ||
import javax.crypto.SecretKey; | ||
import javax.crypto.ShortBufferException; | ||
import javax.crypto.spec.GCMParameterSpec; | ||
import java.io.ByteArrayInputStream; | ||
import java.io.IOException; | ||
import java.io.InputStream; | ||
import java.nio.ByteBuffer; | ||
import java.nio.ByteOrder; | ||
import java.security.InvalidAlgorithmParameterException; | ||
import java.security.InvalidKeyException; | ||
import java.security.NoSuchAlgorithmException; | ||
import java.util.Objects; | ||
|
||
import static org.elasticsearch.repositories.encrypted.EncryptedRepository.GCM_IV_LENGTH_IN_BYTES; | ||
import static org.elasticsearch.repositories.encrypted.EncryptedRepository.GCM_TAG_LENGTH_IN_BYTES; | ||
|
||
/** | ||
* A {@code DecryptionPacketsInputStream} wraps an encrypted input stream and decrypts | ||
* its contents. This is designed (and tested) to decrypt only the encryption format that | ||
* {@link EncryptionPacketsInputStream} generates. No decrypted bytes are returned before | ||
* they are authenticated. | ||
* <p> | ||
* The same parameters, namely {@code secretKey}, {@code nonce} and {@code packetLength}, | ||
* that have been used during encryption must also be used for decryption, otherwise | ||
* decryption will fail. | ||
* <p> | ||
* This implementation buffers the encrypted packet in memory. The maximum packet size it can | ||
* accommodate is {@link EncryptedRepository#MAX_PACKET_LENGTH_IN_BYTES}. | ||
* <p> | ||
* This implementation does not support {@code mark} and {@code reset}. | ||
* <p> | ||
* The {@code close} call will close the decryption input stream and any subsequent {@code read}, | ||
* {@code skip}, {@code available} and {@code reset} calls will throw {@code IOException}s. | ||
* <p> | ||
* This is NOT thread-safe, multiple threads sharing a single instance must synchronize access. | ||
* | ||
* @see EncryptionPacketsInputStream | ||
*/ | ||
public final class DecryptionPacketsInputStream extends ChainingInputStream { | ||
|
||
private final InputStream source; | ||
private final SecretKey secretKey; | ||
private final int nonce; | ||
private final int packetLength; | ||
private final byte[] packetBuffer; | ||
|
||
private boolean hasNext; | ||
private long counter; | ||
|
||
/** | ||
* Computes and returns the length of the plaintext given the {@code ciphertextLength} and the {@code packetLength} | ||
* used during encryption. | ||
* Each ciphertext packet is prepended by the Initilization Vector and has the Authentication Tag appended. | ||
* Decryption is 1:1, and the ciphertext is not padded, but stripping away the IV and the AT amounts to a shorter | ||
* plaintext compared to the ciphertext. | ||
* | ||
* @see EncryptionPacketsInputStream#getEncryptionLength(long, int) | ||
*/ | ||
public static long getDecryptionLength(long ciphertextLength, int packetLength) { | ||
long encryptedPacketLength = packetLength + GCM_TAG_LENGTH_IN_BYTES + GCM_IV_LENGTH_IN_BYTES; | ||
long completePackets = ciphertextLength / encryptedPacketLength; | ||
long decryptedSize = completePackets * packetLength; | ||
if (ciphertextLength % encryptedPacketLength != 0) { | ||
decryptedSize += (ciphertextLength % encryptedPacketLength) - GCM_IV_LENGTH_IN_BYTES - GCM_TAG_LENGTH_IN_BYTES; | ||
} | ||
return decryptedSize; | ||
} | ||
|
||
public DecryptionPacketsInputStream(InputStream source, SecretKey secretKey, int nonce, int packetLength) { | ||
this.source = Objects.requireNonNull(source); | ||
this.secretKey = Objects.requireNonNull(secretKey); | ||
this.nonce = nonce; | ||
if (packetLength <= 0 || packetLength >= EncryptedRepository.MAX_PACKET_LENGTH_IN_BYTES) { | ||
throw new IllegalArgumentException("Invalid packet length [" + packetLength + "]"); | ||
} | ||
this.packetLength = packetLength; | ||
this.packetBuffer = new byte[packetLength + GCM_TAG_LENGTH_IN_BYTES]; | ||
this.hasNext = true; | ||
this.counter = EncryptedRepository.PACKET_START_COUNTER; | ||
} | ||
|
||
@Override | ||
InputStream nextComponent(InputStream currentComponentIn) throws IOException { | ||
if (currentComponentIn != null && currentComponentIn.read() != -1) { | ||
throw new IllegalStateException("Stream for previous packet has not been fully processed"); | ||
} | ||
if (false == hasNext) { | ||
return null; | ||
} | ||
PrefixInputStream packetInputStream = new PrefixInputStream(source, | ||
packetLength + GCM_IV_LENGTH_IN_BYTES + GCM_TAG_LENGTH_IN_BYTES, | ||
false); | ||
int currentPacketLength = decrypt(packetInputStream); | ||
// only the last packet is shorter, so this must be the last packet | ||
if (currentPacketLength != packetLength) { | ||
hasNext = false; | ||
} | ||
return new ByteArrayInputStream(packetBuffer, 0, currentPacketLength); | ||
} | ||
|
||
@Override | ||
public boolean markSupported() { | ||
return false; | ||
} | ||
|
||
@Override | ||
public void mark(int readlimit) { | ||
} | ||
|
||
@Override | ||
public void reset() throws IOException { | ||
throw new IOException("Mark/reset not supported"); | ||
} | ||
|
||
private int decrypt(PrefixInputStream packetInputStream) throws IOException { | ||
// read only the IV prefix into the packet buffer | ||
int ivLength = packetInputStream.readNBytes(packetBuffer, 0, GCM_IV_LENGTH_IN_BYTES); | ||
if (ivLength != GCM_IV_LENGTH_IN_BYTES) { | ||
throw new IOException("Packet heading IV error. Unexpected length [" + ivLength + "]."); | ||
} | ||
// extract the nonce and the counter from the packet IV | ||
ByteBuffer ivBuffer = ByteBuffer.wrap(packetBuffer, 0, GCM_IV_LENGTH_IN_BYTES).order(ByteOrder.LITTLE_ENDIAN); | ||
int packetIvNonce = ivBuffer.getInt(0); | ||
long packetIvCounter = ivBuffer.getLong(Integer.BYTES); | ||
if (packetIvNonce != nonce) { | ||
throw new IOException("Packet nonce mismatch. Expecting [" + nonce + "], but got [" + packetIvNonce + "]."); | ||
} | ||
if (packetIvCounter != counter) { | ||
throw new IOException("Packet counter mismatch. Expecting [" + counter + "], but got [" + packetIvCounter + "]."); | ||
} | ||
// counter increment for the subsequent packet | ||
counter++; | ||
// counter wrap around | ||
if (counter == EncryptedRepository.PACKET_START_COUNTER) { | ||
throw new IOException("Maximum packet count limit exceeded"); | ||
} | ||
// cipher used to decrypt only the current packetInputStream | ||
Cipher packetCipher = getPacketDecryptionCipher(packetBuffer); | ||
// read the rest of the packet, reusing the packetBuffer | ||
int packetLength = packetInputStream.readNBytes(packetBuffer, 0, packetBuffer.length); | ||
if (packetLength < GCM_TAG_LENGTH_IN_BYTES) { | ||
throw new IOException("Encrypted packet is too short"); | ||
} | ||
try { | ||
// in-place decryption of the whole packet and return decrypted length | ||
return packetCipher.doFinal(packetBuffer, 0, packetLength, packetBuffer); | ||
} catch (ShortBufferException | IllegalBlockSizeException | BadPaddingException e) { | ||
throw new IOException("Exception during packet decryption", e); | ||
} | ||
} | ||
|
||
private Cipher getPacketDecryptionCipher(byte[] packet) throws IOException { | ||
GCMParameterSpec gcmParameterSpec = new GCMParameterSpec(GCM_TAG_LENGTH_IN_BYTES * Byte.SIZE, packet, 0, GCM_IV_LENGTH_IN_BYTES); | ||
try { | ||
Cipher packetCipher = Cipher.getInstance(EncryptedRepository.GCM_ENCRYPTION_SCHEME); | ||
packetCipher.init(Cipher.DECRYPT_MODE, secretKey, gcmParameterSpec); | ||
return packetCipher; | ||
} catch (NoSuchAlgorithmException | NoSuchPaddingException | InvalidKeyException | InvalidAlgorithmParameterException e) { | ||
throw new IOException("Exception during packet cipher initialisation", e); | ||
} | ||
} | ||
} |
16 changes: 16 additions & 0 deletions
16
...encrypted/src/main/java/org/elasticsearch/repositories/encrypted/EncryptedRepository.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
/* | ||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
* or more contributor license agreements. Licensed under the Elastic License; | ||
* you may not use this file except in compliance with the Elastic License. | ||
*/ | ||
|
||
package org.elasticsearch.repositories.encrypted; | ||
|
||
public class EncryptedRepository { | ||
static final int GCM_TAG_LENGTH_IN_BYTES = 16; | ||
static final int GCM_IV_LENGTH_IN_BYTES = 12; | ||
static final int AES_BLOCK_SIZE_IN_BYTES = 128; | ||
static final String GCM_ENCRYPTION_SCHEME = "AES/GCM/NoPadding"; | ||
static final long PACKET_START_COUNTER = Long.MIN_VALUE; | ||
tvernum marked this conversation as resolved.
Show resolved
Hide resolved
|
||
static final int MAX_PACKET_LENGTH_IN_BYTES = 1 << 30; | ||
} |
31 changes: 31 additions & 0 deletions
31
...ted/src/main/java/org/elasticsearch/repositories/encrypted/EncryptedRepositoryPlugin.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,31 @@ | ||
/* | ||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
* or more contributor license agreements. Licensed under the Elastic License; | ||
* you may not use this file except in compliance with the Elastic License. | ||
*/ | ||
|
||
package org.elasticsearch.repositories.encrypted; | ||
|
||
import org.elasticsearch.common.settings.Setting; | ||
import org.elasticsearch.common.settings.Settings; | ||
import org.elasticsearch.plugins.Plugin; | ||
import org.elasticsearch.plugins.ReloadablePlugin; | ||
import org.elasticsearch.plugins.RepositoryPlugin; | ||
|
||
import java.util.List; | ||
|
||
public final class EncryptedRepositoryPlugin extends Plugin implements RepositoryPlugin, ReloadablePlugin { | ||
|
||
public EncryptedRepositoryPlugin(final Settings settings) { | ||
} | ||
|
||
@Override | ||
public List<Setting<?>> getSettings() { | ||
return List.of(); | ||
} | ||
|
||
@Override | ||
public void reload(Settings settings) { | ||
// Secure settings should be readable inside this method. | ||
} | ||
} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a max packetLength check we can do here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not really, the maximum packet length is implicitly coded in the size of the
packet
array, which does have a check for the maximum packet length on allocation.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But I did discover an error here, which I've since fixed:
int packetLength = packetInputStream.read(packet);
should beint packetLength = packetInputStream.readNBytes(packet, 0, packet.length);
because the whole complete packet must be read, not only a part of it, and plainread
does not guarantee the read length.It was not discovered by tests because the ciphertext is backed by the
ByteArrayInputStream
which always returns the requested byte count for reads. I've changed the success tests to account for input streams that return fewer bytes: 97f5917