Upgrade CXF to latest version to get rid of CVE-2022-46364 and #197
Conversation
CVE-2022-46363 Also upgrade WSS4J to 2.4.1 which is supposed to work with CXF >= 3.4.*.
|
@aaron-kumar : Can you have a look at this? |
| } | ||
|
|
||
| OxalisAlgorithmSuite(final SPConstants.SPVersion version, final Policy nestedPolicy) { | ||
| super(version, nestedPolicy); | ||
| this.setAsymmetricSignature("http://www.w3.org/2001/04/xmldsig-more#rsa-sha256"); |
There was a problem hiding this comment.
I do not understand this change - does it relate to the topic of PR?
There was a problem hiding this comment.
The API of WSS4J have changed. The method setAsymmetricSignature appears to have been moved from the top level class AlgorithmSuite to the nested class AlgorithmSuiteType.
There was a problem hiding this comment.
OK, good! But could it be that AsymmetricSignature is not set to "http://www.w3.org/2001/04/xmldsig-more#rsa-sha256" for
BASIC_128_GCM_SHA_256
but only for
BASIC_128_GCM_SHA_256_MGF_SHA_256?
In previous version, it was defined on a OxalisAlgorithmSuite level, in a new version - on AlgorithmSuiteType, but in
it is set only for BASIC_128_GCM_SHA_256_MGF_SHA_256, right?May be we miss similar line for BASIC_128_GCM_SHA_256?
If we make mistake here, it will be quite difficult to find it out...
There was a problem hiding this comment.
Yes. The asymmetric signature is now only set for the second custom AlgorithmSuiteType, BASIC_128_GCM_SHA_256_MGF_SHA_256. It might well be that it is needed for the first one, too.
I put it on the second suite type, since it already had a couple of customizations, guessing that the first one might not need customisation. But I don't know these WSS4J APIs, I just did the bare minimum to get it to compile against newer CXF and WSS4J. And it appears to be working in our test environment, but that might be a false positive, if some default asymmetric signature with weaker security is used instead...
Do you think its safer to assign the asymmetric signature on both?
There was a problem hiding this comment.
From the WSS4J code, the algorithm suite gets a default asymmetric signature, when we don't specify it. So it's probably safest to set it on both suites. I have added that in the last commit.
There was a problem hiding this comment.
Thank you! Yes, this looks safer - better to keep the same logic as much as possible during upgrades.
I think it is ok now - although I would write it like before, without extracting to a variable algorithmSuiteType for only one of suites:
ALGORITHM_SUITE_TYPES.get(BASIC_128_GCM_SHA_256_MGF_SHA_256).setMGFAlgo(MGF_SHA256);
ALGORITHM_SUITE_TYPES.get(BASIC_128_GCM_SHA_256_MGF_SHA_256).setEncryptionDigest(SPConstants.SHA256);
ALGORITHM_SUITE_TYPES.get(BASIC_128_GCM_SHA_256_MGF_SHA_256)
.setAsymmetricSignature("http://www.w3.org/2001/04/xmldsig-more#rsa-sha256");
ALGORITHM_SUITE_TYPES.get(BASIC_128_GCM_SHA_256)
.setAsymmetricSignature("http://www.w3.org/2001/04/xmldsig-more#rsa-sha256");
In this case, the commit diff would clearly highlight, that you just added .setAsymmetricSignature("http://www.w3.org/2001/04/xmldsig-more#rsa-sha256"); to both suite types because it is removed from Suite.
But it is just a question of styling :)
Anyway, let's see @aaron-kumar review too :)
There was a problem hiding this comment.
Ok, seems like @aaron-kumar did a superset of these changes in the latest release, and also added code to adjust the signature algorithm.
(was: ommitted). If we don't set it explicitly, a default asymmetric signature gets used, so we probably should do it.
CVE-2022-46363
Also upgrade WSS4J to 2.4.1 which is supposed to work with CXF >= 3.4.*.
Pull Request Description
Fixes the above two vulnerabilities being reported against Oxalis-AS4 5.4.0 by snyk and Trivy.
I have barely tested a snapshot of it with Oxalis 5.4.0 in a test environment at Digipost (both inbound and outbound processes), but haven't found any isses so far.
Not 100% sure yet if the upgrade breaks anything else.
Type of Pull Request
Type of Change
Pull Request Checklist:
mvn clean installbefore commit and all tests run successfullymasterbranch