Vytek / xades4j

Automatically exported from code.google.com/p/xades4j
GNU Lesser General Public License v3.0
0 stars 0 forks source link

Support for including intermediate certificates in signed documents #72

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1. Sign a document using a KeyingDataProvider that returns a certificate chain 
containing a signer certificate and one and more CA certificates.
2. Observe the result

What is the expected output? What do you see instead?
Expects to see multiple ds:X509Certificate tags one for each certificate in the 
chain. However in the result there is only ds:X509Certificate.

What version of the product are you using? On what operating system?
1.3.0, Linux

Please provide any additional information below.

In SignerBES.java:151:
List<X509Certificate> signingCertificateChain = 
this.keyingProvider.getSigningCertificateChain();
...
X509Certificate signingCertificate = signingCertificateChain.get(0);
...
this.keyInfoBuilder.buildKeyInfo(signingCertificate, signature);

Note that only the signing certificate is passed on and later in KeyInfoBuilder 
only that certificate is added.

Original issue reported on code.google.com by markuski...@gmail.com on 27 Aug 2013 at 10:33

GoogleCodeExporter commented 9 years ago
Downstream ticket in SignServer:
https://jira.primekey.se/browse/DSS-686

Original comment by markuski...@gmail.com on 27 Aug 2013 at 10:40

GoogleCodeExporter commented 9 years ago
This feature is already planned. The downside is that I might need to break the 
BasicSignatureOptionsProvider interface to add another flag.

In alternative the includeSigningCertificate flag could be used and, if true, 
include not only the signing certificate but the entire chain. This also isn't 
retro-compatible.

The BasicSignatureOptionsProvider will probably be broken in a future version 
anyway, so this should be included there.

Original comment by luis.fgoncalv on 28 Aug 2013 at 6:48

GoogleCodeExporter commented 9 years ago
Just looking at the API the KeyingProvider.getSigningCertificateChain() makes 
me think that if I return multiple certificates (ie a chain) those certificate 
that I return would be included. This would make it possible for the client 
application (KeyingDataProvider implementor) to decide if only the signing 
certificate should be used (by only returning that) or if all certificates 
should be used by returning those.

But as you say this isn't the behaviour today so a change could affect existing 
applications.

Original comment by markuski...@gmail.com on 28 Aug 2013 at 8:29

GoogleCodeExporter commented 9 years ago
I'd like to remove the includeSigningCertificate flag, since XAdES Baseline 
Profile (ETSI TS 103 171) mandates that the signing certificate is present on 
KeyInfo.

Assuming that we'll need some kind of disruption anyway, your proposal is a 
nice solution.

Original comment by luis.fgoncalv on 28 Aug 2013 at 8:50

GoogleCodeExporter commented 9 years ago
As an alternative one could probably go with the existing implementation and 
enrich the signature with the CertificateValues property to include the 
complete certificate chain.

Not sure about the practical difference between using ds:X509Data versus 
xades:CertificateValues and if it still would be conformant to BES if I include 
additional properties intended for more rich forms of signatures? 

Original comment by markuski...@gmail.com on 3 Feb 2014 at 3:47

GoogleCodeExporter commented 9 years ago
My understanding from XAdES annex B.1 and B.2 is that XAdES X-L - which uses 
xades:CertificateValues - builds on XAdES-X which in turn builds on XAdES-C. 
Probably a BES signature with just that long term property should not be deemed 
valid.

Regarding the location of certificates, XAdES section 7.6.1 states that 
xades:CertificateValues does not need to contain a certificate that is already 
on ds:KeyInfo. Also, annex G.2.2.2 states that the certificates for validation 
can be obtained from both those elements. They are considered equivalent.

Original comment by luis.fgoncalv on 4 Feb 2014 at 12:10

GoogleCodeExporter commented 9 years ago
In that case I would still think the feature with the possibility to control if 
the other certificates are included in the KeyInfo would be useful.

I will have a look how an implementation were the 
KeyingProvider.getSigningCertificateChain list would be used could look like.

Original comment by markuski...@gmail.com on 4 Feb 2014 at 8:42

GoogleCodeExporter commented 9 years ago
Attaching patch as discussed. However it has some disadvantages:
 - Changing which certificates are included in the list affects other parts as well. If one for instance removes the root CA certificate from the list to not have it in the KeyInfo, its name and digest will not be included in the xades:SigningCertificate wich one might still want
 - Not backwards compatible

As an alternative and more flexible solution the second patch adds an 
additional provider which controls which list of certificates to include in the 
KeyInfo structure.

This way the calling application will have full control of which certificates 
to include in the KeyInfo without affecting other parts that uses the full 
chain.

Original comment by markuski...@gmail.com on 4 Feb 2014 at 10:56

Attachments:

GoogleCodeExporter commented 9 years ago
What do you think of the type of solution in patch2? It makes it backwards 
compatible while adds full flexibility for the application to decide what 
certificates to include in the KeyInfo?

Original comment by markuski...@gmail.com on 5 Feb 2014 at 8:42

GoogleCodeExporter commented 9 years ago
Have you had time to have a look?

Cheers,
Markus

Original comment by markuski...@gmail.com on 20 Nov 2014 at 9:42