Skip to content

Commit 06cfbe7

Browse files
committed
Replace assertions with explicit runtime checks.
Numerous 'assert' statements were replaced with explicit runtime checks where warranted. Also, mass replace tabs with 4 spaces.
1 parent d4d93c5 commit 06cfbe7

File tree

1 file changed

+86
-84
lines changed

1 file changed

+86
-84
lines changed

src/main/java/org/owasp/esapi/crypto/CipherTextSerializer.java

Lines changed: 86 additions & 84 deletions
Original file line numberDiff line numberDiff line change
@@ -29,18 +29,18 @@
2929
* serialization technique so they could exchange encrypted data.)
3030
*
3131
32+
* @since 2.0
3233
*
3334
*/
3435
public class CipherTextSerializer {
3536
// This should be *same* version as in CipherText & KeyDerivationFunction as
36-
// these versions all need to work together. Therefore, when one changes one
37-
// one these versions, the other should be reviewed and changed as well to
38-
// accommodate any differences.
39-
// Previous versions: 20110203 - Original version (ESAPI releases 2.0 & 2.0.1)
40-
// 20130830 - Fix to issue #306 (release 2.1.0)
41-
// We check that in an static initialization block (when assertions are enabled)
42-
// below.
43-
public static final int cipherTextSerializerVersion = 20130830; // Current version. Format: YYYYMMDD, max is 99991231.
37+
// these versions all need to work together. Therefore, when one changes one
38+
// one these versions, the other should be reviewed and changed as well to
39+
// accommodate any differences.
40+
// Previous versions: 20110203 - Original version (ESAPI releases 2.0 & 2.0.1)
41+
// 20130830 - Fix to issue #306 (release 2.1.0)
42+
// We check that in an static initialization block below.
43+
public static final int cipherTextSerializerVersion = 20130830; // Current version. Format: YYYYMMDD, max is 99991231.
4444
private static final long serialVersionUID = cipherTextSerializerVersion;
4545

4646
private static final Logger logger = ESAPI.getLogger("CipherTextSerializer");
@@ -50,18 +50,20 @@ public class CipherTextSerializer {
5050
// Check if versions of KeyDerivationFunction, CipherText, and
5151
// CipherTextSerializer are all the same.
5252
{
53-
// Ignore error about comparing identical versions and dead code.
54-
// We expect them to be, but the point is to catch us if they aren't.
55-
assert CipherTextSerializer.cipherTextSerializerVersion == CipherText.cipherTextVersion :
56-
"Versions of CipherTextSerializer and CipherText are not compatible.";
57-
assert CipherTextSerializer.cipherTextSerializerVersion == KeyDerivationFunction.kdfVersion :
58-
"Versions of CipherTextSerializer and KeyDerivationFunction are not compatible.";
53+
// Ignore error about comparing identical versions and dead code.
54+
// We expect them to be, but the point is to catch us if they aren't.
55+
if ( CipherTextSerializer.cipherTextSerializerVersion != CipherText.cipherTextVersion ) {
56+
throw new ExceptionInInitializerError("Versions of CipherTextSerializer and CipherText are not compatible.");
57+
}
58+
if ( CipherTextSerializer.cipherTextSerializerVersion != KeyDerivationFunction.kdfVersion ) {
59+
throw new ExceptionInInitializerError("Versions of CipherTextSerializer and KeyDerivationFunction are not compatible.");
60+
}
5961
}
6062

6163
public CipherTextSerializer(CipherText cipherTextObj) {
62-
if ( cipherTextObj == null ) {
63-
throw new IllegalArgumentException("CipherText object must not be null.");
64-
}
64+
if ( cipherTextObj == null ) {
65+
throw new IllegalArgumentException("CipherText object must not be null.");
66+
}
6567
cipherText_ = cipherTextObj;
6668
}
6769

@@ -90,28 +92,28 @@ public byte[] asSerializedByteArray() {
9092
long timestamp = cipherText_.getEncryptionTimestamp();
9193
String cipherXform = cipherText_.getCipherTransformation();
9294
if ( cipherText_.getKeySize() >= Short.MAX_VALUE ) {
93-
throw new IllegalArgumentException("Key size is too large. Max is " + Short.MAX_VALUE);
95+
throw new IllegalArgumentException("Key size is too large. Max is " + Short.MAX_VALUE);
9496
}
9597
short keySize = (short) cipherText_.getKeySize();
9698
if ( cipherText_.getBlockSize() >= Short.MAX_VALUE ) {
97-
throw new IllegalArgumentException("Block size is too large. Max is " + Short.MAX_VALUE);
99+
throw new IllegalArgumentException("Block size is too large. Max is " + Short.MAX_VALUE);
98100
}
99101
short blockSize = (short) cipherText_.getBlockSize();
100102
byte[] iv = cipherText_.getIV();
101103
if ( iv.length >= Short.MAX_VALUE ) {
102-
throw new IllegalArgumentException("IV size too large. Max is " + Short.MAX_VALUE + " bytes");
104+
throw new IllegalArgumentException("IV size too large. Max is " + Short.MAX_VALUE + " bytes");
103105
}
104106
short ivLen = (short) iv.length;
105107
byte[] rawCiphertext = cipherText_.getRawCipherText();
106108
int ciphertextLen = rawCiphertext.length;
107109
// Coverity issue 1352406, GitHub issue # 364 - possible NPE later if assertion disabled.
108110
// Replaced assertion with explicit check.
109111
if ( ciphertextLen < 1 ) {
110-
throw new IllegalArgumentException("Raw ciphertext length must be >= 1 byte.");
112+
throw new IllegalArgumentException("Raw ciphertext length must be >= 1 byte.");
111113
}
112114
byte[] mac = cipherText_.getSeparateMAC();
113115
if ( mac.length >= Short.MAX_VALUE ) {
114-
throw new IllegalArgumentException("MAC length too large. Max is " + Short.MAX_VALUE + " bytes");
116+
throw new IllegalArgumentException("MAC length too large. Max is " + Short.MAX_VALUE + " bytes");
115117
}
116118
short macLen = (short) mac.length;
117119

@@ -136,36 +138,36 @@ public byte[] asSerializedByteArray() {
136138
* @return The {@code CipherText} object that we are serializing.
137139
*/
138140
public CipherText asCipherText() {
139-
if ( cipherText_ == null ) {
140-
throw new IllegalArgumentException("Program error? CipherText object, cipherText_, must not be null.");
141-
}
141+
if ( cipherText_ == null ) {
142+
throw new IllegalArgumentException("Program error? CipherText object, cipherText_, must not be null.");
143+
}
142144
return cipherText_;
143145
}
144146

145147
/**
146148
* Take all the individual elements that make of the serialized ciphertext
147149
* format and put them in order and return them as a byte array.
148-
* @param kdfInfo Info about the KDF... which PRF and the KDF version {@link #asCipherText()}.
149-
* @param timestamp Timestamp when the data was encrypted. Intended to help
150-
* facilitate key change operations and nothing more. If it is meaningless,
151-
* then the expectations are just that the recipient should ignore it. Mostly
152-
* intended when encrypted data is kept long term over a period of many
153-
* key change operations.
154-
* @param cipherXform Details of how the ciphertext was encrypted. The format used
155-
* is the same as used by {@code javax.crypto.Cipher}, namely,
156-
* "cipherAlg/cipherMode/paddingScheme".
157-
* @param keySize The key size used for encrypting. Intended for cipher algorithms
158-
* supporting multiple key sizes such as triple DES (DESede) or
159-
* Blowfish.
160-
* @param blockSize The cipher block size. Intended to support cipher algorithms
161-
* that support variable block sizes, such as Rijndael.
162-
* @param ivLen The length of the IV.
163-
* @param iv The actual IV (initialization vector) bytes.
164-
* @param ciphertextLen The length of the raw ciphertext.
165-
* @param rawCiphertext The actual raw ciphertext itself
166-
* @param macLen The length of the MAC (message authentication code).
167-
* @param mac The MAC itself.
168-
* @return A byte array representing the serialized ciphertext.
150+
* @param kdfInfo Info about the KDF... which PRF and the KDF version {@link #asCipherText()}.
151+
* @param timestamp Timestamp when the data was encrypted. Intended to help
152+
* facilitate key change operations and nothing more. If it is meaningless,
153+
* then the expectations are just that the recipient should ignore it. Mostly
154+
* intended when encrypted data is kept long term over a period of many
155+
* key change operations.
156+
* @param cipherXform Details of how the ciphertext was encrypted. The format used
157+
* is the same as used by {@code javax.crypto.Cipher}, namely,
158+
* "cipherAlg/cipherMode/paddingScheme".
159+
* @param keySize The key size used for encrypting. Intended for cipher algorithms
160+
* supporting multiple key sizes such as triple DES (DESede) or
161+
* Blowfish.
162+
* @param blockSize The cipher block size. Intended to support cipher algorithms
163+
* that support variable block sizes, such as Rijndael.
164+
* @param ivLen The length of the IV.
165+
* @param iv The actual IV (initialization vector) bytes.
166+
* @param ciphertextLen The length of the raw ciphertext.
167+
* @param rawCiphertext The actual raw ciphertext itself
168+
* @param macLen The length of the MAC (message authentication code).
169+
* @param mac The MAC itself.
170+
* @return A byte array representing the serialized ciphertext.
169171
*/
170172
private byte[] computeSerialization(int kdfInfo, long timestamp,
171173
String cipherXform, short keySize,
@@ -297,10 +299,10 @@ private long readLong(ByteArrayInputStream bais)
297299

298300
/** Convert the serialized ciphertext byte array to a {@code CipherText}
299301
* object.
300-
* @param cipherTextSerializedBytes The serialized ciphertext as a byte array.
302+
* @param cipherTextSerializedBytes The serialized ciphertext as a byte array.
301303
* @return The corresponding {@code CipherText} object.
302-
* @throws EncryptionException Thrown if the byte array data is corrupt or
303-
* there are version mismatches, etc.
304+
* @throws EncryptionException Thrown if the byte array data is corrupt or
305+
* there are version mismatches, etc.
304306
*/
305307
private CipherText convertToCipherText(byte[] cipherTextSerializedBytes)
306308
throws EncryptionException
@@ -309,7 +311,7 @@ private CipherText convertToCipherText(byte[] cipherTextSerializedBytes)
309311
if ( cipherTextSerializedBytes == null ) {
310312
throw new IllegalArgumentException("cipherTextSerializedBytes cannot be null.");
311313
}
312-
if ( cipherTextSerializedBytes.length == 0 ) {
314+
if ( cipherTextSerializedBytes.length == 0 ) {
313315
throw new IllegalArgumentException("cipherTextSerializedBytes must be > 0 in length.");
314316
}
315317
ByteArrayInputStream bais = new ByteArrayInputStream(cipherTextSerializedBytes);
@@ -325,22 +327,22 @@ private CipherText convertToCipherText(byte[] cipherTextSerializedBytes)
325327

326328
// First do a quick sanity check on the argument. Previously this was an assertion.
327329
if ( ! CryptoHelper.isValidKDFVersion(kdfVers, false, false) ) {
328-
// TODO: Clean up. Use StringBuilder. Good enough for now.
329-
String logMsg = "KDF version read from serialized ciphertext (" + kdfVers + ") is out of range. " +
330-
"Valid range for KDF version is [" + KeyDerivationFunction.originalVersion + ", " +
331-
"99991231].";
332-
// This should never happen under actual circumstances (barring programming errors; but we've
333-
// tested the code, right?), so it is likely an attempted attack. Thus don't get the originator
334-
// of the suspect ciphertext too much info. They ought to know what they sent anyhow.
335-
throw new EncryptionException("Version info from serialized ciphertext not in valid range.",
336-
"Likely tampering with KDF version on serialized ciphertext." + logMsg);
330+
// TODO: Clean up. Use StringBuilder. Good enough for now.
331+
String logMsg = "KDF version read from serialized ciphertext (" + kdfVers + ") is out of range. " +
332+
"Valid range for KDF version is [" + KeyDerivationFunction.originalVersion + ", " +
333+
"99991231].";
334+
// This should never happen under actual circumstances (barring programming errors; but we've
335+
// tested the code, right?), so it is likely an attempted attack. Thus don't get the originator
336+
// of the suspect ciphertext too much info. They ought to know what they sent anyhow.
337+
throw new EncryptionException("Version info from serialized ciphertext not in valid range.",
338+
"Likely tampering with KDF version on serialized ciphertext." + logMsg);
337339
}
338340

339341
debug("convertToCipherText: kdfPrf = " + kdfPrf + ", kdfVers = " + kdfVers);
340342
if ( ! versionIsCompatible( kdfVers) ) {
341-
throw new EncryptionException("This version of ESAPI is not compatible with the version of ESAPI that encrypted your data.",
342-
"KDF version " + kdfVers + " from serialized ciphertext not compatibile with current KDF version of " +
343-
KeyDerivationFunction.kdfVersion);
343+
throw new EncryptionException("This version of ESAPI is not compatible with the version of ESAPI that encrypted your data.",
344+
"KDF version " + kdfVers + " from serialized ciphertext not compatibile with current KDF version of " +
345+
KeyDerivationFunction.kdfVersion);
344346
}
345347
long timestamp = readLong(bais);
346348
debug("convertToCipherText: timestamp = " + new Date(timestamp));
@@ -391,7 +393,7 @@ private CipherText convertToCipherText(byte[] cipherTextSerializedBytes)
391393
CipherText ct = new CipherText(cipherSpec);
392394
if ( ! (ivLen > 0 && ct.requiresIV()) ) {
393395
throw new EncryptionException("convertToCipherText: Mismatch between IV length and cipher mode.",
394-
"Possible tampering of serialized ciphertext?");
396+
"Possible tampering of serialized ciphertext?");
395397
}
396398
ct.setCiphertext(rawCiphertext);
397399
// Set this *AFTER* setting raw ciphertext because setCiphertext()
@@ -400,11 +402,11 @@ private CipherText convertToCipherText(byte[] cipherTextSerializedBytes)
400402
if ( macLen > 0 ) {
401403
ct.storeSeparateMAC(mac);
402404
}
403-
// Fixed in ESAPI crypto version 20130839. Previously is didn't really matter
404-
// because there was only one version (20110203) and it defaulted to that
405-
// version, which was the current version. But we don't want that as now there
406-
// are two versions and we could be decrypting data encrypted using the previous
407-
// version.
405+
// Fixed in ESAPI crypto version 20130839. Previously is didn't really matter
406+
// because there was only one version (20110203) and it defaulted to that
407+
// version, which was the current version. But we don't want that as now there
408+
// are two versions and we could be decrypting data encrypted using the previous
409+
// version.
408410
ct.setKDF_PRF(kdfPrf);
409411
ct.setKDFVersion(kdfVers);
410412
return ct;
@@ -428,28 +430,28 @@ private CipherText convertToCipherText(byte[] cipherTextSerializedBytes)
428430
* both versions work the same. This checking may get more complicated in
429431
* the future.
430432
*
431-
* @param readKdfVers The version information extracted from the serialized
432-
* ciphertext.
433+
* @param readKdfVers The version information extracted from the serialized
434+
* ciphertext.
433435
*/
434436
private static boolean versionIsCompatible(int readKdfVers) {
435437
if ( readKdfVers <= 0 ) {
436438
throw new IllegalArgumentException("Extracted KDF version is <= 0. Must be integer >= 1.");
437439
}
438-
439-
switch ( readKdfVers ) {
440-
case KeyDerivationFunction.originalVersion: // First version
441-
return true;
442-
// Add new versions here; hard coding is OK...
443-
// case YYYYMMDD:
444-
// return true;
445-
case KeyDerivationFunction.kdfVersion: // Current version
446-
return true;
447-
default:
448-
return false;
449-
}
450-
}
440+
441+
switch ( readKdfVers ) {
442+
case KeyDerivationFunction.originalVersion: // First version
443+
return true;
444+
// Add new versions here; hard coding is OK...
445+
// case YYYYMMDD:
446+
// return true;
447+
case KeyDerivationFunction.kdfVersion: // Current version
448+
return true;
449+
default:
450+
return false;
451+
}
452+
}
451453

452-
private void debug(String msg) {
454+
private void debug(String msg) {
453455
if ( logger.isDebugEnabled() ) {
454456
logger.debug(Logger.EVENT_SUCCESS, msg);
455457
}

0 commit comments

Comments
 (0)