Java: Add MaDs for `java.crypto.KDF` #20345

IdrissRio · 2025-09-02T10:58:37Z

Updates Java MaDs forjava.crypto.KDF.

Java Docs - KDF: https://docs.oracle.com/en/java/javase/24/docs/api/java.base/javax/crypto/KDF.html
Java Docs - HKDFParameterSpec: https://docs.oracle.com/en/java/javase/24/docs/api/java.base/javax/crypto/spec/HKDFParameterSpec.html

Java release note: https://jdk.java.net/25/release-notes#510

github-actions · 2025-09-02T11:01:51Z

⚠️ The head of this PR and the base branch were compared for differences in the framework coverage reports. The generated reports are available in the artifacts of this workflow run. The differences will be picked up by the nightly job after the PR gets merged.

Click to show differences in coverage

java

Generated file changes for java

Changes to framework-coverage-java.rst:

-    Java extensions,"``javax.*``, ``jakarta.*``",87,4159,90,10,4,2,1,1,4
+    Java extensions,"``javax.*``, ``jakarta.*``",87,4189,90,10,4,2,1,1,4
-    Totals,,330,26328,2656,404,16,128,33,1,409
+    Totals,,330,26358,2656,404,16,128,33,1,409

Changes to framework-coverage-java.csv:

- javax.crypto,19,,114,,,12,3,,2,2,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,61,53
+ javax.crypto,19,,144,,,12,3,,2,2,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,83,61

Copilot

Pull Request Overview

This PR adds Model as Data (MaD) support for the java.crypto.KDF API, enabling taint tracking through Key Derivation Function operations. The changes include comprehensive taint flow models for KDF operations and HKDF parameter specifications.

Adds taint flow models for javax.crypto.KDF class methods
Adds taint flow models for javax.crypto.spec.HKDFParameterSpec and its builder pattern
Includes comprehensive test coverage for various KDF usage patterns

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
`java/ql/lib/ext/javax.crypto.model.yml`	Adds summary models for KDF class methods including `deriveKey` and `deriveData`
`java/ql/lib/ext/javax.crypto.spec.model.yml`	Adds taint flow models for HKDF parameter specification builder methods and constructors
`java/ql/test/library-tests/dataflow/kdf/test.ql`	Test query for verifying taint flow through KDF operations
`java/ql/test/library-tests/dataflow/kdf/KDFDataflowTest.java`	Comprehensive test cases covering various KDF usage patterns
`java/ql/test/library-tests/dataflow/kdf/options`	Compilation options enabling Java 25 preview features
`java/ql/lib/change-notes/2025-09-02-kdf-api.md`	Release notes documenting the new KDF taint flow support

Copilot · 2025-09-02T11:41:03Z

java/ql/lib/ext/javax.crypto.spec.model.yml

+      - ["javax.crypto.spec", "HKDFParameterSpec$Builder", True, "addSalt", "(SecretKey)", "", "Argument[this]", "ReturnValue", "taint", "manual"]
+      - ["javax.crypto.spec", "HKDFParameterSpec$Builder", True, "thenExpand", "(byte[],int)", "", "Argument[this]", "ReturnValue", "taint", "manual"]
+      - ["javax.crypto.spec", "HKDFParameterSpec$Builder", True, "thenExpand", "(byte[],int)", "", "Argument[0]", "ReturnValue", "taint", "manual"]
+      - ["javax.crypto.spec", "HKDFParameterSpec", False, "ofExtract", "()", "", "", "ReturnValue", "taint", "manual"]


The ofExtract() method creates a clean builder instance and should not propagate taint by itself. Taint should only flow when actual key material is added via addIKM() methods. This model could lead to false positives where clean HKDF operations are flagged as tainted.

Suggested change

- ["javax.crypto.spec", "HKDFParameterSpec", False, "ofExtract", "()", "", "", "ReturnValue", "taint", "manual"]

aschackmull · 2025-09-04T08:29:18Z

java/ql/lib/ext/javax.crypto.model.yml

+      - ["javax.crypto", "KDF", False, "getInstance", "(String,KDFParameters,Provider)", "", "Argument[0]", "ReturnValue.SyntheticField[javax.crypto.KDF.algorithm]", "value", "manual"]
+      - ["javax.crypto", "KDF", False, "getInstance", "(String,KDFParameters,String)", "", "Argument[0]", "ReturnValue.SyntheticField[javax.crypto.KDF.algorithm]", "value", "manual"]
+      - ["javax.crypto", "KDF", True, "getAlgorithm", "()", "", "Argument[this].SyntheticField[javax.crypto.KDF.algorithm]", "ReturnValue", "value", "manual"]
+      - ["javax.crypto", "KDF", True, "getProvider", "()", "", "Argument[this]", "ReturnValue", "value", "manual"]


This line looks suspicious. It suggests that getProvider() is implemented as return this. Should this have been a taint model rather than a value-preserving step?

aschackmull · 2025-09-04T08:37:25Z

java/ql/lib/ext/javax.crypto.spec.model.yml

+      - ["javax.crypto.spec", "HKDFParameterSpec$Builder", True, "addIKM", "(byte[])", "", "Argument[0]", "Argument[this]", "taint", "manual"]
+      - ["javax.crypto.spec", "HKDFParameterSpec$Builder", True, "addIKM", "(byte[])", "", "Argument[0]", "ReturnValue", "taint", "manual"]
+      - ["javax.crypto.spec", "HKDFParameterSpec$Builder", True, "addIKM", "(byte[])", "", "Argument[this]", "ReturnValue", "taint", "manual"]


This is a fluent method so the flow from this to return should be value. Once that's fixed, the two lines above will contain redundant information, as MaD has some support for fluent apis: You only need to provide the taint model that taints the argument - then the model that taints the output will be automatically added.

Suggested change

- ["javax.crypto.spec", "HKDFParameterSpec$Builder", True, "addIKM", "(byte[])", "", "Argument[0]", "Argument[this]", "taint", "manual"]

- ["javax.crypto.spec", "HKDFParameterSpec$Builder", True, "addIKM", "(byte[])", "", "Argument[0]", "ReturnValue", "taint", "manual"]

- ["javax.crypto.spec", "HKDFParameterSpec$Builder", True, "addIKM", "(byte[])", "", "Argument[this]", "ReturnValue", "taint", "manual"]

- ["javax.crypto.spec", "HKDFParameterSpec$Builder", True, "addIKM", "(byte[])", "", "Argument[0]", "Argument[this]", "taint", "manual"]

- ["javax.crypto.spec", "HKDFParameterSpec$Builder", True, "addIKM", "(byte[])", "", "Argument[this]", "ReturnValue", "value", "manual"]

The same applies to the other fluent api models below.

Thanks! I'll fix this 👍

aschackmull · 2025-09-04T08:38:43Z

java/ql/test-kotlin1/library-tests/java-kotlin-collection-type-generic-methods/test.expected

@@ -16,30 +16,6 @@ methodWithDuplicate
 | AbstractCollection<E> | removeAll | Collection<?> |
 | AbstractCollection<E> | retainAll | Collection<?> |
 | AbstractCollection<E> | toArray | T[] |
-| AbstractCollection<Entry<K,V>> | add | Entry<K,V> |


This diff doesn't belong on this PR?

No, none of the Kotlin changes are part of this PR. I updated the base to idrissrio/java-upgrade-fix, so that commit now appears as well. Rebasing should remove it from the UI. You can safely ignore anything related to Kotlin.

aschackmull · 2025-09-04T09:06:08Z

java/ql/test/library-tests/dataflow/kdf/KDFDataflowTest.java

+
+        KDF kdf = KDF.getInstance("HKDF-SHA256");
+        byte[] result = kdf.deriveData(spec);
+        sink(result); // should flag


please use inline expectation

Java: accept new test results after extractor update

a34b362

github-actions bot added Java Kotlin labels Sep 2, 2025

IdrissRio changed the base branch from main to idrissrio/java-upgrade-fix September 2, 2025 11:00

github-actions bot added documentation and removed Kotlin labels Sep 2, 2025

Java: add dataflow test for newly added KDF API

f762cf1

IdrissRio force-pushed the idrissrio/java-kdf branch from e284e6e to fa73d5e Compare September 2, 2025 11:37

IdrissRio marked this pull request as ready for review September 2, 2025 11:40

IdrissRio requested a review from a team as a code owner September 2, 2025 11:40

Copilot AI review requested due to automatic review settings September 2, 2025 11:40

Copilot AI reviewed Sep 2, 2025

View reviewed changes

IdrissRio added 3 commits September 2, 2025 13:50

Java: Add MaDs for java.crypto.KDF

dba12f3

Java: accept new test results

ae786de

Java: Add new change note

ccfbf55

IdrissRio force-pushed the idrissrio/java-kdf branch from fa73d5e to ccfbf55 Compare September 2, 2025 11:50

IdrissRio force-pushed the idrissrio/java-upgrade-fix branch from a34b362 to 5d2268f Compare September 2, 2025 18:19

IdrissRio requested a review from a team as a code owner September 2, 2025 18:19

aschackmull reviewed Sep 4, 2025

View reviewed changes

Base automatically changed from idrissrio/java-upgrade-fix to main September 4, 2025 14:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Java: Add MaDs for `java.crypto.KDF` #20345

Java: Add MaDs for `java.crypto.KDF` #20345

Uh oh!

IdrissRio commented Sep 2, 2025 •

edited

Loading

Uh oh!

github-actions bot commented Sep 2, 2025

java

Generated file changes for java

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Sep 2, 2025

Uh oh!

aschackmull Sep 4, 2025

Uh oh!

aschackmull Sep 4, 2025

Uh oh!

aschackmull Sep 4, 2025

Uh oh!

IdrissRio Sep 4, 2025

Uh oh!

aschackmull Sep 4, 2025

Uh oh!

IdrissRio Sep 4, 2025

Uh oh!

aschackmull Sep 4, 2025

Uh oh!

Uh oh!

Java: Add MaDs for java.crypto.KDF #20345

Are you sure you want to change the base?

Java: Add MaDs for java.crypto.KDF #20345

Uh oh!

Conversation

IdrissRio commented Sep 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Sep 2, 2025

java

Generated file changes for java

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Sep 2, 2025

Choose a reason for hiding this comment

Uh oh!

aschackmull Sep 4, 2025

Choose a reason for hiding this comment

Uh oh!

aschackmull Sep 4, 2025

Choose a reason for hiding this comment

Uh oh!

aschackmull Sep 4, 2025

Choose a reason for hiding this comment

Uh oh!

IdrissRio Sep 4, 2025

Choose a reason for hiding this comment

Uh oh!

aschackmull Sep 4, 2025

Choose a reason for hiding this comment

Uh oh!

IdrissRio Sep 4, 2025

Choose a reason for hiding this comment

Uh oh!

aschackmull Sep 4, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Java: Add MaDs for `java.crypto.KDF` #20345

Java: Add MaDs for `java.crypto.KDF` #20345

IdrissRio commented Sep 2, 2025 •

edited

Loading