You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@solr.apache.org by br...@apache.org on 2024/03/28 16:42:03 UTC
(solr-sandbox) branch main updated: Remove EncryptionDirectory.shouldCheckEncryptionWhenReading optimization. (#102)
This is an automated email from the ASF dual-hosted git repository.
broustant pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/solr-sandbox.git
The following commit(s) were added to refs/heads/main by this push:
new 130ba53 Remove EncryptionDirectory.shouldCheckEncryptionWhenReading optimization. (#102)
130ba53 is described below
commit 130ba536c58f94bf8be15fabe903c36aa5089979
Author: Bruno Roustant <33...@users.noreply.github.com>
AuthorDate: Thu Mar 28 17:41:58 2024 +0100
Remove EncryptionDirectory.shouldCheckEncryptionWhenReading optimization. (#102)
---
.../solr/encryption/EncryptionDirectory.java | 53 ++++++----------------
.../solr/encryption/EncryptionTransactionLog.java | 31 +++++++------
.../solr/encryption/EncryptionUpdateLog.java | 4 +-
.../org/apache/solr/encryption/EncryptionUtil.java | 12 -----
4 files changed, 33 insertions(+), 67 deletions(-)
diff --git a/encryption/src/main/java/org/apache/solr/encryption/EncryptionDirectory.java b/encryption/src/main/java/org/apache/solr/encryption/EncryptionDirectory.java
index aa18cea..4616753 100644
--- a/encryption/src/main/java/org/apache/solr/encryption/EncryptionDirectory.java
+++ b/encryption/src/main/java/org/apache/solr/encryption/EncryptionDirectory.java
@@ -19,6 +19,7 @@ package org.apache.solr.encryption;
import org.apache.lucene.index.IndexFileNames;
import org.apache.lucene.index.SegmentCommitInfo;
import org.apache.lucene.index.SegmentInfos;
+import org.apache.lucene.store.DataOutput;
import org.apache.lucene.store.Directory;
import org.apache.lucene.store.FilterDirectory;
import org.apache.lucene.store.IOContext;
@@ -87,9 +88,6 @@ public class EncryptionDirectory extends FilterDirectory {
/** Cache of the latest commit user data. */
protected volatile CommitUserData commitUserData;
- /** Optimization flag to avoid checking encryption when reading a file if we know the index is cleartext. */
- protected volatile boolean shouldCheckEncryptionWhenReading;
-
/** Optimization flag to only read the commit user data once after a commit. */
protected volatile boolean shouldReadCommitUserData;
@@ -106,11 +104,6 @@ public class EncryptionDirectory extends FilterDirectory {
this.encrypterFactory = encrypterFactory;
this.keySupplier = keySupplier;
commitUserData = readLatestCommitUserData();
-
- // If there is no encryption key id parameter in the latest commit user data, then we know the index
- // is cleartext, so we can skip fast any encryption check. This flag becomes true indefinitely if we
- // detect an encryption key when opening a file for writing.
- shouldCheckEncryptionWhenReading = hasKeyIdInCommit(commitUserData.data);
}
/** Gets the {@link AesCtrEncrypterFactory} used. */
@@ -118,11 +111,6 @@ public class EncryptionDirectory extends FilterDirectory {
return encrypterFactory;
}
- /** Gets the {@link KeySupplier} used. */
- public KeySupplier getKeySupplier() {
- return keySupplier;
- }
-
@Override
public IndexOutput createOutput(String fileName, IOContext context) throws IOException {
return maybeWrapOutput(in.createOutput(fileName, context));
@@ -153,10 +141,13 @@ public class EncryptionDirectory extends FilterDirectory {
}
boolean success = false;
try {
- String keyRef = getKeyRefForWriting(indexOutput);
+ String keyRef = getActiveKeyRefFromCommit(getLatestCommitData().data);
if (keyRef != null) {
+ // Get the key secret first. If it fails, we do not write anything.
+ byte[] keySecret = getKeySecret(keyRef);
// The IndexOutput has to be wrapped to be encrypted with the key.
- indexOutput = new EncryptingIndexOutput(indexOutput, getKeySecret(keyRef), encrypterFactory);
+ writeEncryptionHeader(keyRef, indexOutput);
+ indexOutput = new EncryptingIndexOutput(indexOutput, keySecret, encrypterFactory);
}
success = true;
} finally {
@@ -168,25 +159,10 @@ public class EncryptionDirectory extends FilterDirectory {
return indexOutput;
}
- /**
- * Gets the active key reference number for writing an index output.
- * <p>
- * The active key ref is defined in the user data of the latest commit. If it is present, then this method
- * writes to the output the {@link #ENCRYPTION_MAGIC} header, followed by the key reference number as a
- * 4B big-endian int.
- *
- * @return the key reference number; or null if none.
- */
- protected String getKeyRefForWriting(IndexOutput indexOutput) throws IOException {
- String keyRef;
- if ((keyRef = getActiveKeyRefFromCommit(getLatestCommitData().data)) == null) {
- return null;
- }
- shouldCheckEncryptionWhenReading = true;
- // Write the encryption magic header and the key reference number.
- writeBEInt(indexOutput, ENCRYPTION_MAGIC);
- writeBEInt(indexOutput, Integer.parseInt(keyRef));
- return keyRef;
+ /** Writes the encryption magic header and the key reference number. */
+ private static void writeEncryptionHeader(String keyRef, DataOutput out) throws IOException {
+ writeBEInt(out, ENCRYPTION_MAGIC);
+ writeBEInt(out, Integer.parseInt(keyRef));
}
/**
@@ -270,10 +246,6 @@ public class EncryptionDirectory extends FilterDirectory {
@Override
public IndexInput openInput(String fileName, IOContext context) throws IOException {
IndexInput indexInput = in.openInput(fileName, context);
- if (!shouldCheckEncryptionWhenReading) {
- // Return the IndexInput directly as we know it is not encrypted.
- return indexInput;
- }
boolean success = false;
try {
String keyRef = getKeyRefForReading(indexInput);
@@ -299,9 +271,12 @@ public class EncryptionDirectory extends FilterDirectory {
* If the file is cleartext, it starts with the
* {@link org.apache.lucene.codecs.CodecUtil#CODEC_MAGIC} header.
*
- * @return the key reference number; or null if none.
+ * @return the key reference number; or null if none, meaning the index is not encrypted.
*/
protected String getKeyRefForReading(IndexInput indexInput) throws IOException {
+ // Always reading the magic number, even for non-encrypted indexes, is not a performance
+ // issue because it will be read immediately again when the Directory is returned, to
+ // check the index header (CodecUtil.checkIndexHeader()).
long filePointer = indexInput.getFilePointer();
int magic = readBEInt(indexInput);
if (magic == ENCRYPTION_MAGIC) {
diff --git a/encryption/src/main/java/org/apache/solr/encryption/EncryptionTransactionLog.java b/encryption/src/main/java/org/apache/solr/encryption/EncryptionTransactionLog.java
index 6b62329..6d4b2b1 100644
--- a/encryption/src/main/java/org/apache/solr/encryption/EncryptionTransactionLog.java
+++ b/encryption/src/main/java/org/apache/solr/encryption/EncryptionTransactionLog.java
@@ -125,7 +125,7 @@ public class EncryptionTransactionLog extends TransactionLog {
/**
* Reads the encryption magic header and the key reference number.
*
- * @return The key reference number as a string.
+ * @return The key reference number as a string; or null if the log is not encrypted.
*/
static String readEncryptionHeader(FileChannel channel, ByteBuffer readBuffer) throws IOException {
long position = channel.position();
@@ -188,13 +188,14 @@ public class EncryptionTransactionLog extends TransactionLog {
try {
String keyRef = getActiveKeyRefFromCommit(directory.getLatestCommitData().data);
if (keyRef != null) {
+ // Get the key secret first. If it fails, we do not write anything.
+ byte[] keySecret = directory.getKeySecret(keyRef);
// The output stream has to be wrapped to be encrypted with the key.
- directory.shouldCheckEncryptionWhenReading = true;
writeEncryptionHeader(keyRef, outputStream);
EncryptingOutputStream eos = createEncryptingOutputStream(outputStream,
position,
ivHolder.iv,
- directory.getKeySecret(keyRef),
+ keySecret,
directory.getEncrypterFactory());
ivHolder.iv = eos.getIv();
return eos;
@@ -233,18 +234,18 @@ public class EncryptionTransactionLog extends TransactionLog {
public ChannelFastInputStream open(FileChannel channel, long position) throws IOException {
EncryptionDirectory directory = directorySupplier.get();
try {
- if (directory.shouldCheckEncryptionWhenReading) {
- String keyRef = readEncryptionHeader(channel, readBuffer);
- if (keyRef != null) {
- // The IndexInput has to be wrapped to be decrypted with the key.
- DecryptingChannelInputStream dcis = createDecryptingChannelInputStream(channel,
- ENCRYPTION_KEY_HEADER_LENGTH,
- position,
- directory.getKeySecret(keyRef),
- directory.getEncrypterFactory());
- ivHolder.iv = dcis.getIv();
- return dcis;
- }
+ String keyRef = readEncryptionHeader(channel, readBuffer);
+ if (keyRef != null) {
+ // The IndexInput has to be wrapped to be decrypted with the key.
+ DecryptingChannelInputStream dcis =
+ createDecryptingChannelInputStream(
+ channel,
+ ENCRYPTION_KEY_HEADER_LENGTH,
+ position,
+ directory.getKeySecret(keyRef),
+ directory.getEncrypterFactory());
+ ivHolder.iv = dcis.getIv();
+ return dcis;
}
} finally {
directorySupplier.release(directory);
diff --git a/encryption/src/main/java/org/apache/solr/encryption/EncryptionUpdateLog.java b/encryption/src/main/java/org/apache/solr/encryption/EncryptionUpdateLog.java
index ab60c8a..660b372 100644
--- a/encryption/src/main/java/org/apache/solr/encryption/EncryptionUpdateLog.java
+++ b/encryption/src/main/java/org/apache/solr/encryption/EncryptionUpdateLog.java
@@ -177,9 +177,11 @@ public class EncryptionUpdateLog extends UpdateLog {
directory.getKeySecret(inputKeyRef),
directory.getEncrypterFactory());
if (activeKeyRef != null) {
+ // Get the key secret first. If it fails, we do not write anything.
+ byte[] keySecret = directory.getKeySecret(activeKeyRef);
writeEncryptionHeader(activeKeyRef, outputStream);
outputStream = new EncryptingOutputStream(outputStream,
- directory.getKeySecret(activeKeyRef),
+ keySecret,
directory.getEncrypterFactory());
}
byte[] buffer = new byte[REENCRYPTION_BUFFER_SIZE];
diff --git a/encryption/src/main/java/org/apache/solr/encryption/EncryptionUtil.java b/encryption/src/main/java/org/apache/solr/encryption/EncryptionUtil.java
index ff353db..9665c2b 100644
--- a/encryption/src/main/java/org/apache/solr/encryption/EncryptionUtil.java
+++ b/encryption/src/main/java/org/apache/solr/encryption/EncryptionUtil.java
@@ -129,18 +129,6 @@ public class EncryptionUtil {
return keyId;
}
- /**
- * @return Whether the provided commit user data contain some encryption key ids (active or not).
- */
- public static boolean hasKeyIdInCommit(Map<String, String> commitUserData) {
- for (String entryKey : commitUserData.keySet()) {
- if (entryKey.startsWith(COMMIT_KEY_ID)) {
- return true;
- }
- }
- return false;
- }
-
/**
* Gets the cookies (key-value pairs) for all the key ids, from the provided commit user data.
*