[GitHub] [nifi] markap14 commented on a change in pull request #3594: NIFI-3833 Support for Encrypted Flow File Repositories
markap14 commented on a change in pull request #3594: NIFI-3833 Support for Encrypted Flow File Repositories URL: https://github.com/apache/nifi/pull/3594#discussion_r314039486 ## File path: nifi-docs/src/main/asciidoc/administration-guide.adoc ## @@ -2477,6 +2477,13 @@ implementation. |`nifi.flowfile.repository.always.sync`|If set to `true`, any change to the repository will be synchronized to the disk, meaning that NiFi will ask the operating system not to cache the information. This is very expensive and can significantly reduce NiFi performance. However, if it is `false`, there could be the potential for data loss if either there is a sudden power loss or the operating system crashes. The default value is `false`. | + Encryption + +The FlowFile repository can be configured to encrypt all files as they are written to disk. To enable this encryption, +set the `nifi.flowfile.repository.always.key.1` property to a 16 or 32 bit value like this: Review comment: In the description, it indicates setting "nifi.flowfile.repository.always.key.1" whereas in the example, you use "...cipher.key.1" instead of "...always.key.1" - I'm not sure that I understand the difference. Also, is it expecting multiple keys? If so, it probably makes sense in the example to show multiple keys and to explain how the different keys may be used. If it expects only a single key be provided, why use a suffix such as `.1`? This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [nifi] markap14 commented on a change in pull request #3594: NIFI-3833 Support for Encrypted Flow File Repositories
markap14 commented on a change in pull request #3594: NIFI-3833 Support for Encrypted Flow File Repositories URL: https://github.com/apache/nifi/pull/3594#discussion_r314024179 ## File path: nifi-commons/nifi-write-ahead-log/src/main/java/org/apache/nifi/wali/HashMapSnapshot.java ## @@ -264,10 +267,11 @@ public synchronized void writeSnapshot(final SnapshotCapture snapshot) throws } // Write to the partial file. -try (final FileOutputStream fileOut = new FileOutputStream(getPartialFile()); Review comment: Is there a reason to avoid the creation of the Output Streams here from within the try-with-resources? With this change, if a Throwable were to be thrown after the FileOutputStream (for example OOME thrown by `new BufferedOutputStream(...)` then we should never close the `FileOutputStream` and leak the file handle This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [nifi] markap14 commented on a change in pull request #3594: NIFI-3833 Support for Encrypted Flow File Repositories
markap14 commented on a change in pull request #3594: NIFI-3833 Support for
Encrypted Flow File Repositories
URL: https://github.com/apache/nifi/pull/3594#discussion_r31403
##
File path:
nifi-commons/nifi-write-ahead-log/src/main/java/org/apache/nifi/wali/SimpleCipherInputStream.java
##
@@ -0,0 +1,104 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.wali;
+
+import org.bouncycastle.crypto.io.CipherInputStream;
+import org.bouncycastle.crypto.modes.AEADBlockCipher;
+
+import javax.crypto.SecretKey;
+import java.io.IOException;
+import java.io.InputStream;
+
+/**
+ * This class extends {@link CipherInputStream} with a static factory method
for constructing
+ * an input stream with an AEAD block cipher.
+ */
+public class SimpleCipherInputStream extends CipherInputStream {
+protected AEADBlockCipher cipher;
+
+/**
+ * Constructs an {@link InputStream} from an existing {@link InputStream}
and block cipher.
+ *
+ * @param in input stream to wrap.
+ * @param cipher block cipher, initialized for decryption.
+ */
+public SimpleCipherInputStream(InputStream in, AEADBlockCipher cipher) {
+super(in, cipher);
+this.cipher = cipher;
+}
+
+/**
+ * Static factory for wrapping an input stream with a block cipher.
+ *
+ * NB: this function eagerly reads the initial cipher values from the
plain input stream before returning the cipher stream.
+ *
+ * @param in input stream to wrap.
+ * @param key cipher key.
+ * @return wrapped input stream.
+ * @throws IOException if the stream cannot be read eagerly, or if the
cipher cannot be initialized.
+ */
+public static InputStream wrapWithKey(InputStream in, SecretKey key)
throws IOException {
+if (key == null) {
+return in;
+}
+
+if (in.markSupported()) {
+in.mark(0);
+}
+
+// Read the marker, the iv, and the aad in the same order as they're
written in the SimpleCipherOutputStream:
+try {
+final int marker = in.read();
+if (marker != SimpleCipherUtil.MARKER_BYTE) {
Review comment:
This feels off to me. If the first byte read is not what we expect, it will
attempt to roll back that byte. Regardless of whether or not it's successfully,
it will return the InputStream given to it. So if I pass in an InputStream, I
may get back the same InputStream or I may get back a different one. If I get
back the same stream I passed in, it may have some data consumed from it, or it
may not.
I think it makes more sense to require that the stream supports mark/reset
or else throw an Exception. Also, if the first byte is not what it should be,
it should probably throw an Exception, because what it is returning is not an
InputStream that is wrapped with a key - it is the original InputStream.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [nifi] markap14 commented on a change in pull request #3594: NIFI-3833 Support for Encrypted Flow File Repositories
markap14 commented on a change in pull request #3594: NIFI-3833 Support for
Encrypted Flow File Repositories
URL: https://github.com/apache/nifi/pull/3594#discussion_r314036569
##
File path:
nifi-commons/nifi-write-ahead-log/src/main/java/org/wali/MinimalLockingWriteAheadLog.java
##
@@ -360,8 +369,13 @@ private Long recoverFromSnapshot(final Map
recordMap) throws IOExcept
// at this point, we know the snapshotPath exists because if it
didn't, then we either returned null
// or we renamed partialPath to snapshotPath. So just Recover from
snapshotPath.
-try (final DataInputStream dataIn = new DataInputStream(new
BufferedInputStream(Files.newInputStream(snapshotPath,
StandardOpenOption.READ {
-final String waliImplementationClass = dataIn.readUTF();
+try (final DataInputStream dataIn = new DataInputStream(new
BufferedInputStream(SimpleCipherInputStream.wrapWithKey(Files.newInputStream(snapshotPath,
StandardOpenOption.READ), cipherKey {
+String waliImplementationClass;
+try {
+waliImplementationClass = dataIn.readUTF();
+} catch (final java.io.UTFDataFormatException e) {
+throw new IOException("malformed input");
Review comment:
I don't have a problem with wrapping the Exception with an IOException, but
the error message provided should provide more detail about what is happening,
such as `"Unable to read Write-Ahead Log implementation class from " +
snapshotPath` and should include the `UTFDataFormatException` as the cause of
the Exception
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [nifi] markap14 commented on a change in pull request #3594: NIFI-3833 Support for Encrypted Flow File Repositories
markap14 commented on a change in pull request #3594: NIFI-3833 Support for
Encrypted Flow File Repositories
URL: https://github.com/apache/nifi/pull/3594#discussion_r314034532
##
File path:
nifi-commons/nifi-write-ahead-log/src/main/java/org/apache/nifi/wali/SimpleCipherInputStream.java
##
@@ -0,0 +1,104 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.wali;
+
+import org.bouncycastle.crypto.io.CipherInputStream;
+import org.bouncycastle.crypto.modes.AEADBlockCipher;
+
+import javax.crypto.SecretKey;
+import java.io.IOException;
+import java.io.InputStream;
+
+/**
+ * This class extends {@link CipherInputStream} with a static factory method
for constructing
+ * an input stream with an AEAD block cipher.
+ */
+public class SimpleCipherInputStream extends CipherInputStream {
+protected AEADBlockCipher cipher;
+
+/**
+ * Constructs an {@link InputStream} from an existing {@link InputStream}
and block cipher.
+ *
+ * @param in input stream to wrap.
+ * @param cipher block cipher, initialized for decryption.
+ */
+public SimpleCipherInputStream(InputStream in, AEADBlockCipher cipher) {
+super(in, cipher);
+this.cipher = cipher;
+}
+
+/**
+ * Static factory for wrapping an input stream with a block cipher.
+ *
+ * NB: this function eagerly reads the initial cipher values from the
plain input stream before returning the cipher stream.
+ *
+ * @param in input stream to wrap.
+ * @param key cipher key.
+ * @return wrapped input stream.
+ * @throws IOException if the stream cannot be read eagerly, or if the
cipher cannot be initialized.
+ */
+public static InputStream wrapWithKey(InputStream in, SecretKey key)
throws IOException {
+if (key == null) {
+return in;
+}
+
+if (in.markSupported()) {
+in.mark(0);
+}
+
+// Read the marker, the iv, and the aad in the same order as they're
written in the SimpleCipherOutputStream:
+try {
+final int marker = in.read();
+if (marker != SimpleCipherUtil.MARKER_BYTE) {
+if (in.markSupported()) {
+in.reset();
+}
+return in;
+}
+
+byte[] iv = new byte[SimpleCipherUtil.IV_BYTE_LEN];
+int len = in.read(iv);
+if (len != iv.length) {
+throw new IOException("Could not read IV.");
+}
+
+byte[] aad = new byte[SimpleCipherUtil.AAD_BYTE_LEN];
+len = in.read(aad);
+if (len != aad.length) {
+throw new IOException("Could not read AAD.");
+}
+
+AEADBlockCipher cipher = SimpleCipherUtil.initCipher(key, false,
iv, aad);
+return new SimpleCipherInputStream(in, cipher);
+
+} catch (final IOException ignored) {
+if (in.markSupported()) {
+in.reset();
+}
+return in;
+}
+}
+
+@Override
+public void close() throws IOException {
+try {
Review comment:
The only checked Exception thrown by `CipherInputStream#close` is
`IOException` so I don't think we need to catch and wrap the Exception. Which
means that we don't need to define the `close` method at all, just let the
parent handle it.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [nifi] markap14 commented on a change in pull request #3594: NIFI-3833 Support for Encrypted Flow File Repositories
markap14 commented on a change in pull request #3594: NIFI-3833 Support for
Encrypted Flow File Repositories
URL: https://github.com/apache/nifi/pull/3594#discussion_r314028968
##
File path:
nifi-commons/nifi-write-ahead-log/src/main/java/org/apache/nifi/wali/LengthDelimitedJournal.java
##
@@ -411,9 +417,10 @@ public JournalRecovery recoverRecords(final Map recordMap, final Set<
final double journalLength = journalFile.length();
try (final InputStream fis = new FileInputStream(journalFile);
-final InputStream bufferedIn = new BufferedInputStream(fis);
-final ByteCountingInputStream byteCountingIn = new
ByteCountingInputStream(bufferedIn);
-final DataInputStream in = new DataInputStream(byteCountingIn)) {
+ final InputStream bufferedIn = new BufferedInputStream(fis);
+ final InputStream plainIn =
SimpleCipherInputStream.wrapWithKey(bufferedIn, cipherKey);
+ final ByteCountingInputStream byteCountingIn = new
ByteCountingInputStream(plainIn);
Review comment:
`byteCountingIn` is used to know how much data has been consumed for logging
purposes. It probably makes sense to wrap `bufferedIn` with `byteCountingIn` so
that any log message that indicates "X number of bytes have been read" tells us
how many bytes were actually consumed from the file. As-is here, this tells us
how many post-decryption bytes were consumed.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [nifi] markap14 commented on a change in pull request #3594: NIFI-3833 Support for Encrypted Flow File Repositories
markap14 commented on a change in pull request #3594: NIFI-3833 Support for
Encrypted Flow File Repositories
URL: https://github.com/apache/nifi/pull/3594#discussion_r314033676
##
File path:
nifi-commons/nifi-write-ahead-log/src/main/java/org/apache/nifi/wali/SimpleCipherInputStream.java
##
@@ -0,0 +1,104 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.wali;
+
+import org.bouncycastle.crypto.io.CipherInputStream;
+import org.bouncycastle.crypto.modes.AEADBlockCipher;
+
+import javax.crypto.SecretKey;
+import java.io.IOException;
+import java.io.InputStream;
+
+/**
+ * This class extends {@link CipherInputStream} with a static factory method
for constructing
+ * an input stream with an AEAD block cipher.
+ */
+public class SimpleCipherInputStream extends CipherInputStream {
+protected AEADBlockCipher cipher;
+
+/**
+ * Constructs an {@link InputStream} from an existing {@link InputStream}
and block cipher.
+ *
+ * @param in input stream to wrap.
+ * @param cipher block cipher, initialized for decryption.
+ */
+public SimpleCipherInputStream(InputStream in, AEADBlockCipher cipher) {
+super(in, cipher);
+this.cipher = cipher;
+}
+
+/**
+ * Static factory for wrapping an input stream with a block cipher.
+ *
+ * NB: this function eagerly reads the initial cipher values from the
plain input stream before returning the cipher stream.
+ *
+ * @param in input stream to wrap.
+ * @param key cipher key.
+ * @return wrapped input stream.
+ * @throws IOException if the stream cannot be read eagerly, or if the
cipher cannot be initialized.
+ */
+public static InputStream wrapWithKey(InputStream in, SecretKey key)
throws IOException {
+if (key == null) {
+return in;
+}
+
+if (in.markSupported()) {
+in.mark(0);
+}
+
+// Read the marker, the iv, and the aad in the same order as they're
written in the SimpleCipherOutputStream:
+try {
+final int marker = in.read();
+if (marker != SimpleCipherUtil.MARKER_BYTE) {
+if (in.markSupported()) {
+in.reset();
+}
+return in;
+}
+
+byte[] iv = new byte[SimpleCipherUtil.IV_BYTE_LEN];
+int len = in.read(iv);
+if (len != iv.length) {
+throw new IOException("Could not read IV.");
+}
+
+byte[] aad = new byte[SimpleCipherUtil.AAD_BYTE_LEN];
+len = in.read(aad);
+if (len != aad.length) {
+throw new IOException("Could not read AAD.");
+}
+
+AEADBlockCipher cipher = SimpleCipherUtil.initCipher(key, false,
iv, aad);
+return new SimpleCipherInputStream(in, cipher);
+
+} catch (final IOException ignored) {
Review comment:
If an `IOException` is thrown, I think we should just let it fly, rather
than catching. It's not great to just swallow the Exception.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [nifi] markap14 commented on a change in pull request #3594: NIFI-3833 Support for Encrypted Flow File Repositories
markap14 commented on a change in pull request #3594: NIFI-3833 Support for
Encrypted Flow File Repositories
URL: https://github.com/apache/nifi/pull/3594#discussion_r314033233
##
File path:
nifi-commons/nifi-write-ahead-log/src/main/java/org/apache/nifi/wali/SimpleCipherInputStream.java
##
@@ -0,0 +1,104 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.wali;
+
+import org.bouncycastle.crypto.io.CipherInputStream;
+import org.bouncycastle.crypto.modes.AEADBlockCipher;
+
+import javax.crypto.SecretKey;
+import java.io.IOException;
+import java.io.InputStream;
+
+/**
+ * This class extends {@link CipherInputStream} with a static factory method
for constructing
+ * an input stream with an AEAD block cipher.
+ */
+public class SimpleCipherInputStream extends CipherInputStream {
+protected AEADBlockCipher cipher;
+
+/**
+ * Constructs an {@link InputStream} from an existing {@link InputStream}
and block cipher.
+ *
+ * @param in input stream to wrap.
+ * @param cipher block cipher, initialized for decryption.
+ */
+public SimpleCipherInputStream(InputStream in, AEADBlockCipher cipher) {
+super(in, cipher);
+this.cipher = cipher;
+}
+
+/**
+ * Static factory for wrapping an input stream with a block cipher.
+ *
+ * NB: this function eagerly reads the initial cipher values from the
plain input stream before returning the cipher stream.
+ *
+ * @param in input stream to wrap.
+ * @param key cipher key.
+ * @return wrapped input stream.
+ * @throws IOException if the stream cannot be read eagerly, or if the
cipher cannot be initialized.
+ */
+public static InputStream wrapWithKey(InputStream in, SecretKey key)
throws IOException {
+if (key == null) {
+return in;
+}
+
+if (in.markSupported()) {
+in.mark(0);
+}
+
+// Read the marker, the iv, and the aad in the same order as they're
written in the SimpleCipherOutputStream:
+try {
+final int marker = in.read();
+if (marker != SimpleCipherUtil.MARKER_BYTE) {
+if (in.markSupported()) {
+in.reset();
+}
+return in;
+}
+
+byte[] iv = new byte[SimpleCipherUtil.IV_BYTE_LEN];
+int len = in.read(iv);
+if (len != iv.length) {
+throw new IOException("Could not read IV.");
+}
+
+byte[] aad = new byte[SimpleCipherUtil.AAD_BYTE_LEN];
+len = in.read(aad);
Review comment:
Same comment as above.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [nifi] markap14 commented on a change in pull request #3594: NIFI-3833 Support for Encrypted Flow File Repositories
markap14 commented on a change in pull request #3594: NIFI-3833 Support for
Encrypted Flow File Repositories
URL: https://github.com/apache/nifi/pull/3594#discussion_r314035006
##
File path:
nifi-commons/nifi-write-ahead-log/src/main/java/org/apache/nifi/wali/SimpleCipherOutputStream.java
##
@@ -0,0 +1,72 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.wali;
+
+import org.bouncycastle.crypto.io.CipherOutputStream;
+import org.bouncycastle.crypto.modes.AEADBlockCipher;
+
+import javax.crypto.SecretKey;
+import java.io.IOException;
+import java.io.OutputStream;
+
+/**
+ * This class extends {@link CipherOutputStream} with a static factory method
for constructing
+ * an output stream with an AEAD block cipher.
+ *
+ * Note that the {@link CipherOutputStream} implementation writes the MAC at
the end of the stream during `close`.
+ * If streams using this class aren't closed properly, the result may be a
stream without a MAC written, which
+ * causes a MAC authentication failure in the input stream.
+ *
+ */
+public class SimpleCipherOutputStream extends CipherOutputStream {
+/**
+ * Constructs an {@link OutputStream} from an existing {@link
OutputStream} and block cipher.
+ *
+ * @param out output stream to wrap.
+ * @param cipher block cipher, initialized for encryption.
+ */
+public SimpleCipherOutputStream(OutputStream out, AEADBlockCipher cipher) {
+super(out, cipher);
+}
+
+/**
+ * Static factory for wrapping an output stream with a block cipher.
+ *
+ * NB: this function eagerly writes the initial cipher values to the
plain output stream before returning the cipher stream.
+ *
+ * @param out output stream to wrap.
+ * @param key cipher key.
+ * @return wrapped output stream.
+ * @throws IOException if the stream cannot be written eagerly, or if the
cipher cannot be initialized.
+ */
+public static OutputStream wrapWithKey(OutputStream out, SecretKey key)
throws IOException {
+if (key == null) {
+return out;
Review comment:
If key is null, I would rather see a `NullPointerException` thrown. As-is,
the Stream that is returned may not be wrapped at all
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [nifi] markap14 commented on a change in pull request #3594: NIFI-3833 Support for Encrypted Flow File Repositories
markap14 commented on a change in pull request #3594: NIFI-3833 Support for
Encrypted Flow File Repositories
URL: https://github.com/apache/nifi/pull/3594#discussion_r314027272
##
File path:
nifi-commons/nifi-write-ahead-log/src/main/java/org/apache/nifi/wali/LengthDelimitedJournal.java
##
@@ -373,7 +378,8 @@ public synchronized void fsync() throws IOException {
try {
if (fileOut != null) {
-fileOut.getChannel().force(false);
+// Before transparent crypto, this was:
fileOut.getChannel().force(false);
Review comment:
This change is not equivalent. We must be sure that we cause a `fsync` to
occur, or else we face the possibility of data loss. To do this, we will need
access to the `FileOutputStream`. So we will need to somehow couple the
FileOutputStream along with the CipherOutputStream - either by storing a
Tuple (which is not ideal, as it's not
very clean) or - preferably - by creating a wrapper with either a getter for
the underlying FileOutputStream and a getter for the underlying
CipherOutputStream or an `fsync()` method and a getter for the underlying
CipherOutputStream. Either way, we must ensure that we `fsync` here to avoid
loss of data.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [nifi] markap14 commented on a change in pull request #3594: NIFI-3833 Support for Encrypted Flow File Repositories
markap14 commented on a change in pull request #3594: NIFI-3833 Support for
Encrypted Flow File Repositories
URL: https://github.com/apache/nifi/pull/3594#discussion_r314033147
##
File path:
nifi-commons/nifi-write-ahead-log/src/main/java/org/apache/nifi/wali/SimpleCipherInputStream.java
##
@@ -0,0 +1,104 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.wali;
+
+import org.bouncycastle.crypto.io.CipherInputStream;
+import org.bouncycastle.crypto.modes.AEADBlockCipher;
+
+import javax.crypto.SecretKey;
+import java.io.IOException;
+import java.io.InputStream;
+
+/**
+ * This class extends {@link CipherInputStream} with a static factory method
for constructing
+ * an input stream with an AEAD block cipher.
+ */
+public class SimpleCipherInputStream extends CipherInputStream {
+protected AEADBlockCipher cipher;
+
+/**
+ * Constructs an {@link InputStream} from an existing {@link InputStream}
and block cipher.
+ *
+ * @param in input stream to wrap.
+ * @param cipher block cipher, initialized for decryption.
+ */
+public SimpleCipherInputStream(InputStream in, AEADBlockCipher cipher) {
+super(in, cipher);
+this.cipher = cipher;
+}
+
+/**
+ * Static factory for wrapping an input stream with a block cipher.
+ *
+ * NB: this function eagerly reads the initial cipher values from the
plain input stream before returning the cipher stream.
+ *
+ * @param in input stream to wrap.
+ * @param key cipher key.
+ * @return wrapped input stream.
+ * @throws IOException if the stream cannot be read eagerly, or if the
cipher cannot be initialized.
+ */
+public static InputStream wrapWithKey(InputStream in, SecretKey key)
throws IOException {
+if (key == null) {
+return in;
+}
+
+if (in.markSupported()) {
+in.mark(0);
+}
+
+// Read the marker, the iv, and the aad in the same order as they're
written in the SimpleCipherOutputStream:
+try {
+final int marker = in.read();
+if (marker != SimpleCipherUtil.MARKER_BYTE) {
+if (in.markSupported()) {
+in.reset();
+}
+return in;
+}
+
+byte[] iv = new byte[SimpleCipherUtil.IV_BYTE_LEN];
+int len = in.read(iv);
Review comment:
We cannot assume that a single call to `InputStream#read` will read the
desired number of bytes. This has to be done in a loop - which can be
simplified by calling the convenience method `StreamUtils.fillBuffer(iv)` if
it's on the classpath (though I wouldn't recommend adding it just for this).
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
