bc-java icon indicating copy to clipboard operation
bc-java copied to clipboard

CMSSignedData.replaceSigners() does not handle DigestAlgorithms parameters properly

Open bsanchezb opened this issue 1 year ago • 3 comments

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 avatar Feb 21 '24 11:02 bsanchezb

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?

dghgit avatar Feb 22 '24 02:02 dghgit

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.

bsanchezb avatar Feb 22 '24 07:02 bsanchezb