@@ -133,8 +133,7 @@ public static Encryptor getInstance() throws EncryptionException {
133133 // decryption fails. This is to prevent information leakage that may be
134134 // valuable in various forms of ciphertext attacks, such as the
135135 // Padded Oracle attack described by Rizzo and Duong.
136- private static final String DECRYPTION_FAILED =
137- "Decryption failed; see logs for details." ;
136+ private static final String DECRYPTION_FAILED = "Decryption failed; see logs for details." ;
138137
139138 // # of seconds that all failed decryption attempts will take. Used to
140139 // help prevent side-channel timing attacks.
@@ -236,21 +235,34 @@ private JavaEncryptor() throws EncryptionException {
236235 byte [] salt = ESAPI .securityConfiguration ().getMasterSalt ();
237236 byte [] skey = ESAPI .securityConfiguration ().getMasterKey ();
238237
239- assert salt != null : "Can't obtain master salt, Encryptor.MasterSalt" ;
240- assert salt .length >= 16 : "Encryptor.MasterSalt must be at least 16 bytes. " +
241- "Length is: " + salt .length + " bytes." ;
242- assert skey != null : "Can't obtain master key, Encryptor.MasterKey" ;
243- assert skey .length >= 7 : "Encryptor.MasterKey must be at least 7 bytes. " +
244- "Length is: " + skey .length + " bytes." ;
238+ if ( salt == null ) {
239+ throw new ConfigurationException ("Can't obtain master salt, Encryptor.MasterSalt" );
240+ }
241+
242+ if ( salt .length < 16 ) {
243+ throw new ConfigurationException ("Encryptor.MasterSalt must be at least 16 bytes. " +
244+ "Length is: " + salt .length + " bytes." );
245+ }
246+
247+ if ( skey == null ) {
248+ throw new ConfigurationException ("Can't obtain master key, Encryptor.MasterKey" );
249+ }
250+
251+ if ( skey .length < 7 ) {
252+ throw new ConfigurationException ("Encryptor.MasterKey must be at least 7 bytes. " +
253+ "Length is: " + skey .length + " bytes." );
254+ }
245255
246256 // Set up secretKeySpec for use for symmetric encryption and decryption,
247257 // and set up the public/private keys for asymmetric encryption /
248258 // decryption.
249- // TODO: Note: If we dump ESAPI 1.4 crypto backward compatibility,
250- // then we probably will ditch the Encryptor.EncryptionAlgorithm
259+ // TODO: Note: Since we've dumped ESAPI 1.4 crypto backward compatibility,
260+ // then we probably can ditch the Encryptor.EncryptionAlgorithm
251261 // property. If so, encryptAlgorithm should probably use
252262 // Encryptor.CipherTransformation and just pull off the cipher
253- // algorithm name so we can use it here.
263+ // algorithm name so we can use it here. That just requires
264+ // advance notice and proper deprecation, which I'm not prepared
265+ // to do at this time. -kevin wall
254266 synchronized (JavaEncryptor .class ) {
255267 if ( ! initialized ) {
256268 //
@@ -360,7 +372,10 @@ public CipherText encrypt(SecretKey key, PlainText plain)
360372 try {
361373 xform = ESAPI .securityConfiguration ().getCipherTransformation ();
362374 String [] parts = xform .split ("/" );
363- assert parts .length == 3 : "Malformed cipher transformation: " + xform ;
375+ if ( parts .length != 3 ) {
376+ throw new ConfigurationException ("Malformed cipher transformation: " + xform +
377+ ". Should have format of cipher_alg/cipher_mode/padding_scheme." );
378+ }
364379 String cipherMode = parts [1 ];
365380
366381 // This way we can prevent modes like OFB and CFB where the IV should never
@@ -383,63 +398,33 @@ public CipherText encrypt(SecretKey key, PlainText plain)
383398 // and this method will just call that one.
384399 Cipher encrypter = Cipher .getInstance (xform );
385400 String cipherAlg = encrypter .getAlgorithm ();
386- int keyLen = ESAPI .securityConfiguration ().getEncryptionKeyLength ();
387401
388- // DISCUSS: OK, what do we want to do here if keyLen != keySize? If use keyLen, encryption
389- // could fail with an exception, but perhaps that's what we want. Or we may just be
390- // OK with silently using keySize as long as keySize >= keyLen, which then interprets
391- // ESAPI.EncryptionKeyLength as the *minimum* key size, but as long as we have something
392- // stronger it's OK to use it. For now, I am just going to log warning if different, but use
393- // keySize unless keySize is SMALLER than ESAPI.EncryptionKeyLength, in which case I'm going
394- // to log an error.
395- //
396- // IMPORTANT NOTE: When we generate key sizes for both DES and DESede the result of
397- // SecretKey.getEncoding().length includes the TRUE key size (i.e.,
398- // *with* the even parity bits) rather than the EFFECTIVE key size
399- // (which incidentally is what KeyGenerator.init() expects for DES
400- // and DESede; duh! Nothing like being consistent). This leads to
401- // the following dilemma:
402- //
403- // EFFECTIVE Key Size TRUE Key Size
404- // (KeyGenerator.init()) (SecretKey.getEncoding().length)
405- // ========================================================================
406- // For DES: 56 bits 64 bits
407- // For DESede: 112 bits / 168 bits 192 bits (always)
408- //
409- // We are trying to automatically determine the key size from SecretKey
410- // based on 8 * SecretKey.getEncoding().length, but as you can see, the
411- // 2 key 3DES and the 3 key 3DES both use the same key size (192 bits)
412- // regardless of what is passed to KeyGenerator.init(). There are no advertised
413- // methods to get the key size specified by the init() method so I'm not sure how
414- // this is actually working internally. However, it does present a problem if we
415- // wish to communicate the 3DES key size to a recipient for later decryption as
416- // they would not be able to distinguish 2 key 3DES from 3 key 3DES.
417- //
418- // The only workaround I know is to pass the explicit key size down. However, if
419- // we are going to do that, I'd propose passing in a CipherSpec object so we could
420- // tell what cipher transformation to use as well instead of just the key size. Then
421- // we would extract keySize from the CipherSpec object of from the SecretKey object.
422- //
423- if ( keySize != keyLen ) {
424- // DISCUSS: Technically this is not a security "failure" per se, but not really a "success" either.
425- logger .warning (Logger .SECURITY_FAILURE , "Encryption key length mismatch. ESAPI.EncryptionKeyLength is " +
426- keyLen + " bits, but length of actual encryption key is " + keySize +
427- " bits. Did you remember to regenerate your master key (if that is what you are using)???" );
428- }
429- // DISCUSS: Reconsider these warnings. If thousands of encryptions are done in tight loop, no one needs
430- // more than 1 warning. Should we do something more intelligent here?
431- if ( keySize < keyLen ) {
432- // ESAPI.EncryptionKeyLength defaults to 128, but that means that we could not use DES (as weak as it
433- // is), even for legacy code. Therefore, this has been changed to simple log a warning rather than
434- // throw the following exception.
435- // throw new ConfigurationException("Actual key size of " + keySize + " bits smaller than specified " +
436- // "encryption key length (ESAPI.EncryptionKeyLength) of " + keyLen + " bits.");
437- logger .warning (Logger .SECURITY_FAILURE , "Actual key size of " + keySize + " bits SMALLER THAN specified " +
438- "encryption key length (ESAPI.EncryptionKeyLength) of " + keyLen + " bits with cipher algorithm " + cipherAlg );
402+ int minKeyLen = 112 ; // Use for hard-coded default to support 2TDEA
403+ try {
404+ minKeyLen = ESAPI .securityConfiguration ().getIntProp ("Encryptor.MinEncryptionKeyLength" );
405+ } catch ( Exception ex ) {
406+ logger .warning (Logger .EVENT_FAILURE ,
407+ "Property 'Encryptor.MinEncryptionKeyLength' not properly set in ESAPI.properties file; using hard coded default of 112 for min key size for encryption." ,
408+ ex );
409+ ; // Do NOT rethrow.
410+ }
411+
412+ if ( keySize < minKeyLen ) {
413+ // NOTE: This used to just log a warning. It now logs an error & throws an exception.
414+ //
415+ // ESAPI.EncryptionKeyLength defaults to 128. This means that someone wants to use ESAPI to
416+ // encrypt with something like 2-key TDES, they would have to set this to that property
417+ // to 112 bits.
418+ logger .error (Logger .SECURITY_FAILURE , "Actual key size of " + keySize + " bits SMALLER THAN MINIMUM allowed " +
419+ "encryption key length (ESAPI.EncryptionKeyLength) of " + minKeyLen + " bits with cipher algorithm " + cipherAlg );
420+ throw new ConfigurationException ("Actual key size of " + keySize + " bits smaller than specified " +
421+ "encryption key length (ESAPI.EncryptionKeyLength) of " + minKeyLen + " bits." );
439422 }
440423 if ( keySize < 112 ) { // NIST Special Pub 800-57 considers 112-bits to be the minimally safe key size from 2010-2030.
441- // Note that 112 bits 'just happens' to be size of 2-key Triple DES!
442- logger .warning (Logger .SECURITY_FAILURE , "Potentially unsecure encryption. Key size of " + keySize + "bits " +
424+ // Note that 112 bits 'just happens' to be size of 2-key Triple DES! So for example, if they
425+ // have configured ESAPI's Encryptor.EncryptionKeyLength to (say) 56 bits, we are going to
426+ // nag them like their mother! :)
427+ logger .warning (Logger .SECURITY_FAILURE , "Potentially insecure encryption. Key size of " + keySize + "bits " +
443428 "not sufficiently long for " + cipherAlg + ". Should use appropriate algorithm with key size " +
444429 "of *at least* 112 bits except when required by legacy apps. See NIST Special Pub 800-57." );
445430 }
@@ -548,9 +533,9 @@ public CipherText encrypt(SecretKey key, PlainText plain)
548533 // Don't overwrite anything in the case of exceptions because they may wish to retry.
549534 if ( success && overwritePlaintext ) {
550535 plain .overwrite (); // Note: Same as overwriting 'plaintext' byte array.
551- }
552- }
553- }
536+ }
537+ }
538+ }
554539
555540 /**
556541 * {@inheritDoc}
@@ -655,6 +640,8 @@ public PlainText decrypt(SecretKey key, CipherText ciphertext)
655640 // convert to from nanoseconds to milliseconds.
656641 long millis = extraSleep / 1000000L ;
657642 long nanos = (extraSleep - (millis * 1000000L ));
643+
644+ // N_SECS is hard-coded so assertion should be okay here.
658645 assert nanos >= 0 && nanos <= Integer .MAX_VALUE :
659646 "Nanosecs out of bounds; nanos = " + nanos ;
660647 try {
@@ -830,7 +817,8 @@ public String unseal(String seal) throws EncryptionException {
830817 cipherText = CipherText .fromPortableSerializedBytes (encryptedBytes );
831818 } catch ( AssertionError e ) {
832819 // Some of the tests in EncryptorTest.testVerifySeal() are examples of
833- // this if assertions are enabled.
820+ // this if assertions are enabled, but otherwise it should not
821+ // normally happen.
834822 throw new EncryptionException ("Invalid seal" ,
835823 "Seal passed garbarge data resulting in AssertionError: " + e );
836824 }
@@ -993,11 +981,16 @@ private SecretKey computeDerivedKey(int kdfVersion, KeyDerivationFunction.PRF_AL
993981 // We would choose a larger minimum key size, but we want to be
994982 // able to accept DES for legacy encryption needs. NIST says 112-bits is min. If less than that,
995983 // we print warning.
996- assert keySize >= 56 : "Key has size of " + keySize + ", which is less than minimum of 56-bits." ;
984+ assert keySize >= 56 : "Key has size of " + keySize + ", which is less than absolute minimum of 56-bits." ;
997985 assert (keySize % 8 ) == 0 : "Key size (" + keySize + ") must be a even multiple of 8-bits." ;
998- assert purpose != null : "Purpose cannot be null. Should be 'encryption' or 'authenticity'." ;
999- assert purpose .equals ("encryption" ) || purpose .equals ("authenticity" ) :
1000- "Purpose must be \" encryption\" or \" authenticity\" ." ;
986+
987+ // However, this one we want as a runtime check because we don't have this check
988+ // in KeyDerivationFunction.computeDerivedKey() as we want that method
989+ // to be more general.
990+ if ( !( purpose .equals ("encryption" ) || purpose .equals ("authenticity" ) ) ) {
991+ String exMsg = "Programming error in ESAPI?? 'purpose' for computeDerivedKey() must be \" encryption\" or \" authenticity\" ." ;
992+ throw new EncryptionException (exMsg , exMsg );
993+ }
1001994
1002995 KeyDerivationFunction kdf = new KeyDerivationFunction (prf );
1003996 if ( kdfVersion != 0 ) {
@@ -1034,7 +1027,8 @@ private static void initKeyPair(SecureRandom prng) throws NoSuchAlgorithmExcepti
10341027 // in the case that SHA1withDSA or SHAwithDSA was specified. This is
10351028 // all just to make these 2 work as expected. Sigh. (Note:
10361029 // this was tested with JDK 1.6.0_21, but likely fails with earlier
1037- // versions of the JDK as well.)
1030+ // versions of the JDK as well. Haven't experimented with later
1031+ // versions.)
10381032 //
10391033 sigAlg = "DSA" ;
10401034 } else if ( sigAlg .endsWith ("withrsa" ) ) {
0 commit comments