Merge branch 'advisory-fix-1'

Author: samrocketman

CHANGELOG.md

@@ -3,6 +3,28 @@
 This file contains all of the notable changes from Jervis releases.  For the
 full change log see the commit log.
 
+# jervis 2.2
+
+## Bug fixes
+
+- SecurityIO.sha256sum padding bug fixed.
+- SecurityIO switching to `AES/GCM/NoPadding` and old AES functions marked as
+  deprecated to be removed in Jervis 2.3.
+- SecurityIO.avoidTimingAttack uses cryptographically secure PRNG.
+- SecurityIO.verifyJsonWebToken does additional JWT structure validation.
+- SecurityIO.isBase64 now checks for base64 padding.  Previously, it was
+  possible for a String to pass as base64 characters but not be actually valid
+  base64.
+- SecurityIO.sha256Sum hex string padding fixed so resulting checksum is always
+  valid for 3rd party checksum functions.
+
+## Breaking changes
+
+- `CipherMap` encrypted data will be discarded when upgrading to Jervis 2.2.
+  Currently, this only affects GitHub app authentication in Jervis and new
+  tokens will be issued instead of reusing old tokens within their previous
+  expiration.  In general, GitHub app issued tokens expire after one hour.
+
 # jervis 2.1
 
 ## Bug fixes

src/main/groovy/net/gleske/jervis/tools/CipherMap.groovy

@@ -42,29 +42,20 @@ import javax.crypto.BadPaddingException
 
   <ul>
   <li>
-    Upon instantiation the AES cipher secret and initialization vector (IV) are
-    randomly generated bytes.  The random secret is 32 bytes and the random IV
-    is 16 bytes.
+    Upon instantiation the AES cipher secret is randomly generated (32 bytes).
+    A random 12-byte nonce is generated for each encryption operation.
   </li>
   <li>
-    The cipher secret and IV are asymmetrically encrypted with RSA.  The
-    stronger the RSA key provided the more secure the encryption at rest.
+    The cipher secret is asymmetrically encrypted with RSA using OAEP padding.
+    OAEP padding prevents Bleichenbacher padding oracle attacks. The stronger
+    the RSA key provided the more secure the encryption at rest.
     Keys below 2048-bits will throw an exception for being too weak.
     Recommended RSA private key size is 4096-bit.
   </li>
   <li>
-    The data is encrypted with AES-256 CBC with PKCS5 padding.  The cipher
-    secret and IV are the inputs for encryption and decryption.
-  </li>
-  <li>
-    The random IV is hashed with 5000 iterations of SHA-256 (let's call this
-    initialization hash) and the resulting hash is passed into PBKDF2 with Hmac
-    SHA-256 (PBKDF for short).  The PBKDF has variable iterations based on the
-    provided initialization hash.  The iterations for PBKDF range from 100100
-    to 960000 iterations.  Since this is configurable via
-    <tt>{@link #hash_iterations}</tt> it's possible to fully disable this
-    behavior (setting <tt>hash_iterations</tt> to zero) and only use the
-    provided secret and IV unmodified.
+    The data is encrypted with AES-256-GCM authenticated encryption.
+    GCM mode provides both confidentiality and integrity protection,
+    preventing padding oracle attacks and detecting tampering.
   </li>
   <li>
     After encryption, the encrypted value is signed with an RS256 signature
@@ -73,10 +64,9 @@ import javax.crypto.BadPaddingException
     An empty map is the default if the signature is invalid.
   </li>
   <li>
-    The cipher secret and IV infrequently change to protect against RSA attack
-    utilizing Chinese Remainder Theorem.  The cipher secret and IV are
-    automatically rotated if the secrets (secret/IV) are older than 30 days
-    when data is encrypted.
+    The cipher secret infrequently changes to protect against RSA attack
+    utilizing Chinese Remainder Theorem.  The cipher secret is automatically
+    rotated if the secret is older than 30 days when data is encrypted.
   </li>
   </ul>
 
@@ -113,15 +103,7 @@ timing = time {
 
 println("Time to load from String and decrypt: ${timing} second(s)")
 
-// re-encrypt with stronger security
-def cmap3 = new CipherMap(new File('src/test/resources/rsa_keys/good_id_rsa_4096').text)
-cmap3.hash_iterations = 100100
-
-timing = time {
-    cmap3.plainMap = cmap1.plainMap
-}
-println("Time migrating to stronger encryption with 100100 hash iterations: ${timing} second(s)")
-println(['\n', '='*80, 'Encrypted contents with CipherMap toString()'.with { ' '*(40 - it.size()/2) + it }, '='*80, "\n${cmap3}"].join('\n'))
+println(['\n', '='*80, 'Encrypted contents with CipherMap toString()'.with { ' '*(40 - it.size()/2) + it }, '='*80, "\n${cmap1}"].join('\n'))
 </code></pre>
   */
 class CipherMap implements Serializable {
@@ -136,16 +118,17 @@ class CipherMap implements Serializable {
     private transient Map hidden
 
     /**
-      Customize the number of SHA-256 hash iterations performed during AES
-      encryption operations.
+      Deprecated: This field is no longer used. AES-256-GCM mode uses random
+      nonces generated for each encryption operation instead of derived IVs.
 
-      @see net.gleske.jervis.tools.SecurityIO#DEFAULT_AES_ITERATIONS
+      @deprecated No longer used with AES-256-GCM authenticated encryption.
       */
+    @Deprecated
     Integer hash_iterations
 
     /**
-      The time limit in seconds before AES secret and IV need to be rotated.
-      Once it reaches this age then new secrets will be generated.  Default:
+      The time limit in seconds before AES secret needs to be rotated.
+      Once it reaches this age then a new secret will be generated.  Default:
       <tt>2592000</tt> seconds (number of seconds in 30 days).
 
       @see #setPlainMap(java.util.Map)
@@ -159,17 +142,18 @@ class CipherMap implements Serializable {
       @param privateKey A PKCS1 or PKCS8 private key PEM.
       */
     CipherMap(String privateKey) {
-        this(privateKey, SecurityIO.DEFAULT_AES_ITERATIONS)
+        this.security = new SecurityIO(privateKey)
     }
 
     /**
       Instantiates a new CipherMap object with the given private key.  This is
       used for asymmetric encryption wrapping symmetric encryption.
 
-      @see #hash_iterations
+      @deprecated The hash_iterations parameter is no longer used with AES-256-GCM.
       @param privateKey A PKCS1 or PKCS8 private key PEM.
-      @param hash_iterations Customize the hash iterations on instantiation.
+      @param hash_iterations Deprecated, ignored. AES-GCM uses random nonces.
       */
+    @Deprecated
     CipherMap(String privateKey, Integer hash_iterations) {
         this.hash_iterations = hash_iterations
         this.security = new SecurityIO(privateKey)
@@ -189,52 +173,47 @@ class CipherMap implements Serializable {
       Instantiates a new CipherMap object with the given private key.  This is
       used for asymmetric encryption wrapping symmetric encryption.
 
-      @see #hash_iterations
+      @deprecated The hash_iterations parameter is no longer used with AES-256-GCM.
       @param privateKey A PKCS1 or PKCS8 private key.
-      @param hash_iterations Customize the hash iterations on instantiation.
+      @param hash_iterations Deprecated, ignored. AES-GCM uses random nonces.
       */
+    @Deprecated
     CipherMap(File privateKey, Integer hash_iterations) {
         this(privateKey.text, hash_iterations)
     }
 
     /**
-      Encrypts the data with AES.
+      Encrypts the data with AES-256-GCM authenticated encryption.
 
       @param data To be encrypted.
       @return Returns encrypted String.
       */
     private String encrypt(String data) {
-        this.hidden.cipher.with { secret ->
-            return security.encryptWithAES256Base64(
-                security.encodeBase64(security.rsaDecryptBytes(security.decodeBase64Bytes(secret[0]))),
-                security.encodeBase64(security.rsaDecryptBytes(security.decodeBase64Bytes(secret[1]))),
-                data,
-                this.hash_iterations
-            )
-        }
+        String encryptedSecret = this.hidden.cipher
+        // Decrypt the RSA-wrapped AES secret using OAEP padding
+        byte[] aesSecret = security.rsaDecryptBytesOaep(security.decodeBase64Bytes(encryptedSecret))
+        // Encrypt data with AES-256-GCM (nonce is generated automatically and prepended)
+        SecurityIO.encryptWithAES256GCMBase64(security.encodeBase64(aesSecret), data)
     }
 
     /**
-      Decrypts the data with AES.
+      Decrypts the data with AES-256-GCM authenticated decryption.
       @param data To be decrypted.
       @return Returns the plaintext data.
       */
     private String decrypt(String data) {
-        this.hidden.cipher.with { secret ->
-            return security.decryptWithAES256Base64(
-                security.encodeBase64(security.rsaDecryptBytes(security.decodeBase64Bytes(secret[0]))),
-                security.encodeBase64(security.rsaDecryptBytes(security.decodeBase64Bytes(secret[1]))),
-                data,
-                this.hash_iterations
-            )
-        }
+        String encryptedSecret = this.hidden.cipher
+        // Decrypt the RSA-wrapped AES secret using OAEP padding
+        byte[] aesSecret = security.rsaDecryptBytesOaep(security.decodeBase64Bytes(encryptedSecret))
+        // Decrypt data with AES-256-GCM
+        SecurityIO.decryptWithAES256GCMBase64(security.encodeBase64(aesSecret), data)
     }
 
     /**
       Returns a string meant for signing and verifying signed data.
       */
     private String signedData(Map obj) {
-        ([obj.age] + obj.cipher + [obj.data]).join('\n')
+        [obj.age, obj.cipher, obj.data].join('\n')
     }
 
     private Boolean verifyCipherObj(def obj) {
@@ -245,13 +224,10 @@ class CipherMap implements Serializable {
         if(!(['age', 'cipher', 'data', 'signature'] == obj.keySet().toList())) {
             return false
         }
-        if(!((obj.cipher in List) && obj.cipher.size() == 2)) {
-            return false
-        }
+        // cipher is now a single String (RSA-OAEP encrypted AES secret)
         Boolean stringCheck = [
             obj.age,
-            obj.cipher[0],
-            obj.cipher[1],
+            obj.cipher,
             obj.data,
             obj.signature
         ].every { it in String }
@@ -268,23 +244,19 @@ class CipherMap implements Serializable {
     }
 
     /**
-      Creates a new cipher either for the first time or as part of rotation.
-      @return Returns a new random cipher secret and IV.
+      Creates a new cipher secret using RSA-OAEP encryption.
+      @return Returns a new RSA-OAEP encrypted random AES-256 secret.
       */
-    private List newCipher() {
-        // Get the max size an RSA key can encrypt.  Sometimes this is less
-        // than 256 bytes which means padding will be required.
-        Integer maxSize = [((security.getRsa_keysize() / 8) - 11), 256].min()
-        [
-            security.encodeBase64(security.rsaEncryptBytes(security.randomBytes(maxSize))),
-            security.encodeBase64(security.rsaEncryptBytes(security.randomBytes(16)))
-        ]
+    private String newCipher() {
+        // Generate a 32-byte (256-bit) random AES secret
+        // Use OAEP padding for RSA encryption to prevent Bleichenbacher attacks
+        security.encodeBase64(security.rsaEncryptBytesOaep(SecurityIO.randomBytes(32)))
     }
 
     private void initialize() {
         this.hidden = [
             age: '',
-            cipher: ['', ''],
+            cipher: '',
             data: '',
             signature: ''
         ]
@@ -384,11 +356,9 @@ class CipherMap implements Serializable {
       Returns an encrypted object as text meant for storing at rest.
 
 <pre><code>
-age: AES encrypted timestamp
-cipher:
-  - asymmetrically encrypted AES secret
-  - asymmetrically encrypted AES IV
-data: AES encrypted data
+age: AES-GCM encrypted timestamp
+cipher: RSA-OAEP encrypted AES-256 secret
+data: AES-GCM encrypted data (with nonce prepended)
 signature: RS256 Base64URL signature.
 </code></pre>