Skip to content

Commit ffcf064

Browse files
author
kevin.w.wall
committed
Changes related to decision to use a MAC rather than a MIC to indicate authenticity. Some method names were renamed as a result of these changes.
1 parent 4200d41 commit ffcf064

1 file changed

Lines changed: 51 additions & 71 deletions

File tree

‎src/main/java/org/owasp/esapi/reference/DefaultCipherText.java‎

Lines changed: 51 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -39,20 +39,20 @@
3939
// But if we do wish to restrict it to use from JavaEncryptor,
4040
// it I can assume this class will be used correctly and not be
4141
// concerned about making it act like it is immutable once everything
42-
// has been collected and the MIC has been computed...all of which make
42+
// has been collected and the MAC has been computed...all of which make
4343
// this class much more complex than it otherwise needs to be. But if
4444
// we are uncertain as to the context of how it will be used, then it's
4545
// probably best to be defensive.
4646
/**
4747
* Reference implementation of <code>CipherText</code>. This object is both
4848
* serializable, and once all the required information has been collected and
49-
* the Message Integrity Code (MIC) has been computed, it acts as though it is
49+
* the Message Authentication Code (MAC) has been computed, it acts as though it is
5050
* immutable as well. At that point, one can no longer call any of the 'setter'
5151
* methods.
5252
* <p>
5353
* <b>NOTE:</b> This class is <i>not</i> thread-safe.
5454
* </p><p>
55-
* Copyright (c) 2009 - The OWASP Foundation
55+
* Copyright &copy; 2009 - The OWASP Foundation
5656
* </p>
5757
* @author kevin.w.wall@gmail.com
5858
* @since 2.0
@@ -64,8 +64,7 @@ public final class DefaultCipherText implements CipherText {
6464

6565
private CipherSpec cipherSpec_ = null;
6666
private byte[] raw_ciphertext_ = null;
67-
private byte[] nonce_ = null;
68-
private byte[] mic_ = null;
67+
private byte[] mac_ = null;
6968

7069
// All the various pieces that can be set, either directly or indirectly
7170
// via CipherSpec.
@@ -234,57 +233,57 @@ public String getEncodedIVCipherText() {
234233
/**
235234
* {@inheritDoc}
236235
*/
237-
public void computeAndStoreMIC(byte[] raw_ciphertext) {
238-
assert !micComputed() : "Programming error: Can't store message integrity code while encrypting; " +
239-
"computeAndStoreMIC() called multiple times.";
240-
assert collectedAll() : "Have not collected all required information to compute and store MIC.";
241-
byte[] result = computeMIC(raw_ciphertext);
236+
public void computeAndStoreMAC(SecretKey authKey) {
237+
assert !macComputed() : "Programming error: Can't store message integrity code while encrypting; " +
238+
"computeAndStoreMAC() called multiple times.";
239+
assert collectedAll() : "Have not collected all required information to compute and store MAC.";
240+
byte[] result = computeMAC(authKey);
242241
if ( result != null ) {
243-
mic_ = new byte[ result.length ];
244-
CryptoHelper.copyByteArray(result, mic_);
245-
assert micComputed();
242+
mac_ = new byte[ result.length ];
243+
CryptoHelper.copyByteArray(result, mac_);
244+
assert macComputed();
246245
}
247-
// If 'result' is null, we already logged this in computeMIC().
246+
// If 'result' is null, we already logged this in computeMAC().
248247
}
249248

250249
/**
251250
* {@inheritDoc}
252251
*/
253-
public boolean validateMIC(byte[] secretKey) {
254-
boolean usesMIC = ESAPI.securityConfiguration().useMICforCipherText();
252+
public boolean validateMAC(SecretKey authKey) {
253+
boolean usesMAC = ESAPI.securityConfiguration().useMACforCipherText();
255254

256-
if ( usesMIC && micComputed() ) { // Uses MIC and it was computed
257-
// Calculate MIC from HMAC-SHA1(nonce, IV + plaintext) and
258-
// compare to stored value (mic_). If same, then return true,
255+
if ( usesMAC && macComputed() ) { // Uses MAC and it was computed
256+
// Calculate MAC from HMAC-SHA1(nonce, IV + plaintext) and
257+
// compare to stored value (mac_). If same, then return true,
259258
// else return false.
260-
assert getNonce() != null : "Cannot validate MIC while nonce is null.";
261-
byte[] mic = computeMIC(secretKey);
262-
assert mic.length == mic_.length : "MICs are of differnt lengths. Should both be the same.";
263-
for ( int i = 0; i < mic.length; i++ ) {
264-
if ( mic[i] != mic_[i] ) {
259+
byte[] mac = computeMAC(authKey);
260+
assert mac.length == mac_.length : "MACs are of differnt lengths. Should both be the same.";
261+
for ( int i = 0; i < mac.length; i++ ) {
262+
if ( mac[i] != mac_[i] ) {
265263
return false;
266264
}
267265
}
268266
return true;
269-
} else if ( ! usesMIC ) { // Doesn't use MIC
267+
} else if ( ! usesMAC ) { // Doesn't use MAC
270268
return true;
271-
} else { // Uses MIC but it has not been computed / stored.
272-
logger.warning(Logger.SECURITY_FAILURE, "Cannot validate MIC as it was never computed and stored. Decryption result may be garbage.");
273-
return true; // Need to return 'true' here because of CryptoHelper encrypt() / decrypt() methods.
269+
} else { // Uses MAC but it has not been computed / stored.
270+
logger.warning(Logger.SECURITY_FAILURE, "Cannot validate MAC as it was never computed and stored. " +
271+
"Decryption result may be garbage even when decryption succeeds.");
272+
return true; // Need to return 'true' here because of encrypt() / decrypt() methods don't support this.
274273
}
275274
}
276275

277276

278277
/**
279278
* Set the raw ciphertext.
280279
* @param ciphertext The raw ciphertext.
281-
* @throws EncryptionException Thrown if the MIC has already been computed
282-
* via {@link #computeAndStoreMIC(byte[])}.
280+
* @throws EncryptionException Thrown if the MAC has already been computed
281+
* via {@link #computeAndStoreMAC(byte[])}.
283282
*/
284283
public void setCiphertext(byte[] ciphertext)
285284
throws EncryptionException
286285
{
287-
if ( ! micComputed() ) {
286+
if ( ! macComputed() ) {
288287
if ( ciphertext == null || ciphertext.length == 0 ) {
289288
throw new EncryptionException("Encryption faled; no ciphertext",
290289
"Ciphertext may not be null or 0 length!");
@@ -295,7 +294,7 @@ public void setCiphertext(byte[] ciphertext)
295294
raw_ciphertext_ = new byte[ ciphertext.length ];
296295
CryptoHelper.copyByteArray(ciphertext, raw_ciphertext_);
297296
} else {
298-
String logMsg = "Programming error: Attempt to set ciphertext after MIC already computed.";
297+
String logMsg = "Programming error: Attempt to set ciphertext after MAC already computed.";
299298
logger.error(Logger.SECURITY_FAILURE, logMsg);
300299
throw new EncryptionException("Cannot store raw ciphertext.", logMsg);
301300
}
@@ -316,7 +315,7 @@ public void setIVandCiphertext(byte[] iv, byte[] ciphertext)
316315
if ( isCollected(CipherTextFlags.CIPHERTEXT) ) {
317316
logger.warning(Logger.SECURITY_FAILURE, "Raw ciphertext was already set; resetting.");
318317
}
319-
if ( ! micComputed() ) {
318+
if ( ! macComputed() ) {
320319
if ( ciphertext == null || ciphertext.length == 0 ) {
321320
throw new EncryptionException("Encryption faled; no ciphertext",
322321
"Ciphertext may not be null or 0 length!");
@@ -335,7 +334,7 @@ public void setIVandCiphertext(byte[] iv, byte[] ciphertext)
335334
setCiphertext( ciphertext );
336335
received(CipherTextFlags.CIPHERTEXT);
337336
} else {
338-
String logMsg = "MIC already computed from previously set IV and raw ciphertext; may not be reset -- object is immutable.";
337+
String logMsg = "MAC already computed from previously set IV and raw ciphertext; may not be reset -- object is immutable.";
339338
logger.error(Logger.SECURITY_FAILURE, logMsg); // Discuss: By throwing, this gets logged as warning, but it's really error! Why is an exception only a warning???
340339
throw new EncryptionException("Validation of decryption failed.", logMsg);
341340
}
@@ -355,71 +354,52 @@ public boolean requiresIV() {
355354
public String toString() {
356355
StringBuilder sb = new StringBuilder( "DefaultCipherText: " );
357356
String rawCipherText = (( getRawCipherText() != null ) ? "present" : "absent");
358-
String mic = (( mic_ != null ) ? "present" : "absent");
357+
String mac = (( mac_ != null ) ? "present" : "absent");
359358
sb.append("raw ciphertext is ").append(rawCipherText);
360-
sb.append(", MIC is ").append(mic).append("; ");
359+
sb.append(", MAC is ").append(mac).append("; ");
361360
sb.append( cipherSpec_.toString() );
362361
return sb.toString();
363362
}
364363

365364
//////////////////////////////////// P R I V A T E /////////////////////////////////////////
366365

367366
/**
368-
* Compute a MIC, but do not store it. May set the nonce value as a
369-
* side-effect. The MIC is calculated as:
367+
* Compute a MAC, but do not store it. May set the nonce value as a
368+
* side-effect. The MAC is calculated as:
370369
* <pre>
371370
* HMAC-SHA1(nonce, IV + plaintext)
372371
* </pre>
373-
* @param ciphertext The ciphertext value for which the MIC is computed.
374-
* @return The value for the MIC.
372+
* @param ciphertext The ciphertext value for which the MAC is computed.
373+
* @return The value for the MAC.
375374
*/
376-
private byte[] computeMIC(byte[] ciphertext) {
377-
assert ciphertext != null && ciphertext.length != 0 : "Plaintext may not be null or empty.";
375+
private byte[] computeMAC(SecretKey authKey) {
376+
assert raw_ciphertext_ != null && raw_ciphertext_.length != 0 : "Raw ciphertext may not be null or empty.";
377+
assert authKey != null && authKey.getEncoded().length != 0 : "Authenticity secret key may not be null or zero length.";
378378
try {
379-
KeyGenerator kg = KeyGenerator.getInstance("HmacSHA1");
380-
SecretKey sk = null;
381-
if ( nonce_ == null ) {
382-
sk = kg.generateKey();
383-
} else {
384-
sk = new SecretKeySpec(nonce_, "HmacSHA1");
385-
}
379+
SecretKey sk = new SecretKeySpec(authKey.getEncoded(), "HmacSHA1");
386380
Mac mac = Mac.getInstance("HmacSHA1");
387381
mac.init(sk);
388-
byte[] result = mac.doFinal(ciphertext);
389-
// POSSIBLE SIDE-EFFECT !!!!
390-
if ( nonce_ == null ) {
391-
nonce_ = sk.getEncoded(); // Side-effect -- nonce set here!
392-
assert nonce_ != null : "Failed to create nonce value.";
382+
if ( requiresIV() ) {
383+
mac.update( getIV() );
393384
}
385+
byte[] result = mac.doFinal( getRawCipherText() );
394386
return result;
395387
} catch (NoSuchAlgorithmException e) {
396388
e.printStackTrace(System.err);
397-
logger.error(Logger.SECURITY_FAILURE, "Cannot compute MIC w/out HmacSHA1.", e);
389+
logger.error(Logger.SECURITY_FAILURE, "Cannot compute MAC w/out HmacSHA1.", e);
398390
return null;
399391
} catch (InvalidKeyException e) {
400392
e.printStackTrace(System.err);
401-
logger.error(Logger.SECURITY_FAILURE, "Cannot comput MIC; invalid 'key' for HmacSHA1.", e);
393+
logger.error(Logger.SECURITY_FAILURE, "Cannot comput MAC; invalid 'key' for HmacSHA1.", e);
402394
return null;
403395
}
404396
}
405397

406398
/**
407-
* Return true if the MIC has already been computed (i.e., not null).
399+
* Return true if the MAC has already been computed (i.e., not null).
408400
*/
409-
private boolean micComputed() {
410-
return (mic_ != null);
411-
}
412-
413-
/**
414-
* Retrieve the nonce value used in the calculation of the MIC.
415-
*/
416-
private byte[] getNonce() {
417-
if ( micComputed() ) {
418-
return nonce_;
419-
} else {
420-
logger.error(Logger.SECURITY_FAILURE, "Nonce for MIC not set yet; unable to retrieve; returning null");
421-
return null;
422-
}
401+
private boolean macComputed() {
402+
return (mac_ != null);
423403
}
424404

425405
/**

0 commit comments

Comments
 (0)