luisgoncalves / xades4j

A Java library for XAdES signature services
GNU Lesser General Public License v3.0
111 stars 66 forks source link

Invalid Signature produced by Xades4j #269

Closed Ulterior closed 1 year ago

Ulterior commented 1 year ago

I am using version 1.6.0 and lately have found some signatures to be faulty produced by xades4j. References for signedproperties and whole document are OK, but the SignatureValue decoded contains different digest of SignedInfo. I am using PKCS11 hardware keys for signing and maybe the Provider is messing up hash for signature? I am very afraid now that I cannot depend on the output of xades4j.

I have checked everything and now have to add verification after each signing request to try catching this error, but this will take a lot of time and resources. I can provide you with xml and public key for verification if you are interested understanding the problem?

luisgoncalves commented 1 year ago

References for signedproperties and whole document are OK

You mean that the digest in the XML matches the one calculated during verification, right?

the SignatureValue decoded contains different digest of SignedInfo

What do you mean?

Are you also verifying signatures using xades4j, or using some other system? Core XML-DSIG production/verification is delegated to Apache Santurario, so it seems unlikely that there are issues there..

Side-note: could also be good to update xades4j to latest version, to also get updates to Apache Santuario.

Ulterior commented 1 year ago

I am using javax.xml.crypto.dsig.XMLSignature for veryfication. When its bad It also fails in our c++ validator - thats how we caught it.

When validating SignatureValue is decoded with public key and contains SignedInfo digest. This is also referred as a core when being validated. References in my case - an information signed in SignedInfo - signedproperties and whole doc uri="" validates correctly.

Here is the link with xades4j produced xml and a certificate for verification https://drive.google.com/file/d/1b_EPr9yuqCCp2cpZ0hbRsw4Zh2TvhIUz/view?usp=share_link

luisgoncalves commented 1 year ago

SignatureValue is not decoded with any public key. It is compared against the result of applying the signature algorithm to the canonicalized SignedInfo currently on the signature.

Anyway, do you get errors in every signature, or onyl on some? If only some, is there a difference in the producing settings?

It should be possible to get some logs from Apache Santuario, so that you get some details of the verification process: https://github.com/apache/santuario-xml-security-java/blob/main/src/main/java/org/apache/xml/security/signature/XMLSignature.java#L204

Ulterior commented 1 year ago

Its a rare occurrence. I can manually verify signature is correct with this code:

Cipher rsa = Cipher.getInstance("RSA/ECB/PKCS1Padding");
rsa.init(Cipher.DECRYPT_MODE, cert.getPublicKey());
byte[] decoded = rsa.doFinal(signature.getSignatureValue().getValue());

Normally decoded contains prepended structure with, in this case, sha256 algo id and Canocialized SignedInfo digest. You cannot decode this without correct public key. In the provided example it decodes, but the digest is not the same as if you would get it by canonicalizing SignedInfo. I guess I ll have to follow you advice and update library version - is it updated regarding santuario compared to version 1.6.0?

luisgoncalves commented 1 year ago

I'm not completely following what are you trying to do with the snippet above. Are you sure that's equivalent to e.g. SHA256withRSA? If anything, to check stuff manually shouldn't you use Signature.getInstance("SHA256withRSA")?

I don't see how xades4j would output an incorrect SignatureValue or incorrectly calculate the canonicalized SignedInfo only in some cases, if the configuration is the same. Could there be something on the producing/verifying environment that causes this? Encoding issues, ...

Ulterior commented 1 year ago

updated to 1.7.0. added forced verification after each signing - will update you on my findings when I catch it, Thank Luis

luisgoncalves commented 1 year ago

The latest version of Apache Santuario is added on xades4j 2.1.0. If you don't want to upgrade to 2.X, you probabyl can add Santuario directly to your project, not sure.

Anyway, don't forget the idea to check Santuario logs. It may log the digests at the different steps.

Ulterior commented 1 year ago

My latest findings revelead a HW operation failure with CKR_FUNCTION_FAILED exception and right after that it produces wrong Signature. I suspect HW cleanup was not done correctly and old hash was left to produce SignatureValue

luisgoncalves commented 1 year ago

Something at the PKCS11 device, then? It doesn't come up as an error on the Java side?

Let me know if there's anything that xades4j could/should be doing to make that error visible.

luisgoncalves commented 1 year ago

Closing as there doesn't seem to be any issue with xades4j and this ticket has been inactive.