DER NULL validation fail after 1.69

[javier-aliaga-form3]

The latest release (1.69) of the library contains the following changes:

AlgorithmIdentifiers involving message digests now attempt to follow the latest conventions for the parameters field (basically DER NULL appears less)

After upgrading the library to this version, there is a third party system unable to validate the signature. We think this is due to the change mentioned above. After reading the rfc5754 we believe some implementations could still require this field to be present.

We could benefit from this behaviour being parameterised somehow. What is your opinion on this? Would it be worth it to investigate how feasible is to update library to be compatible with both behaviors?

[slaupster]

This affects me too. A unit test we have that

• makes a Signature using SHA256WithRSA
• hashes with SHA256 then encrypts with RSA/NONE/PKCS1Padding

and compares the bytes passes at 1.68 and fails at 1.69

[slaupster]

tests.zip

Here is a recreate of that unit test. Passes with 1.68 and fails with 1.69

[harri35]

Hei!

I have the same issue. We use BC both on the mobile client and backend side for a system that creates digital signatures.
If I update the mobile client to use BC 1.69 the components fail to work in a compatible way.
Also, a bunch of our unit tests fail on the client-side. More specifically, it seems there is a clear difference in the algorithm ID (SHA256).

• Our tests expect and pre-1.69 BC versions give 0x30 0x31 0x30 0x0d 0x06 0x09 0x60 0x86 0x48 0x01 0x65 0x03 0x04 0x02 0x01 0x05 0x00 0x04 0x20
• With the 1.69 our tests fails as the ID part is 0xFF 0x00 0x30 0x2F 0x30 0x0B 0x06 0x09 0x60 0x86 0x48 0x01 0x65 0x03 0x04 0x02 0x01 0x04 0x20

As we have a lot of mobile clients out in the field and our client libraries are in a code-freeze then for us it seems that we will not be able to update past 1.68 for some while now.

[harri35]

Hei!

I was wondering - did you break the backwards compatibility here on purpose? Eg code that uses this library version to generate messages will not be able to communicate with the same code using an older library. Would it be possible it make this change in a way that would not break backwards compatibility?

[dghgit]

So the value for the parameters field in the AlgorithmIdentifier depends on the DigestAlgorithmIdentifierFinder in use in the CMSSignedDataGenerator/CMSSignedDataStreamGenerator in use. This is dependent on what the digestAlgIdFinder field is set to. It's actually possible to override this already in an extension class - I've pushed a new beta with some additional constructors to make this clearer.

Try what's now in https://www.bouncycastle.org/betas and pass an appropriate algorithm finder (I'd suggest just copying the 1.68 for now) and you should find it reproduces the old behavior.

[harri35]

Hei @dghgit !

Thanks for your fast answer! I think the solution you proposed will work fine for any of our new development. Thanks!

I was wondering if there would be some solution also for our library version that is in a code freeze. We can't change this version as it has been certified by TÜViT as a QSCD.
For us, it would work the best, if the integrator could affect the default DigestAlgorithmIdentifierFinder during integration without changing the binary of our library. Or should we just tell the people who integrate it to not upgrade the BC library above 1.68? We would prefer not to do the latter as it could lock us out of any potential future security updates.

[dghgit]

@harri35 from what you've said it sounds like your libeary is in code freeze, but not the dependencies, is that right? The bcpkix API does not need to be signed to work, only the bcprov jar needs to be (although we do sign it for other reasons). For now you could probably get away with patching the bcpkix API with the old DefaultDigestAlgorithmIdentifier finder if you need to do an update.

[harri35]

Hei @dghgit !

Thanks for the suggestion, this would be one way to go for sure.

library is in code freeze, but not the dependencies, is that right?

Well, yes, this is a little bit of a grey area for us. We actually do both for different components:

• For the backend side we have kept everything frozen. And in general, will only update libraries if there are security patches needed. Thus the version of BC in the backend is significantly older atm.
• For the Android library our clients integrate we can't freeze the library versions easily as it would create a problem for our integrators who will use the same libraries for their other components also. And do not want to use old ones. Also, there is an expectation from Google that you keep your whole project and the resulting binary up-to-date, or you may have some penalties there. So we do update and test the library binary with updated dependencies regularly. Luckily we do not have many dependencies.

[harri35]

@dghgit But if I may ask the reasoning behind the ID change - would it be safer to make the DigestAlgorithmIdentifierFinder to default to older values? So the backwards compatibility would remain by default? And make using the newer ones an explicit choice? At least for some transitional period? Or is there a reason for this immediate change to the new IDs?

[dghgit]

The reason behind the ID change is that most of the standards have now moved on - and for some time, in a sense we've already had a transitional period. Sadly, it doesn't matter what we do, I'll still wind up explaining it to someone on github. On the up side we're now following the latest usage, which should mean this issue will progressively vanish.

[harri35]

@dghgit Understood, thanks!