[
https://issues.apache.org/jira/browse/HADOOP-10628?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14008333#comment-14008333
]
Yi Liu commented on HADOOP-10628:
-
The additional javadoc and code style comments in HADOOP-10603 are as following.
---
In general, please add blank lines before all block comments.
AESCTRCryptoCodec.java
+public abstract class AESCTRCryptoCodec extends CryptoCodec {
+ /**
+ * For AES, the algorithm block is fixed size of 128 bits.
+ * @see http://en.wikipedia.org/wiki/Advanced_Encryption_Standard
+ */
+ private static final int AES_BLOCK_SIZE = 16;
+ /**
+ * IV is produced by combining initial IV and the counter using addition.
+ * IV length should be the same as
{@link #AES_BLOCK_SIZE}
+ */
The IV is produced by adding the initial IV to the counter. IV length
should be the same as {@link #AES_BLOCK_SIZE}
.
+ @Override
+ public void calculateIV(byte[] initIV, long counter, byte[] IV) {
...
+ ByteBuffer buf = ByteBuffer.wrap(IV);
add a final decl.
CryptoCodec.java
+ /**
+ * Get block size of a block cipher.
Get the block size of a block cipher.
+ * For different algorithms, the block size may be different.
+ * @return int block size
@return the block size
+ * Get a
{@link #org.apache.hadoop.crypto.Encryptor}
.
s/Get a/Get an/
+ * @return Encryptor
@return the Encryptor
+ * @return Decryptor
@return the Decryptor
+ * This interface is only for Counter (CTR) mode. Typically calculating
+ * IV(Initialization Vector) is up to Encryptor or Decryptor, for
+ * example
{@link #javax.crypto.Cipher} will maintain encryption context
+ * internally when do encryption/decryption continuously using its
+ * Cipher#update interface.
This interface is only needed for AES-CTR mode. The IV is generally
calculated by the Encryptor or Decryptor and maintained as internal
state. For example, a {@link #javax.crypto.Cipher}
will maintain its
encryption context internally using the Cipher#update interface.
+ * In Hadoop, multiple nodes may read splits of a file, so decrypting of
+ * file is not continuous, even for encrypting may be not continuous. For
+ * each part, we need to calculate the counter through file position.
Encryption/Decryption is not always on the entire file. For example,
in Hadoop, a node may only decrypt a portion of a file (i.e. a
split). In these situations, the counter is derived from the file
position.
+ * p/
+ * Typically IV for a file position is produced by combining initial IV and
+ * the counter using any lossless operation (concatenation, addition, or XOR).
+ * @see
http://en.wikipedia.org/wiki/Block_cipher_mode_of_operation#Counter_.28CTR.29
The IV can be calculated based on the file position by combining the
initial IV and the counter with a lossless operation (concatenation, addition,
or XOR).
@see
http://en.wikipedia.org/wiki/Block_cipher_mode_of_operation#Counter_.28CTR.29
CryptoInputStream.java
+public class CryptoInputStream extends FilterInputStream implements
+ Seekable, PositionedReadable, ByteBufferReadable, HasFileDescriptor,
+ CanSetDropBehind, CanSetReadahead, HasEnhancedByteBufferAccess {
Add a newline here please.
+ /**
+ * Whether underlying stream supports
s/Whether underlying/Whether the underlying/
+ /**
+ * Padding = pos%(algorithm blocksize); Padding is put into
{@link #inBuffer}
+ * before any other data goes in. The purpose of padding is to put input data
+ * at proper position.
s/put input data/put the input data/
+ @Override
+ public int read(byte[] b, int off, int len) throws IOException {
+ int remaining = outBuffer.remaining();
final int remaining...
+ if (usingByteBufferRead == null) {
+ if (in instanceof ByteBufferReadable) {
+ try { + n = ((ByteBufferReadable) in).read(inBuffer); + usingByteBufferRead =
Boolean.TRUE; + } catch (UnsupportedOperationException e) { +
usingByteBufferRead = Boolean.FALSE; + }
+ }
+ if (!usingByteBufferRead.booleanValue()) { + n = readFromUnderlyingStream();
+ }
+ } else {
+ if (usingByteBufferRead.booleanValue()) { + n = ((ByteBufferReadable)
in).read(inBuffer); + } else { + n = readFromUnderlyingStream(); + }
+ }
For the code above, I wonder if we shouldn't maintain a reference to
the actual ByteBuffer once it is known to be ByteBufferReadable. If
the caller switches BBs, then it is possible that this could throw a
UnsupportedOperationException. So the check would be to see if the BB
was the same one that was already known to be BBReadable, and if not,
then check it again.
+ // Read data from underlying stream.
+ private int readFromUnderlyingStream() throws IOException {
+ int toRead = inBuffer.remaining();
+ byte[] tmp = getTmpBuf();
+ int n = in.read(tmp, 0, toRead);
finals could be added to each of these decls.
+ private void decrypt() throws IOException {
+
