Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CMSSignedData.replaceSigners() does not handle DigestAlgorithms parameters properly #1585

Open
bsanchezb opened this issue Feb 21, 2024 · 2 comments
Assignees

Comments

@bsanchezb
Copy link

bsanchezb commented Feb 21, 2024

Hello,

I'm currently working on implementation of CAdES-E-ERS (ETSI TS 119 122-3) for my project, which requires computation of CMS digest to be protected by an embedded evidence-record or its validation.

As an evidence-record is placed within UnsignedAttributes, I need to remove the attribute in order to compute the original hash of the covered CMS, which involves re-computation of the CMS. What I'm currently doing is I remove the evidence-record attribute from UnsignedAttributes of a corresponding SignerInformation, re-compute collection of all present SignerInformations in a form of SignerInformationStore, and call CMSSignedData.replaceSigners(originalCMS, newSignerInformationStore), so it looks like:

CMSSignedData originalSignedData = ...
List<SignerInformation> newSignerInformationList = new ArrayList<>();
for (SignerInformation signerInformation : cmsSignedData.getSignerInfos().getSigners()) {
    AttributeTable unsignedAttributes = signerInformation.getUnsignedAttributes();
    if (unsignedAttributesContainER(unsignedAttributes)) {
        unsignedAttributes = unsignedAttributes.remove(erAttributeIdentifier);
        signerInformation = SignerInformation.replaceUnsignedAttributes(signerInformation, unsignedAttributes);
    }
    newSignerInformationList.add(signerInformation);
}
CMSSignedData newSignedData = CMSSignedData.replaceSigners(originalSignedData , new SignerInformationStore(newSignerInformationList));

The method CMSSignedData.replaceSigners(cmsSignedData, signerInformationStore) seems to cause a problem on digestAlgs set computation, ignoring algorithm parameters. I.e. when processing a CMS having DigestAlgorithms defined like:
image
the call of CMSSignedData.replaceSigners method produces a signature with the parameters attribute being removed:
image

This of course modifies the binary content of the signature, which impacts the hash computation, making it impossible to verify the embedded evidence-record.

The issue can be originally caused by CMSUtils.addDigestAlgs() method, which ignores (?) the embedded parameters.

Worth noticing, that the SignerInformation in question has a DigestAlgorithm attribute defined in the same way, before and after its replacement within the CMS:
image

Thus, I find the SignedData.digestAlgorithms field to be built incorrectly.

Kind regards,
Aleksandr

@bsanchezb bsanchezb changed the title CMSSignedData.replaceSigners() does not preserve original DigestAlgortihms structure CMSSignedData.replaceSigners() does not handle DigestAlgorithms parameters properly Feb 21, 2024
@dghgit
Copy link
Contributor

dghgit commented Feb 22, 2024

Strictly speaking the parameters should be absent. I agree this is not a really feature though. I'll have to look into this - the DigestAlgorithms is supposed to represent a set of all the digest algorithms in use, the re-encoding is happening as the set is being rebuilt.

I know this will sound a bit crazy, but I'd assume if you call CMSSignedData.replaceSigners() twice, the first time with the original signer information set, things actually work out?

@bsanchezb
Copy link
Author

Strictly speaking the parameters should be absent. I agree this is not a really feature though. I'll have to look into this - the DigestAlgorithms is supposed to represent a set of all the digest algorithms in use, the re-encoding is happening as the set is being rebuilt.

Indeed, I understand that there is no interest in having NULL parameters, but as we do not have control over signatures provided to our app, we should be able to handle all of them. In fact, all solutions implementing the embedded ERs that we currently found, have this problem. Therefore, it is important to support these cases.

I know this will sound a bit crazy, but I'd assume if you call CMSSignedData.replaceSigners() twice, the first time with the original signer information set, things actually work out?

Maybe I did not understand the suggestion, but I was still not able to produce a new CMS with NULL parameters within DigestAlgorithms field. I tried to call CMSSignedData.replaceSigners() with the original (not modified) set of SignerInformations and simply execute it twice in order.

@dghgit dghgit self-assigned this Feb 22, 2024
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

No branches or pull requests

2 participants