Implementation of the HKDF derivation function #271

jrihtarsic · 2024-01-19T09:00:34Z

Purpose of the PR is to implement support for the HMAC-based Extract-and-Expand Key Derivation Function (HKDF [RFC5869]) used by the KeyAgreement method.
The details of the PR are in the ticket SANTUARIO-607

The code is contributed on behalf of the European Commission’s edelivery project to support eDelivery AS4 2.0 profile.

jrihtarsic · 2024-01-19T09:07:40Z

@coheigea please note that we did not yet receive final feedback from IETF regarding the
HKDF scheme. For now we are using the one defined bellow
src/main/resources/bindings/schemas/dsig-more_2021_04.xsd

I will let you know when this PR will be finalized for the review.

jrihtarsic · 2024-02-09T08:09:56Z

Hi @coheigea I want to inform you that the update for HKDF xsd scheme for RFC9231
https://datatracker.ietf.org/doc/draft-eastlake-rfc9231bis-xmlsec-uris/
was accepted and the PR can be reviewed now.

coheigea

@seanjmullan Can you have a look as well please?

src/main/java/org/apache/xml/security/encryption/XMLCipherUtil.java

src/main/java/org/apache/xml/security/encryption/keys/content/derivedKey/HKDFParamsImpl.java

src/test/java/org/apache/xml/security/encryption/keys/content/derivedKey/HKDFTest.java

coheigea · 2024-02-16T08:45:23Z

@jrihtarsic I'm going to call a vote on 4.0.2/3.0.4 to get the Key Agreement feature released and unblock the next CXF release. The HKDF will have to wait until the next release.

jrihtarsic · 2024-02-16T08:56:44Z

@coheigea understand and thanks for the info. I look forward to the first version of the key agreement option as part of the cxf.

seanjmullan · 2024-03-05T19:27:37Z

@seanjmullan Can you have a look as well please?

I can try to look at this over the next couple of weeks. I was off work for a while so I am still catching up with things.

…security-java into ec-edelivery-key-agreement-hkdf

coheigea · 2024-03-21T07:02:34Z

@seanjmullan Just a reminder please, as this PR blocks another Pr in WSS4J

coheigea · 2024-03-21T07:03:12Z

@jrihtarsic Can you remove the whitespace changes in this PR? It makes it difficult to get to the actual changes

jrihtarsic · 2024-03-21T08:02:47Z

I reverted the "cleaning of the code" (empty lines) from commit PR updates (docs and clean empty lines)

seanjmullan

I reviewed some of it, so far just a few minor comments. Will need another day or two to finish reviewing.

src/main/java/org/apache/xml/security/resource/xmlsecurity_en.properties

src/main/java/org/apache/xml/security/encryption/KeyDerivationMethod.java

src/main/java/org/apache/xml/security/encryption/params/HKDFParams.java

jrihtarsic · 2024-03-22T05:30:07Z

@seanjmullan Thank you for checking it. And no worries about time. I'd rather see my code thoroughly vetted by a security expert than to skip/miss some security issues or bugs. So take as much time as you need.

seanjmullan

Still reviewing, but here are some more comments.

src/main/java/org/apache/xml/security/encryption/KeyDerivationMethod.java

src/main/java/org/apache/xml/security/encryption/keys/content/derivedKey/KDFParams.java

...java/org/apache/xml/security/encryption/keys/content/derivedKey/KeyDerivationMethodImpl.java

src/main/java/org/apache/xml/security/utils/KeyUtils.java

src/main/java/org/apache/xml/security/encryption/params/HKDFParams.java

src/main/java/org/apache/xml/security/encryption/XMLCipherUtil.java

src/main/java/org/apache/xml/security/encryption/keys/content/derivedKey/HKDF.java

seanjmullan

Updates looks good.

src/main/java/org/apache/xml/security/encryption/XMLCipherUtil.java

…Utils.constructKeyDerivationParameter

src/main/java/org/apache/xml/security/encryption/XMLCipherUtil.java

coheigea · 2024-05-08T11:32:29Z

@seanjmullan Is it ready to be merged from your PoV?

coheigea · 2024-05-08T11:33:49Z

@jrihtarsic There is a build error:

[INFO] -------------------------------------------------------------
Error:  COMPILATION ERROR : 
[INFO] -------------------------------------------------------------
Error:  /home/runner/work/santuario-xml-security-java/santuario-xml-security-java/src/test/java/org/apache/xml/security/test/dom/encryption/XMLCipherTest.java:[581,58] ConcatKDFParams(int,java.lang.String) has protected access in org.apache.xml.security.encryption.params.ConcatKDFParams
Error:  /home/runner/work/santuario-xml-security-java/santuario-xml-security-java/src/test/java/org/apache/xml/security/test/dom/encryption/XMLEncryption11BrainpoolTest.java:[123,58] ConcatKDFParams(int,java.lang.String) has protected access in org.apache.xml.security.encryption.params.ConcatKDFParams

src/main/java/org/apache/xml/security/encryption/keys/content/derivedKey/HKDF.java

src/main/java/org/apache/xml/security/utils/KeyUtils.java

src/main/java/org/apache/xml/security/encryption/keys/content/derivedKey/HKDF.java

…security-java into ec-edelivery-key-agreement-hkdf

…ionKey

jrihtarsic · 2024-05-13T11:59:39Z

The branch is now updated with latest changes from the main, the build after the merge should pass now.

seanjmullan · 2024-05-15T13:01:22Z

@seanjmullan Is it ready to be merged from your PoV?

Yes, although I think we should try to add the secureValidation mode support before we post the next release.

jrihtarsic · 2024-05-15T13:10:33Z

Yes, although I think we should try to add the secureValidation mode support before we post the next release.
@seanjmullan I can make the PR for this by the end of the next week.
The scope is shortly described here: SANTUARIO-620, please let me know if I should implement anything else.

coheigea · 2024-05-16T12:23:28Z

@jrihtarsic Do you need this merged to 3.0.x as well?

jrihtarsic · 2024-05-16T12:29:34Z

@jrihtarsic Do you need this merged to 3.0.x as well?

@coheigea, yes indeed we would need it in 3.0.x so that we can use the latest feature with current apache/cxf

…JCEMapper.translateURItoJCEID

* HKDF derivation function * HKDF derivation function updates * PR updates (docs and clean empty lines) * Revert cleaning the code: empty lines from commit 491a7d3 * PR: Update comments * Fix PR comments * Fix PR comments * change XMLSecurityException with XMLEncryptionException for XMLCipherUtils.constructKeyDerivationParameter * fx the type in the documentation * Update branch with changes on main 'main' to fix build after merge * Rename the confusing overloading of methods KeyUtils.deriveKeyEncryptionKey * Remove XMLChipper.getJCEMacHashForUri URI/JCE name mapping and reuse JCEMapper.translateURItoJCEID --------- Co-authored-by: RIHTARSIC Joze <joze.rihtarsic@ext.ec.europa.eu>

HKDF derivation function

79abb13

jrihtarsic and others added 2 commits February 9, 2024 07:13

Merge branch 'apache:main' into ec-edelivery-key-agreement-hkdf

d1be5df

HKDF derivation function updates

47a097d

coheigea requested changes Feb 14, 2024

View reviewed changes

PR updates (docs and clean empty lines)

491a7d3

jrihtarsic requested a review from coheigea February 16, 2024 08:54

Merge branch 'main' of https://github.com/ec-edelivery/santuario-xml-…

9eb0592

…security-java into ec-edelivery-key-agreement-hkdf

jrihtarsic mentioned this pull request Mar 19, 2024

[WSS-710] Implementation of the configurations for KeyDerivation functions apache/ws-wss4j#298

Merged

Revert cleaning the code: empty lines from commit 491a7d3

1c22a43

seanjmullan reviewed Mar 21, 2024

View reviewed changes

PR: Update comments

a66df78

seanjmullan reviewed Apr 22, 2024

View reviewed changes

seanjmullan reviewed Apr 23, 2024

View reviewed changes

RIHTARSIC Joze added 3 commits April 27, 2024 19:58

Fix PR comments

e79f5f7

Fix PR comments

1211fc8

Resolve conflicts with changed on 'main'

fb135f9

jrihtarsic requested a review from seanjmullan May 2, 2024 06:07

seanjmullan approved these changes May 6, 2024

View reviewed changes

src/main/java/org/apache/xml/security/encryption/XMLCipherUtil.java Outdated Show resolved Hide resolved

change XMLSecurityException with XMLEncryptionException for XMLCipher…

cf3611b

…Utils.constructKeyDerivationParameter

seanjmullan reviewed May 7, 2024

View reviewed changes

src/main/java/org/apache/xml/security/encryption/XMLCipherUtil.java Outdated Show resolved Hide resolved

fx the type in the documentation

b5d726e

github-advanced-security bot found potential problems May 8, 2024

View reviewed changes

RIHTARSIC Joze added 3 commits May 13, 2024 13:03

Merge branch 'main' of https://github.com/ec-edelivery/santuario-xml-…

6b880ac

…security-java into ec-edelivery-key-agreement-hkdf

Update branch with changes on main 'main' to fix build after merge

3f6d9db

Rename the confusing overloading of methods KeyUtils.deriveKeyEncrypt…

1314fec

…ionKey

Remove XMLChipper.getJCEMacHashForUri URI/JCE name mapping and reuse …

4b8cc3e

…JCEMapper.translateURItoJCEID

coheigea approved these changes May 27, 2024

View reviewed changes

coheigea merged commit d8c9e86 into apache:main May 27, 2024
3 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implementation of the HKDF derivation function #271

Implementation of the HKDF derivation function #271

jrihtarsic commented Jan 19, 2024 •

edited

Loading

jrihtarsic commented Jan 19, 2024 •

edited

Loading

jrihtarsic commented Feb 9, 2024

coheigea left a comment

coheigea commented Feb 16, 2024

jrihtarsic commented Feb 16, 2024

seanjmullan commented Mar 5, 2024

coheigea commented Mar 21, 2024

coheigea commented Mar 21, 2024

jrihtarsic commented Mar 21, 2024

seanjmullan left a comment

jrihtarsic commented Mar 22, 2024

seanjmullan left a comment

seanjmullan left a comment

coheigea commented May 8, 2024

coheigea commented May 8, 2024

jrihtarsic commented May 13, 2024

seanjmullan commented May 15, 2024

jrihtarsic commented May 15, 2024 •

edited

Loading

coheigea commented May 16, 2024

jrihtarsic commented May 16, 2024

Implementation of the HKDF derivation function #271

Implementation of the HKDF derivation function #271

Conversation

jrihtarsic commented Jan 19, 2024 • edited Loading

jrihtarsic commented Jan 19, 2024 • edited Loading

jrihtarsic commented Feb 9, 2024

coheigea left a comment

Choose a reason for hiding this comment

coheigea commented Feb 16, 2024

jrihtarsic commented Feb 16, 2024

seanjmullan commented Mar 5, 2024

coheigea commented Mar 21, 2024

coheigea commented Mar 21, 2024

jrihtarsic commented Mar 21, 2024

seanjmullan left a comment

Choose a reason for hiding this comment

jrihtarsic commented Mar 22, 2024

seanjmullan left a comment

Choose a reason for hiding this comment

seanjmullan left a comment

Choose a reason for hiding this comment

coheigea commented May 8, 2024

coheigea commented May 8, 2024

jrihtarsic commented May 13, 2024

seanjmullan commented May 15, 2024

jrihtarsic commented May 15, 2024 • edited Loading

coheigea commented May 16, 2024

jrihtarsic commented May 16, 2024

jrihtarsic commented Jan 19, 2024 •

edited

Loading

jrihtarsic commented Jan 19, 2024 •

edited

Loading

jrihtarsic commented May 15, 2024 •

edited

Loading