Skip to content
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.

Sign up for GitHub

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

Move 2 engineDoFinal methods from OpenSSLCipher.java to its subclasses. #1264

Merged
merged 7 commits into from
Dec 9, 2024
Merged

Move 2 engineDoFinal methods from OpenSSLCipher.java to its subclasses. #1264

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
14c4d10
Move engineDoFinal to subclasses.
juergw Nov 26, 2024
4445fa5
Revert some unrelated changes.
juergw Nov 26, 2024
f6df77e
Fix whitespaces.
juergw Nov 26, 2024
2828968
Add tests that call Cipher with different call patterns.
juergw Dec 4, 2024
5d654bc
Merge branch 'google:master' into master
juergw Dec 4, 2024
1c6f736
Also cover update with output array in Cipher tests.
juergw Dec 4, 2024
a7fe2cb
Merge branch 'master' of https://github.com/juergw/conscrypt
juergw Dec 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 55 additions & 5 deletions common/src/main/java/org/conscrypt/OpenSSLAeadCipher.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ protected int engineDoFinal(ByteBuffer input, ByteBuffer output) throws ShortBuf
throw new IllegalArgumentException("Cannot write to Read Only ByteBuffer");
}
if (bufCount != 0) {
return super.engineDoFinal(input, output);// traditional case
return super.engineDoFinal(input, output); // traditional case
}
int bytesWritten;
if (!input.isDirect()) {
Expand All @@ -268,20 +268,71 @@ protected int engineDoFinal(ByteBuffer input, ByteBuffer output) throws ShortBuf
return bytesWritten;
}

@Override
protected byte[] engineDoFinal(byte[] input, int inputOffset, int inputLen)
throws IllegalBlockSizeException, BadPaddingException {
final int maximumLen = getOutputSizeForFinal(inputLen);
/* Assume that we'll output exactly on a byte boundary. */
final byte[] output = new byte[maximumLen];

int bytesWritten;
if (inputLen > 0) {
try {
bytesWritten = updateInternal(input, inputOffset, inputLen, output, 0, maximumLen);
} catch (ShortBufferException e) {
/* This should not happen since we sized our own buffer. */
throw new RuntimeException("our calculated buffer was too small", e);
}
} else {
bytesWritten = 0;
}

try {
bytesWritten += doFinalInternal(output, bytesWritten, maximumLen - bytesWritten);
} catch (ShortBufferException e) {
/* This should not happen since we sized our own buffer. */
throw new RuntimeException("our calculated buffer was too small", e);
}

if (bytesWritten == output.length) {
return output;
} else if (bytesWritten == 0) {
return EmptyArray.BYTE;
} else {
return Arrays.copyOfRange(output, 0, bytesWritten);
}
}

@Override
protected int engineDoFinal(byte[] input, int inputOffset, int inputLen, byte[] output,
int outputOffset) throws ShortBufferException, IllegalBlockSizeException,
BadPaddingException {
// Because the EVP_AEAD updateInternal processes input but doesn't create any output
// (and thus can't check the output buffer), we need to add this check before the
// superclass' processing to ensure that updateInternal is never called if the
// (and thus can't check the output buffer), we need to add this check
// to ensure that updateInternal is never called if the
// output buffer isn't large enough.
if (output != null) {
if (getOutputSizeForFinal(inputLen) > output.length - outputOffset) {
throw new ShortBufferWithoutStackTraceException("Insufficient output space");
}
}
return super.engineDoFinal(input, inputOffset, inputLen, output, outputOffset);
if (output == null) {
throw new NullPointerException("output == null");
}

int maximumLen = getOutputSizeForFinal(inputLen);

final int bytesWritten;
if (inputLen > 0) {
bytesWritten = updateInternal(input, inputOffset, inputLen, output, outputOffset,
maximumLen);
outputOffset += bytesWritten;
maximumLen -= bytesWritten;
} else {
bytesWritten = 0;
}

return bytesWritten + doFinalInternal(output, outputOffset, maximumLen);
}

@Override
Expand Down Expand Up @@ -351,7 +402,6 @@ int doFinalInternal(ByteBuffer input, ByteBuffer output)
return bytesWritten;
}

@Override
int doFinalInternal(byte[] output, int outputOffset, int maximumLen)
throws ShortBufferException, IllegalBlockSizeException, BadPaddingException {
checkInitialization();
Expand Down
68 changes: 0 additions & 68 deletions common/src/main/java/org/conscrypt/OpenSSLCipher.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,16 +151,6 @@ abstract void engineInitInternal(byte[] encodedKey, AlgorithmParameterSpec param
abstract int updateInternal(byte[] input, int inputOffset, int inputLen,
byte[] output, int outputOffset, int maximumLen) throws ShortBufferException;

/**
* API-specific implementation of the final block. The {@code maximumLen}
* will be the maximum length of the possible output as returned by
* {@link #getOutputSizeForFinal(int)}. The return value must be the number
* of bytes processed and placed into {@code output}. On error, an exception
* must be thrown.
*/
abstract int doFinalInternal(byte[] output, int outputOffset, int maximumLen)
throws IllegalBlockSizeException, BadPaddingException, ShortBufferException;

/**
* Returns the standard name for the particular algorithm.
*/
Expand Down Expand Up @@ -349,64 +339,6 @@ protected int engineUpdate(byte[] input, int inputOffset, int inputLen, byte[] o
return updateInternal(input, inputOffset, inputLen, output, outputOffset, maximumLen);
}

@Override
protected byte[] engineDoFinal(byte[] input, int inputOffset, int inputLen)
throws IllegalBlockSizeException, BadPaddingException {
final int maximumLen = getOutputSizeForFinal(inputLen);
/* Assume that we'll output exactly on a byte boundary. */
final byte[] output = new byte[maximumLen];

int bytesWritten;
if (inputLen > 0) {
try {
bytesWritten = updateInternal(input, inputOffset, inputLen, output, 0, maximumLen);
} catch (ShortBufferException e) {
/* This should not happen since we sized our own buffer. */
throw new RuntimeException("our calculated buffer was too small", e);
}
} else {
bytesWritten = 0;
}

try {
bytesWritten += doFinalInternal(output, bytesWritten, maximumLen - bytesWritten);
} catch (ShortBufferException e) {
/* This should not happen since we sized our own buffer. */
throw new RuntimeException("our calculated buffer was too small", e);
}

if (bytesWritten == output.length) {
return output;
} else if (bytesWritten == 0) {
return EmptyArray.BYTE;
} else {
return Arrays.copyOfRange(output, 0, bytesWritten);
}
}

@Override
protected int engineDoFinal(byte[] input, int inputOffset, int inputLen, byte[] output,
int outputOffset) throws ShortBufferException, IllegalBlockSizeException,
BadPaddingException {
if (output == null) {
throw new NullPointerException("output == null");
}

int maximumLen = getOutputSizeForFinal(inputLen);

final int bytesWritten;
if (inputLen > 0) {
bytesWritten = updateInternal(input, inputOffset, inputLen, output, outputOffset,
maximumLen);
outputOffset += bytesWritten;
maximumLen -= bytesWritten;
} else {
bytesWritten = 0;
}

return bytesWritten + doFinalInternal(output, outputOffset, maximumLen);
}

@Override
protected byte[] engineWrap(Key key) throws IllegalBlockSizeException, InvalidKeyException {
try {
Expand Down
56 changes: 54 additions & 2 deletions common/src/main/java/org/conscrypt/OpenSSLCipherChaCha20.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@
import java.security.NoSuchAlgorithmException;
import java.security.SecureRandom;
import java.security.spec.AlgorithmParameterSpec;
import java.util.Arrays;
import javax.crypto.BadPaddingException;
import javax.crypto.IllegalBlockSizeException;
import javax.crypto.NoSuchPaddingException;
import javax.crypto.ShortBufferException;
import javax.crypto.spec.IvParameterSpec;
Expand Down Expand Up @@ -101,9 +104,58 @@ int updateInternal(byte[] input, int inputOffset, int inputLen, byte[] output, i
}

@Override
int doFinalInternal(byte[] output, int outputOffset, int maximumLen) {
protected byte[] engineDoFinal(byte[] input, int inputOffset, int inputLen)
throws IllegalBlockSizeException, BadPaddingException {
final int maximumLen = getOutputSizeForFinal(inputLen);
/* Assume that we'll output exactly on a byte boundary. */
final byte[] output = new byte[maximumLen];

int bytesWritten;
if (inputLen > 0) {
try {
bytesWritten = updateInternal(input, inputOffset, inputLen, output, 0, maximumLen);
} catch (ShortBufferException e) {
/* This should not happen since we sized our own buffer. */
throw new RuntimeException("our calculated buffer was too small", e);
}
} else {
bytesWritten = 0;
}

reset();
return 0;

if (bytesWritten == output.length) {
return output;
} else if (bytesWritten == 0) {
return EmptyArray.BYTE;
} else {
return Arrays.copyOfRange(output, 0, bytesWritten);
}
}

@Override
protected int engineDoFinal(byte[] input, int inputOffset, int inputLen, byte[] output,
int outputOffset) throws ShortBufferException, IllegalBlockSizeException,
BadPaddingException {
if (output == null) {
throw new NullPointerException("output == null");
}

int maximumLen = getOutputSizeForFinal(inputLen);

final int bytesWritten;
if (inputLen > 0) {
bytesWritten = updateInternal(input, inputOffset, inputLen, output, outputOffset,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No tests seem to hit this branch, could you add one, please.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. The Cipher interface has many similar methods, and depending on how exactly it is called, a different method of the implementation is executed. I've now changed the tests of Cipher so that it tries a range of different call patterns. I think this should give a better coverage.

maximumLen);
outputOffset += bytesWritten;
maximumLen -= bytesWritten;
} else {
bytesWritten = 0;
}

reset();

return bytesWritten;
}

private void reset() {
Expand Down
60 changes: 59 additions & 1 deletion common/src/main/java/org/conscrypt/OpenSSLEvpCipher.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import java.security.InvalidKeyException;
import java.security.SecureRandom;
import java.security.spec.AlgorithmParameterSpec;
import java.util.Arrays;
import javax.crypto.BadPaddingException;
import javax.crypto.IllegalBlockSizeException;
import javax.crypto.ShortBufferException;
Expand Down Expand Up @@ -127,7 +128,6 @@ int updateInternal(byte[] input, int inputOffset, int inputLen, byte[] output,
return outputOffset - intialOutputOffset;
}

@Override
int doFinalInternal(byte[] output, int outputOffset, int maximumLen)
throws IllegalBlockSizeException, BadPaddingException, ShortBufferException {
/* Remember this so we can tell how many characters were written. */
Expand Down Expand Up @@ -163,6 +163,64 @@ int doFinalInternal(byte[] output, int outputOffset, int maximumLen)
return outputOffset - initialOutputOffset;
}

@Override
protected byte[] engineDoFinal(byte[] input, int inputOffset, int inputLen)
throws IllegalBlockSizeException, BadPaddingException {
final int maximumLen = getOutputSizeForFinal(inputLen);
/* Assume that we'll output exactly on a byte boundary. */
final byte[] output = new byte[maximumLen];

int bytesWritten;
if (inputLen > 0) {
try {
bytesWritten = updateInternal(input, inputOffset, inputLen, output, 0, maximumLen);
} catch (ShortBufferException e) {
/* This should not happen since we sized our own buffer. */
throw new RuntimeException("our calculated buffer was too small", e);
}
} else {
bytesWritten = 0;
}

try {
bytesWritten += doFinalInternal(output, bytesWritten, maximumLen - bytesWritten);
} catch (ShortBufferException e) {
/* This should not happen since we sized our own buffer. */
throw new RuntimeException("our calculated buffer was too small", e);
}

if (bytesWritten == output.length) {
return output;
} else if (bytesWritten == 0) {
return EmptyArray.BYTE;
} else {
return Arrays.copyOfRange(output, 0, bytesWritten);
}
}

@Override
protected int engineDoFinal(byte[] input, int inputOffset, int inputLen, byte[] output,
int outputOffset) throws ShortBufferException, IllegalBlockSizeException,
BadPaddingException {
if (output == null) {
throw new NullPointerException("output == null");
}

int maximumLen = getOutputSizeForFinal(inputLen);

final int bytesWritten;
if (inputLen > 0) {
bytesWritten = updateInternal(input, inputOffset, inputLen, output, outputOffset,
maximumLen);
outputOffset += bytesWritten;
maximumLen -= bytesWritten;
} else {
bytesWritten = 0;
}

return bytesWritten + doFinalInternal(output, outputOffset, maximumLen);
}

@Override
int getOutputSizeForFinal(int inputLen) {
if (modeBlockSize == 1) {
Expand Down
Loading
Loading