Skip to content

Conversation

@akafredperry
Copy link
Collaborator

@akafredperry akafredperry commented Jan 28, 2026

- What I did
This PR introduces the spotless and checkstyle plugins to the maven lifecyle. To ensure that all the code conforms a single formatting convention (spotless) and established coding style practices (checkstyle).

In order to pass, the code base has been adjusted.

In the intention here is to standardize all the code in a single commit and make future PRs easier to diff.

- How I did it
Added checkstyle and spotless maven plugins
Added java spotless rules
Added java checkstyle
Fixed checkstyle failures (lots of IF without braces, some deep block nesting)
Fixed spotless failures

- How to verify it
Run the tests

closes #343

- Description for the changelog
enforce a single java format and style convention

@akafredperry akafredperry marked this pull request as draft January 28, 2026 17:49
String[] separatedByHyphen = errorCodeSegment.split("-");
errorCode = separatedByHyphen[0].trim();

errorText = rawErrorResponse.replaceFirst(errorCodeSegment + ":", "").trim();

Check failure

Code scanning / CodeQL

Regular expression injection High

This regular expression is constructed from a
user-provided value
.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

will be addressed by #345

IllegalBlockSizeException, BadPaddingException, NoSuchProviderException {
SecretKey key = _aesKeyFromBase64(keyBase64);
Cipher cipher = Cipher.getInstance("AES/SIC/PKCS7Padding", "BC");
cipher.init(Cipher.ENCRYPT_MODE, key, EMPTY_IV);

Check failure

Code scanning / CodeQL

Using a static initialization vector for encryption High

A
static initialization vector
should not be used for encryption.

Copilot Autofix

AI about 10 hours ago

In general, the problem is that encryption uses a fixed, static IV (EMPTY_IV). For secure use of block ciphers in modes like CBC, GCM, or CTR/SIC, each encryption operation under the same key must use a fresh, unpredictable IV/nonce. The standard pattern is: (1) generate a random IV with SecureRandom, (2) initialize the cipher with that IV, (3) include the IV alongside the ciphertext so decryption can reconstruct it (e.g., prefix the IV to the ciphertext or return/provide it separately).

The best focused fix here, without changing external functionality more than necessary, is:

  • Stop using EMPTY_IV for encryption.
  • Generate a random 16‑byte IV for each call to aesEncryptToBase64.
  • Initialize the cipher with that random IV.
  • Return the IV together with the ciphertext in a way that aesDecryptFromBase64 can consume without changing its signature. The simplest is to prefix the raw IV bytes to the ciphertext bytes and then base64-encode the combined array.
  • Update aesDecryptFromBase64(String cipherTextBase64, String keyBase64) so that, when ivNonce is not provided, it expects this “IV + ciphertext” format: it decodes from base64, splits the first 16 bytes as IV and the rest as ciphertext, and then decrypts using the extracted IV. This preserves backward compatibility of the method signature while making encryption secure.
  • Keep the overload aesDecryptFromBase64(String cipherTextBase64, String keyBase64, String ivNonce) for cases where the IV is transmitted separately, but discourage/default away from the EMPTY_IV path.

Concretely, within EncryptionUtil.java:

  1. Add a SecureRandom-based IV generation in aesEncryptToBase64, using a 16‑byte IV (AES block size): e.g., SecureRandom random = new SecureRandom(); byte[] ivBytes = new byte[16]; random.nextBytes(ivBytes); IvParameterSpec iv = new IvParameterSpec(ivBytes);.
  2. Change cipher.init(Cipher.ENCRYPT_MODE, key, EMPTY_IV); to use the newly generated IV.
  3. Change the encrypted output to prefix the IV bytes: allocate a new byte array of length ivBytes.length + encrypted.length, copy the IV first, ciphertext after, then base64-encode that combined array.
  4. In aesDecryptFromBase64(String cipherTextBase64, String keyBase64, String ivNonce), avoid defaulting to EMPTY_IV except where strictly necessary. For the 2‑argument overload (no explicit IV), implement the “decode, split IV + ciphertext, then decrypt” logic rather than delegating with ivNonce = null.
  5. No new external dependencies are required; java.security.SecureRandom is already imported via java.security.*;.
Suggested changeset 1
at_client/src/main/java/org/atsign/client/util/EncryptionUtil.java

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/at_client/src/main/java/org/atsign/client/util/EncryptionUtil.java b/at_client/src/main/java/org/atsign/client/util/EncryptionUtil.java
--- a/at_client/src/main/java/org/atsign/client/util/EncryptionUtil.java
+++ b/at_client/src/main/java/org/atsign/client/util/EncryptionUtil.java
@@ -25,9 +25,20 @@
       IllegalBlockSizeException, BadPaddingException, NoSuchProviderException {
     SecretKey key = _aesKeyFromBase64(keyBase64);
     Cipher cipher = Cipher.getInstance("AES/SIC/PKCS7Padding", "BC");
-    cipher.init(Cipher.ENCRYPT_MODE, key, EMPTY_IV);
+    // Generate a fresh random IV for each encryption
+    byte[] ivBytes = new byte[16];
+    SecureRandom secureRandom = new SecureRandom();
+    secureRandom.nextBytes(ivBytes);
+    IvParameterSpec ivSpec = new IvParameterSpec(ivBytes);
+    cipher.init(Cipher.ENCRYPT_MODE, key, ivSpec);
     byte[] encrypted = cipher.doFinal(clearText.getBytes());
-    return Base64.getEncoder().encodeToString(encrypted);
+
+    // Prefix IV to ciphertext so it can be recovered during decryption
+    byte[] ivAndCiphertext = new byte[ivBytes.length + encrypted.length];
+    System.arraycopy(ivBytes, 0, ivAndCiphertext, 0, ivBytes.length);
+    System.arraycopy(encrypted, 0, ivAndCiphertext, ivBytes.length, encrypted.length);
+
+    return Base64.getEncoder().encodeToString(ivAndCiphertext);
   }
 
   public static String aesDecryptFromBase64(String cipherTextBase64, String keyBase64, String ivNonce)
@@ -49,7 +59,23 @@
   public static String aesDecryptFromBase64(String cipherTextBase64, String keyBase64)
       throws NoSuchPaddingException, NoSuchAlgorithmException, InvalidAlgorithmParameterException, InvalidKeyException,
       IllegalBlockSizeException, BadPaddingException, NoSuchProviderException {
-    return aesDecryptFromBase64(cipherTextBase64, keyBase64, null);
+    SecretKey key = _aesKeyFromBase64(keyBase64);
+    Cipher cipher = Cipher.getInstance("AES/SIC/PKCS7Padding", "BC");
+
+    byte[] ivAndCiphertext = Base64.getDecoder().decode(cipherTextBase64);
+    if (ivAndCiphertext.length < 16) {
+      throw new InvalidAlgorithmParameterException("Ciphertext too short to contain IV");
+    }
+
+    byte[] ivBytes = new byte[16];
+    byte[] ciphertextBytes = new byte[ivAndCiphertext.length - 16];
+    System.arraycopy(ivAndCiphertext, 0, ivBytes, 0, 16);
+    System.arraycopy(ivAndCiphertext, 16, ciphertextBytes, 0, ciphertextBytes.length);
+
+    IvParameterSpec ivSpec = new IvParameterSpec(ivBytes);
+    cipher.init(Cipher.DECRYPT_MODE, key, ivSpec);
+    byte[] decrypted = cipher.doFinal(ciphertextBytes);
+    return new String(decrypted);
   }
 
   public static KeyPair generateRSAKeyPair() throws NoSuchAlgorithmException {
EOF
@@ -25,9 +25,20 @@
IllegalBlockSizeException, BadPaddingException, NoSuchProviderException {
SecretKey key = _aesKeyFromBase64(keyBase64);
Cipher cipher = Cipher.getInstance("AES/SIC/PKCS7Padding", "BC");
cipher.init(Cipher.ENCRYPT_MODE, key, EMPTY_IV);
// Generate a fresh random IV for each encryption
byte[] ivBytes = new byte[16];
SecureRandom secureRandom = new SecureRandom();
secureRandom.nextBytes(ivBytes);
IvParameterSpec ivSpec = new IvParameterSpec(ivBytes);
cipher.init(Cipher.ENCRYPT_MODE, key, ivSpec);
byte[] encrypted = cipher.doFinal(clearText.getBytes());
return Base64.getEncoder().encodeToString(encrypted);

// Prefix IV to ciphertext so it can be recovered during decryption
byte[] ivAndCiphertext = new byte[ivBytes.length + encrypted.length];
System.arraycopy(ivBytes, 0, ivAndCiphertext, 0, ivBytes.length);
System.arraycopy(encrypted, 0, ivAndCiphertext, ivBytes.length, encrypted.length);

return Base64.getEncoder().encodeToString(ivAndCiphertext);
}

public static String aesDecryptFromBase64(String cipherTextBase64, String keyBase64, String ivNonce)
@@ -49,7 +59,23 @@
public static String aesDecryptFromBase64(String cipherTextBase64, String keyBase64)
throws NoSuchPaddingException, NoSuchAlgorithmException, InvalidAlgorithmParameterException, InvalidKeyException,
IllegalBlockSizeException, BadPaddingException, NoSuchProviderException {
return aesDecryptFromBase64(cipherTextBase64, keyBase64, null);
SecretKey key = _aesKeyFromBase64(keyBase64);
Cipher cipher = Cipher.getInstance("AES/SIC/PKCS7Padding", "BC");

byte[] ivAndCiphertext = Base64.getDecoder().decode(cipherTextBase64);
if (ivAndCiphertext.length < 16) {
throw new InvalidAlgorithmParameterException("Ciphertext too short to contain IV");
}

byte[] ivBytes = new byte[16];
byte[] ciphertextBytes = new byte[ivAndCiphertext.length - 16];
System.arraycopy(ivAndCiphertext, 0, ivBytes, 0, 16);
System.arraycopy(ivAndCiphertext, 16, ciphertextBytes, 0, ciphertextBytes.length);

IvParameterSpec ivSpec = new IvParameterSpec(ivBytes);
cipher.init(Cipher.DECRYPT_MODE, key, ivSpec);
byte[] decrypted = cipher.doFinal(ciphertextBytes);
return new String(decrypted);
}

public static KeyPair generateRSAKeyPair() throws NoSuchAlgorithmException {
Copilot is powered by AI and may make mistakes. Always verify output.
} else {
iv = _ivFromBase64(ivNonce);
}
cipher.init(Cipher.DECRYPT_MODE, key, iv);

Check failure

Code scanning / CodeQL

Using a static initialization vector for encryption High

A
static initialization vector
should not be used for encryption.

Copilot Autofix

AI about 10 hours ago

In general, the problem should be fixed by never using a fixed, static IV for encryption. Instead, generate a fresh, unpredictable IV (or nonce) per encryption using SecureRandom, and communicate that IV alongside the ciphertext so that decryption can reconstruct the cipher state. For decryption, you must use the IV that was actually used during encryption; only the encrypt side needs randomness, but both sides must agree on the IV value.

For this specific code, the minimal change while preserving existing functionality shape is:

  1. Stop using EMPTY_IV for encryption. In aesEncryptToBase64, generate a random 16‑byte IV using SecureRandom, initialize the cipher with that IV, and then return both the IV and ciphertext to the caller. Since we must not change the method signature, the most backwards‑compatible approach within this snippet is to encode the IV together with the ciphertext in the returned Base64 string. A common pattern is to prepend the IV bytes to the ciphertext and Base64‑encode the concatenation.
  2. Update aesDecryptFromBase64 so that, when ivNonce is null (the current path that uses EMPTY_IV), it instead parses the returned Base64 string by splitting it into IV and ciphertext: decode Base64, take the first 16 bytes as the IV, and the remaining bytes as ciphertext. When ivNonce is non‑null, keep the current behavior of using _ivFromBase64(ivNonce) so that explicit IVs (if any) still work.
  3. Remove usage of EMPTY_IV for encryption and, ideally, for decryption. If you still need a default for legacy data that truly used the all‑zero IV, you could keep a special case, but the safer default is to expect IV to be present in the encoded value.

This fix requires:

  • Using SecureRandom from java.security (already imported via import java.security.*;).
  • Some byte array concatenation logic inside aesEncryptToBase64 and corresponding splitting logic inside aesDecryptFromBase64.
  • Keeping the existing method signatures unchanged so external callers still compile, while changing the internal encoding of the returned/consumed Base64 values.

Suggested changeset 1
at_client/src/main/java/org/atsign/client/util/EncryptionUtil.java

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/at_client/src/main/java/org/atsign/client/util/EncryptionUtil.java b/at_client/src/main/java/org/atsign/client/util/EncryptionUtil.java
--- a/at_client/src/main/java/org/atsign/client/util/EncryptionUtil.java
+++ b/at_client/src/main/java/org/atsign/client/util/EncryptionUtil.java
@@ -25,9 +25,22 @@
       IllegalBlockSizeException, BadPaddingException, NoSuchProviderException {
     SecretKey key = _aesKeyFromBase64(keyBase64);
     Cipher cipher = Cipher.getInstance("AES/SIC/PKCS7Padding", "BC");
-    cipher.init(Cipher.ENCRYPT_MODE, key, EMPTY_IV);
-    byte[] encrypted = cipher.doFinal(clearText.getBytes());
-    return Base64.getEncoder().encodeToString(encrypted);
+
+    // Generate a random IV for each encryption to avoid IV reuse
+    byte[] ivBytes = new byte[16];
+    SecureRandom secureRandom = new SecureRandom();
+    secureRandom.nextBytes(ivBytes);
+    IvParameterSpec ivSpec = new IvParameterSpec(ivBytes);
+
+    cipher.init(Cipher.ENCRYPT_MODE, key, ivSpec);
+    byte[] ciphertext = cipher.doFinal(clearText.getBytes(StandardCharsets.UTF_8));
+
+    // Prepend IV to ciphertext so the decrypt method can recover it
+    byte[] combined = new byte[ivBytes.length + ciphertext.length];
+    System.arraycopy(ivBytes, 0, combined, 0, ivBytes.length);
+    System.arraycopy(ciphertext, 0, combined, ivBytes.length, ciphertext.length);
+
+    return Base64.getEncoder().encodeToString(combined);
   }
 
   public static String aesDecryptFromBase64(String cipherTextBase64, String keyBase64, String ivNonce)
@@ -35,15 +48,31 @@
       IllegalBlockSizeException, BadPaddingException, NoSuchProviderException {
     SecretKey key = _aesKeyFromBase64(keyBase64);
     Cipher cipher = Cipher.getInstance("AES/SIC/PKCS7Padding", "BC");
+
+    byte[] cipherBytes = Base64.getDecoder().decode(cipherTextBase64);
     IvParameterSpec iv;
+    byte[] actualCiphertext;
+
     if (ivNonce == null) {
-      iv = EMPTY_IV;
+      // Expect IV to be prepended to the ciphertext
+      if (cipherBytes.length < 16) {
+        throw new IllegalBlockSizeException("Ciphertext too short to contain IV");
+      }
+      byte[] ivBytes = new byte[16];
+      System.arraycopy(cipherBytes, 0, ivBytes, 0, 16);
+      iv = new IvParameterSpec(ivBytes);
+
+      int ctLength = cipherBytes.length - 16;
+      actualCiphertext = new byte[ctLength];
+      System.arraycopy(cipherBytes, 16, actualCiphertext, 0, ctLength);
     } else {
       iv = _ivFromBase64(ivNonce);
+      actualCiphertext = cipherBytes;
     }
+
     cipher.init(Cipher.DECRYPT_MODE, key, iv);
-    byte[] decrypted = cipher.doFinal(Base64.getDecoder().decode(cipherTextBase64));
-    return new String(decrypted);
+    byte[] decrypted = cipher.doFinal(actualCiphertext);
+    return new String(decrypted, StandardCharsets.UTF_8);
   }
 
   public static String aesDecryptFromBase64(String cipherTextBase64, String keyBase64)
EOF
@@ -25,9 +25,22 @@
IllegalBlockSizeException, BadPaddingException, NoSuchProviderException {
SecretKey key = _aesKeyFromBase64(keyBase64);
Cipher cipher = Cipher.getInstance("AES/SIC/PKCS7Padding", "BC");
cipher.init(Cipher.ENCRYPT_MODE, key, EMPTY_IV);
byte[] encrypted = cipher.doFinal(clearText.getBytes());
return Base64.getEncoder().encodeToString(encrypted);

// Generate a random IV for each encryption to avoid IV reuse
byte[] ivBytes = new byte[16];
SecureRandom secureRandom = new SecureRandom();
secureRandom.nextBytes(ivBytes);
IvParameterSpec ivSpec = new IvParameterSpec(ivBytes);

cipher.init(Cipher.ENCRYPT_MODE, key, ivSpec);
byte[] ciphertext = cipher.doFinal(clearText.getBytes(StandardCharsets.UTF_8));

// Prepend IV to ciphertext so the decrypt method can recover it
byte[] combined = new byte[ivBytes.length + ciphertext.length];
System.arraycopy(ivBytes, 0, combined, 0, ivBytes.length);
System.arraycopy(ciphertext, 0, combined, ivBytes.length, ciphertext.length);

return Base64.getEncoder().encodeToString(combined);
}

public static String aesDecryptFromBase64(String cipherTextBase64, String keyBase64, String ivNonce)
@@ -35,15 +48,31 @@
IllegalBlockSizeException, BadPaddingException, NoSuchProviderException {
SecretKey key = _aesKeyFromBase64(keyBase64);
Cipher cipher = Cipher.getInstance("AES/SIC/PKCS7Padding", "BC");

byte[] cipherBytes = Base64.getDecoder().decode(cipherTextBase64);
IvParameterSpec iv;
byte[] actualCiphertext;

if (ivNonce == null) {
iv = EMPTY_IV;
// Expect IV to be prepended to the ciphertext
if (cipherBytes.length < 16) {
throw new IllegalBlockSizeException("Ciphertext too short to contain IV");
}
byte[] ivBytes = new byte[16];
System.arraycopy(cipherBytes, 0, ivBytes, 0, 16);
iv = new IvParameterSpec(ivBytes);

int ctLength = cipherBytes.length - 16;
actualCiphertext = new byte[ctLength];
System.arraycopy(cipherBytes, 16, actualCiphertext, 0, ctLength);
} else {
iv = _ivFromBase64(ivNonce);
actualCiphertext = cipherBytes;
}

cipher.init(Cipher.DECRYPT_MODE, key, iv);
byte[] decrypted = cipher.doFinal(Base64.getDecoder().decode(cipherTextBase64));
return new String(decrypted);
byte[] decrypted = cipher.doFinal(actualCiphertext);
return new String(decrypted, StandardCharsets.UTF_8);
}

public static String aesDecryptFromBase64(String cipherTextBase64, String keyBase64)
Copilot is powered by AI and may make mistakes. Always verify output.
…uild lifecycle

reformatted and adjusted to make code pass checkstyle and spotless rules
@akafredperry akafredperry self-assigned this Jan 28, 2026
@akafredperry akafredperry changed the title wip:added spotless and checkstyle configuration and added to maven b… feat:added spotless and checkstyle configuration and added to maven b… Jan 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Enforce code style and format as part of the build process

2 participants