SONARJAVA-6939 Make S4426 key sizes configurable via rule property

[asya-vorobeva]

• Add minimumKeySizes @RuleProperty (format: "RSA:4096,AES:256") that patches the defaults — only listed algorithms are overridden, others keep their default values
• Add EC:224 to defaults; EC via KeyPairGenerator.initialize(int) is now also checked alongside the existing ECGenParameterSpec path
• Extract default key sizes, parsing, and EC curve pattern into CryptographicKeySizeConfiguration in sonar-analyzer-commons. Real version number will be bumped after analyzer-commons' release.

The related changes in analyzer-commons is visible here.

[hashicorp-vault-sonar-prod]

SONARJAVA-6939

[lijun-chen-sonarsource]

💯

[gitar-bot]

Code Review 👍 Approved with suggestions 2 closed / 3 findings

🟡 Medium risk · Configurable cryptographic key-size thresholds change security-analysis behavior across algorithms

Adds configurable key size thresholds for rule S4426 via a minimumKeySizes property with patch semantics, and extends EC curve detection to KeyPairGenerator.initialize(int). The property's defaultValue displays the complete default set in the SonarQube UI, but edits are treated as patches—removing entries like DSA:2048 has no effect, which may confuse operators expecting replacement semantics. Consider using an empty default value with documented built-in thresholds, or switch to replacement semantics where the displayed value is authoritative.

💡 Quality: defaultValue shows full list but semantics are patch-only

📄 java-checks/src/main/java/org/sonar/java/checks/security/CryptographicKeySizeCheck.java:49-54

The property advertises CryptographicKeySizeConfiguration.DEFAULT_KEY_SIZES as its defaultValue, so the SonarQube UI presents the complete default set as the parameter's value, while the semantics are patch-only: entries the user deletes from that string keep their default threshold. An operator who edits the shown value down to RSA:4096 (or deletes DSA:2048 to stop checking DSA) gets no change for the removed algorithms and no feedback, which contradicts the natural reading of an editable full list. Either make the property empty by default and document the built-in defaults in the description, or give it replacement semantics so that the displayed value is authoritative.

Leave the parameter empty by default so the displayed value only contains user overrides, and spell out the defaults plus the patch semantics in the description

@RuleProperty(
  key = "minimumKeySizes",
  description = "Comma-separated list of algorithm:minKeySize pairs (e.g. "RSA:4096,AES:256") overriding the built-in " +
    "minimums (" + CryptographicKeySizeConfiguration.DEFAULT_KEY_SIZES + "). Algorithms that are not listed keep their built-in minimum; " +
    "omitting an algorithm does not disable it.",
  defaultValue = "")
public String minimumKeySizes = "";

✅ 2 closed

✅ Bug: pom pins analyzer-commons to unreleased 2.33-SNAPSHOT

📄 pom.xml:97 📄 pom.xml:180-194
pom.xml:97 changes the shared analyzer.commons.version property to 2.33-SNAPSHOT, which drives six org.sonarsource.analyzer-commons artifacts (sonar-analyzer-commons, sonar-analyzer-recognizers, sonar-xml-parsing, sonar-analyzer-test-commons, sonar-regex-parsing, sonar-performance-measure) declared in pom.xml:180-208, and the pom declares no snapshot repository. Any build without a repox snapshot mirror in settings.xml fails to resolve all six artifacts, and merging a SNAPSHOT makes the 8.44 release non-reproducible; the version string also deviates from the artifact's own X.Y.Z.build scheme (previous value 2.32.0.5319), so even against a snapshot repo 2.33-SNAPSHOT may not be the published snapshot coordinate (2.33.0-SNAPSHOT). The PR description acknowledges this as temporary — it must be pinned to the released version before merge.

Closed: Edge Case: minimumKeySizes parsed per file with no error handling

📄 java-checks/src/main/java/org/sonar/java/checks/security/CryptographicKeySizeCheck.java:56-63
getEffectiveKeySizeMap() calls CryptographicKeySizeConfiguration.effectiveKeySizes(minimumKeySizes) lazily on the first getInstance/ECGenParameterSpec hit with no guard, and caches only on success. A misconfigured value such as RSA:abc or RSA=4096 therefore either throws out of the visitor on every analysed file (the field stays null, so the parse is retried per file and never degrades gracefully) or is silently dropped, leaving the user with default thresholds and no diagnostic. The repo convention for parsed string properties is to fail loudly at parse time — see CommentRegularExpressionCheck wrapping the parse and throwing IllegalRuleParameterException; no test covers a malformed value here either.

🤖 Prompt for agents

Code Review: Adds configurable key size thresholds for rule S4426 via a `minimumKeySizes` property with patch semantics, and extends EC curve detection to `KeyPairGenerator.initialize(int)`. The property's `defaultValue` displays the complete default set in the SonarQube UI, but edits are treated as patches—removing entries like `DSA:2048` has no effect, which may confuse operators expecting replacement semantics. Consider using an empty default value with documented built-in thresholds, or switch to replacement semantics where the displayed value is authoritative.

1. 💡 Quality: defaultValue shows full list but semantics are patch-only
   Files: java-checks/src/main/java/org/sonar/java/checks/security/CryptographicKeySizeCheck.java:49-54

   The property advertises `CryptographicKeySizeConfiguration.DEFAULT_KEY_SIZES` as its `defaultValue`, so the SonarQube UI presents the complete default set as the parameter's value, while the semantics are patch-only: entries the user deletes from that string keep their default threshold. An operator who edits the shown value down to `RSA:4096` (or deletes `DSA:2048` to stop checking DSA) gets no change for the removed algorithms and no feedback, which contradicts the natural reading of an editable full list. Either make the property empty by default and document the built-in defaults in the description, or give it replacement semantics so that the displayed value is authoritative.

   Fix (Leave the parameter empty by default so the displayed value only contains user overrides, and spell out the defaults plus the patch semantics in the description):
   @RuleProperty(
     key = "minimumKeySizes",
     description = "Comma-separated list of algorithm:minKeySize pairs (e.g. "RSA:4096,AES:256") overriding the built-in " +
       "minimums (" + CryptographicKeySizeConfiguration.DEFAULT_KEY_SIZES + "). Algorithms that are not listed keep their built-in minimum; " +
       "omitting an algorithm does not disable it.",
     defaultValue = "")
   public String minimumKeySizes = "";

Review coverage

🧪 Functional validation No results

📋 Rules No rules evaluated

🤖 Auto-approval Not enabled · Set up

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Counting what did not apply, without listing it.

Comment with these commands to change the behavior for this request:

Auto-apply	Compact
gitar auto-apply:on	gitar display:verbose

_{Was this helpful? React with 👍 / 👎 | Gitar}

[gitar-bot]

Checked CryptographicKeySizeConfiguration.parseKeySizes in analyzer-commons — you're right. Malformed pairs like RSA:abc or RSA=4096 are caught internally (NumberFormatException is swallowed, and entries that don't split into exactly 2 parts are just skipped), so nothing ever throws out of the visitor. The "throws per file" part of the finding doesn't hold. The remaining point — invalid entries are silently dropped with no diagnostic — is a deliberate design choice in analyzer-commons rather than a bug here, so no code change needed on this point.

[sonarqube-next]

Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
0 Dependency risks
94.1% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube