Skip to content

Commit

Permalink
Merge pull request ibmruntimes#25 from jasonkatonica/katonica/issue18…
Browse files Browse the repository at this point in the history 
…703/overlappingbuffers

Avoid overlapping buffers in native ChaCha20
  • Loading branch information
@keithc-ca
keithc-ca authored Feb 21, 2024
2 parents 7ef956c + f19f267 commit 441da50
Showing 1 changed file with 106 additions and 22 deletions.
128 changes: 106 additions & 22 deletions closed/src/java.base/share/classes/com/sun/crypto/provider/NativeChaCha20Cipher.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
*/
/*
* ===========================================================================
* (c) Copyright IBM Corp. 2018, 2023 All Rights Reserved
* (c) Copyright IBM Corp. 2018, 2024 All Rights Reserved
* ===========================================================================
*/
package com.sun.crypto.provider;
Expand Down Expand Up @@ -214,20 +214,7 @@ protected int engineGetBlockSize() {
*/
@Override
protected int engineGetOutputSize(int inputLen) {
int outLen = 0;

if (mode == MODE_NONE) {
outLen = inputLen;
} else if (mode == MODE_AEAD) {
if (direction == Cipher.ENCRYPT_MODE) {
outLen = Math.addExact(inputLen, TAG_LENGTH);
} else {
outLen = Integer.max(inputLen, TAG_LENGTH) - TAG_LENGTH;
outLen = Math.addExact(outLen, engine.getCipherBufferLength());
}
}

return outLen;
return engine.getOutputSize(inputLen, true);
}

/**
Expand Down Expand Up @@ -659,6 +646,9 @@ private static byte[] getEncodedKey(Key key) throws InvalidKeyException {
}
byte[] encodedKey = key.getEncoded();
if (encodedKey == null || encodedKey.length != 32) {
if (encodedKey != null) {
Arrays.fill(encodedKey, (byte)0);
}
throw new InvalidKeyException("Key length must be 256 bits");
}
return encodedKey;
Expand All @@ -677,11 +667,11 @@ private static byte[] getEncodedKey(Key key) throws InvalidKeyException {
*/
@Override
protected byte[] engineUpdate(byte[] in, int inOfs, int inLen) {
byte[] out = new byte[inLen];
byte[] out = new byte[engine.getOutputSize(inLen, false)];
try {
int size = engine.doUpdate(in, inOfs, inLen, out, 0);
// Special case for EngineAEADDec, doUpdate only buffers the input
// So the output array must be empty since no encryption happened yet
// so the output array must be empty since no encryption has happened yet.
if (size == 0) {
return new byte[0];
}
Expand Down Expand Up @@ -740,7 +730,7 @@ protected int engineUpdate(byte[] in, int inOfs, int inLen,
@Override
protected byte[] engineDoFinal(byte[] in, int inOfs, int inLen)
throws AEADBadTagException {
byte[] output = new byte[engineGetOutputSize(inLen)];
byte[] output = new byte[engine.getOutputSize(inLen, true)];
try {
engine.doFinal(in, inOfs, inLen, output, 0);
} catch (ShortBufferException | KeyException exc) {
Expand Down Expand Up @@ -889,6 +879,17 @@ private void resetStartState() {
* Interface for the underlying processing engines for ChaCha20
*/
interface ChaChaEngine {
/**
* Size an output buffer based on the input and where applicable
* the current state of the engine in a multipart operation.
*
* @param inLength the input length.
* @param isFinal true if this is invoked from a doFinal call.
*
* @return the recommended size for the output buffer.
*/
int getOutputSize(int inLength, boolean isFinal);

/**
* Perform a multi-part update for ChaCha20.
*
Expand Down Expand Up @@ -935,12 +936,49 @@ int doFinal(byte[] in, int inOff, int inLen, byte[] out, int outOff)
* @return the number of unprocessed bytes left.
*/
int getCipherBufferLength();

/**
* Determines if two arrays have any overlapping memory.
*
* @param input The input array.
* @param inputStart The index of the input arrays start.
* @param inputEnd The index of the input arrays end.
* @param output The output array.
* @param outputStart The index of the output arrays start.
* @param outputEnd The index of the output arrays end.
* @return true if any memory overlaps, false otherwise.
*/
static boolean arraysOverlap(byte[] input, int inputStart, int inputEnd, byte[] output, int outputStart, int outputEnd) {

// Check if there is potential overlap if input and output buffers have same address.
if (input != output) {
return false;
}

// If the start of input is anywhere in the range of the output array, there is an overlap.
if ((outputStart <= inputStart) && (inputStart < outputEnd)) {
return true;
}

// If the start of output is anywhere in the range of the input array, there is an overlap.
if ((inputStart <= outputStart) && (outputStart < inputEnd)) {
return true;
}

return false; // Memory does not overlap, return false.
}
}

private final class EngineStreamOnly implements ChaChaEngine {

EngineStreamOnly() { }

@Override
public int getOutputSize(int inLength, boolean isFinal) {
// The isFinal parameter is not relevant in this kind of engine.
return inLength;
}

@Override
public synchronized int doUpdate(byte[] in, int inOff, int inLen, byte[] out,
int outOff) throws ShortBufferException, KeyException {
Expand All @@ -962,7 +1000,20 @@ public synchronized int doUpdate(byte[] in, int inOff, int inLen, byte[] out,

if (in != null) {
Objects.checkFromIndexSize(inOff, inLen, in.length);
int ret = nativeCrypto.ChaCha20Update(context, in, inOff, inLen, out, outOff, /*aadArray*/ null, /*aadArray.length*/ 0);

// Check if there is any overlap with the input and output
// buffers. If so, create a temporary copy of the input such
// that it does not overlap while performing an update computation
// within the openssl library.
int ret = -1;
if (ChaChaEngine.arraysOverlap(in, inOff, inOff + inLen, out, outOff, outOff + getOutputSize(inLen, false))) {
byte[] newInArray = Arrays.copyOfRange(in, inOff, inOff + inLen);
ret = nativeCrypto.ChaCha20Update(context, newInArray, 0, inLen, out, outOff, /*aadArray*/ null, /*aadArray.length*/ 0);
Arrays.fill(newInArray, (byte)0);
} else {
ret = nativeCrypto.ChaCha20Update(context, in, inOff, inLen, out, outOff, /*aadArray*/ null, /*aadArray.length*/ 0);
}

if (ret == -1) {
throw new ProviderException("Error in Native ChaCha20Cipher");
}
Expand Down Expand Up @@ -992,6 +1043,11 @@ private final class EngineAEADEnc implements ChaChaEngine {
counter = 1;
}

@Override
public int getOutputSize(int inLength, boolean isFinal) {
return (isFinal ? Math.addExact(inLength, TAG_LENGTH) : inLength);
}

@Override
public synchronized int doUpdate(byte[] in, int inOff, int inLen, byte[] out,
int outOff) throws ShortBufferException, KeyException {
Expand Down Expand Up @@ -1021,9 +1077,22 @@ public synchronized int doUpdate(byte[] in, int inOff, int inLen, byte[] out,

byte[] aadArray = aadBuf.toByteArray();
aadBuf.reset();
int ret = nativeCrypto.ChaCha20Update(context, in, inOff, inLen, out, outOff, aadArray, aadArray.length);

// Check if there is any overlap with the input and output
// buffers. If so, create a temporary copy of the input such
// that it does not overlap while performing an update computation
// within the openssl library.
int ret = -1;
if (ChaChaEngine.arraysOverlap(in, inOff, inOff + inLen, out, outOff, outOff + getOutputSize(inLen, false))) {
byte[] newInArray = Arrays.copyOfRange(in, inOff, inOff + inLen);
ret = nativeCrypto.ChaCha20Update(context, newInArray, 0, inLen, out, outOff, aadArray, aadArray.length);
Arrays.fill(newInArray, (byte)0);
} else {
ret = nativeCrypto.ChaCha20Update(context, in, inOff, inLen, out, outOff, aadArray, aadArray.length);
}

if (ret == -1) {
throw new ProviderException("Error in Native CipherBlockChaining");
throw new ProviderException("Error in Native ChaCha20Cipher");
}
}
return inLen;
Expand Down Expand Up @@ -1069,6 +1138,18 @@ private final class EngineAEADDec implements ChaChaEngine {
tag = new byte[TAG_LENGTH];
}

@Override
public int getOutputSize(int inLen, boolean isFinal) {
// If we are performing a decrypt-update we should always return
// zero length since we cannot return any data until the tag has
// been consumed and verified. CipherSpi.engineGetOutputSize will
// always set isFinal to true to get the required output buffer
// size.
return (isFinal ?
Integer.max(Math.addExact((inLen - TAG_LENGTH),
getCipherBufferLength()), 0) : 0);
}

@Override
public int doUpdate(byte[] in, int inOff, int inLen, byte[] out,
int outOff) {
Expand Down Expand Up @@ -1155,7 +1236,10 @@ public synchronized int doFinal(byte[] in, int inOff, int inLen, byte[] out,

@Override
public int getCipherBufferLength() {
return cipherBuf.size();
if (cipherBuf != null) {
return cipherBuf.size();
}
return 0;
}
}

Expand Down

0 comments on commit 441da50

Please sign in to comment.