You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@pdfbox.apache.org by Tobias Brignol Petry <to...@gmail.com> on 2019/01/31 13:27:58 UTC
Enhancement to CertInformationCollector
Hello,
The current version of
org.apache.pdfbox.examples.signature.validation.CertInformationCollector
supports a single certificate as the alternative issuer.
It could support chains also:
---------------------------
Current code (lines 300-308):
try (InputStream in = certUrl.openStream())
{
X509Certificate altIssuerCert = (X509Certificate) certFactory
.generateCertificate(in);
addCertToCertificatesMap(altIssuerCert);
certInfo.alternativeCertChain = new CertSignatureInformation();
traverseChain(altIssuerCert, certInfo.alternativeCertChain, maxDepth -
1);
}
------------------------------
Proposed update:
try (InputStream in = certUrl.openStream())
{
Collection<? extends java.security.cert.Certificate> altIssuerCerts =
certFactory.generateCertificates(in);
for (java.security.cert.Certificate c : altIssuerCerts) {
X509Certificate altIssuerCert = (X509Certificate) c;
addCertToCertificatesMap(altIssuerCert);
certInfo.alternativeCertChain = new CertSignatureInformation();
traverseChain(altIssuerCert, certInfo.alternativeCertChain,
maxDepth - 1);
}
}
--------------------------
Re: Enhancement to CertInformationCollector
Posted by to...@gmail.com,
to...@gmail.com.
If I understand correctly, local variable "altIssuerCert" (scoped within the for loop) will be passed on to addCertToCertificatesMap. If it happens to be the "wrong" one, the consequence will be that the certificates map may end up containing one (or more) unuseful entry(ies).
On 2019/02/02 18:50:13, Tilman Hausherr <TH...@t-online.de> wrote:
> "altIssuerCert" is set each time in the loop. How do we make sure that
> it's the right one, i.e. the one at the beginning of the chain?
>
> Tilman
>
> Am 01.02.2019 um 13:14 schrieb tobiaspetry@gmail.com:
> > In fact, the correct chain might not be found in the right order, so, there could be a try/catch block within the for loop:
> >
> > try (InputStream in = certUrl.openStream())
> > {
> > Collection<? extends java.security.cert.Certificate> altIssuerCerts =
> > certFactory.generateCertificates(in);
> > for (java.security.cert.Certificate c : altIssuerCerts) {
> > X509Certificate altIssuerCert = (X509Certificate) c;
> > addCertToCertificatesMap(altIssuerCert);
> >
> > certInfo.alternativeCertChain = new CertSignatureInformation();
> > try {
> > traverseChain(altIssuerCert, certInfo.alternativeCertChain,
> > maxDepth - 1);
> > } catch (IOException e) {
> > LOG.error("Error getting additional Certificate from " + certInfo.issuerUrl, e);
> > }
> > }
> > }
> >
> > On 2019/01/31 16:43:20, Tilman Hausherr <TH...@t-online.de> wrote:
> >> Yes this sounds good, thanks. I'll add it soon.
> >>
> >> Tilman
> >>
> >> Am 31.01.2019 um 14:27 schrieb Tobias Brignol Petry:
> >>> Hello,
> >>>
> >>> The current version of
> >>> org.apache.pdfbox.examples.signature.validation.CertInformationCollector
> >>> supports a single certificate as the alternative issuer.
> >>> It could support chains also:
> >>>
> >>> ---------------------------
> >>> Current code (lines 300-308):
> >>> try (InputStream in = certUrl.openStream())
> >>> {
> >>> X509Certificate altIssuerCert = (X509Certificate) certFactory
> >>> .generateCertificate(in);
> >>> addCertToCertificatesMap(altIssuerCert);
> >>>
> >>> certInfo.alternativeCertChain = new CertSignatureInformation();
> >>> traverseChain(altIssuerCert, certInfo.alternativeCertChain, maxDepth -
> >>> 1);
> >>> }
> >>> ------------------------------
> >>> Proposed update:
> >>> try (InputStream in = certUrl.openStream())
> >>> {
> >>> Collection<? extends java.security.cert.Certificate> altIssuerCerts =
> >>> certFactory.generateCertificates(in);
> >>> for (java.security.cert.Certificate c : altIssuerCerts) {
> >>> X509Certificate altIssuerCert = (X509Certificate) c;
> >>> addCertToCertificatesMap(altIssuerCert);
> >>>
> >>> certInfo.alternativeCertChain = new CertSignatureInformation();
> >>> traverseChain(altIssuerCert, certInfo.alternativeCertChain,
> >>> maxDepth - 1);
> >>> }
> >>> }
> >>> --------------------------
> >>>
> >>
> >> ---------------------------------------------------------------------
> >> To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org
> >> For additional commands, e-mail: dev-help@pdfbox.apache.org
> >>
> >>
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org
> > For additional commands, e-mail: dev-help@pdfbox.apache.org
> >
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org
> For additional commands, e-mail: dev-help@pdfbox.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org
For additional commands, e-mail: dev-help@pdfbox.apache.org
Re: Enhancement to CertInformationCollector
Posted by Tilman Hausherr <TH...@t-online.de>.
Am 05.02.2019 um 14:50 schrieb tobiaspetry@gmail.com:
> This leads to a second point. The key to the certificates map (certificatesMap) is currently a BigInteger corresponding to the serial number of the certificate. This will work in most scenarios, however, nothing prevents certificates from different issuers to have equal serial numbers - issuers from Brazil often assign low integers to the serial numbers, meaning a high chance of serial number clash with certificates from other issuers.
> Therefore, changing the certificates map key to a String corresponding to the canonical Subject Name, followed by a separator character (e.g. colon ":"), followed by the Serial Number toString would be a handy improvement as well.
I have prepared something locally re: the key. I'm wondering why this is
a map at all, a set could be used as well. Or is there a risk that
locally identical certificates would not be equal? According to the
javadoc the certificate is encoded and then compared. So I'm wondering
why the original developer used a map and not a set. If it was about
speed, that would probably be gone partly with the recent change.
Tilman
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org
For additional commands, e-mail: dev-help@pdfbox.apache.org
Re: Enhancement to CertInformationCollector
Posted by to...@gmail.com,
to...@gmail.com.
Furthermore, should the alternate issuer itself be issued by another "alternate" issuer, the entries inserted at the certificates map on previous iterations (on which traverseChain might have thrown an exception by not having found the correct issuer yet) might be required later, in order to match the whole chain.
This leads to a second point. The key to the certificates map (certificatesMap) is currently a BigInteger corresponding to the serial number of the certificate. This will work in most scenarios, however, nothing prevents certificates from different issuers to have equal serial numbers - issuers from Brazil often assign low integers to the serial numbers, meaning a high chance of serial number clash with certificates from other issuers.
Therefore, changing the certificates map key to a String corresponding to the canonical Subject Name, followed by a separator character (e.g. colon ":"), followed by the Serial Number toString would be a handy improvement as well.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org
For additional commands, e-mail: dev-help@pdfbox.apache.org
Re: Enhancement to CertInformationCollector
Posted by Tilman Hausherr <TH...@t-online.de>.
"altIssuerCert" is set each time in the loop. How do we make sure that
it's the right one, i.e. the one at the beginning of the chain?
Tilman
Am 01.02.2019 um 13:14 schrieb tobiaspetry@gmail.com:
> In fact, the correct chain might not be found in the right order, so, there could be a try/catch block within the for loop:
>
> try (InputStream in = certUrl.openStream())
> {
> Collection<? extends java.security.cert.Certificate> altIssuerCerts =
> certFactory.generateCertificates(in);
> for (java.security.cert.Certificate c : altIssuerCerts) {
> X509Certificate altIssuerCert = (X509Certificate) c;
> addCertToCertificatesMap(altIssuerCert);
>
> certInfo.alternativeCertChain = new CertSignatureInformation();
> try {
> traverseChain(altIssuerCert, certInfo.alternativeCertChain,
> maxDepth - 1);
> } catch (IOException e) {
> LOG.error("Error getting additional Certificate from " + certInfo.issuerUrl, e);
> }
> }
> }
>
> On 2019/01/31 16:43:20, Tilman Hausherr <TH...@t-online.de> wrote:
>> Yes this sounds good, thanks. I'll add it soon.
>>
>> Tilman
>>
>> Am 31.01.2019 um 14:27 schrieb Tobias Brignol Petry:
>>> Hello,
>>>
>>> The current version of
>>> org.apache.pdfbox.examples.signature.validation.CertInformationCollector
>>> supports a single certificate as the alternative issuer.
>>> It could support chains also:
>>>
>>> ---------------------------
>>> Current code (lines 300-308):
>>> try (InputStream in = certUrl.openStream())
>>> {
>>> X509Certificate altIssuerCert = (X509Certificate) certFactory
>>> .generateCertificate(in);
>>> addCertToCertificatesMap(altIssuerCert);
>>>
>>> certInfo.alternativeCertChain = new CertSignatureInformation();
>>> traverseChain(altIssuerCert, certInfo.alternativeCertChain, maxDepth -
>>> 1);
>>> }
>>> ------------------------------
>>> Proposed update:
>>> try (InputStream in = certUrl.openStream())
>>> {
>>> Collection<? extends java.security.cert.Certificate> altIssuerCerts =
>>> certFactory.generateCertificates(in);
>>> for (java.security.cert.Certificate c : altIssuerCerts) {
>>> X509Certificate altIssuerCert = (X509Certificate) c;
>>> addCertToCertificatesMap(altIssuerCert);
>>>
>>> certInfo.alternativeCertChain = new CertSignatureInformation();
>>> traverseChain(altIssuerCert, certInfo.alternativeCertChain,
>>> maxDepth - 1);
>>> }
>>> }
>>> --------------------------
>>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org
>> For additional commands, e-mail: dev-help@pdfbox.apache.org
>>
>>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org
> For additional commands, e-mail: dev-help@pdfbox.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org
For additional commands, e-mail: dev-help@pdfbox.apache.org
Re: Enhancement to CertInformationCollector
Posted by to...@gmail.com,
to...@gmail.com.
In fact, the correct chain might not be found in the right order, so, there could be a try/catch block within the for loop:
try (InputStream in = certUrl.openStream())
{
Collection<? extends java.security.cert.Certificate> altIssuerCerts =
certFactory.generateCertificates(in);
for (java.security.cert.Certificate c : altIssuerCerts) {
X509Certificate altIssuerCert = (X509Certificate) c;
addCertToCertificatesMap(altIssuerCert);
certInfo.alternativeCertChain = new CertSignatureInformation();
try {
traverseChain(altIssuerCert, certInfo.alternativeCertChain,
maxDepth - 1);
} catch (IOException e) {
LOG.error("Error getting additional Certificate from " + certInfo.issuerUrl, e);
}
}
}
On 2019/01/31 16:43:20, Tilman Hausherr <TH...@t-online.de> wrote:
> Yes this sounds good, thanks. I'll add it soon.
>
> Tilman
>
> Am 31.01.2019 um 14:27 schrieb Tobias Brignol Petry:
> > Hello,
> >
> > The current version of
> > org.apache.pdfbox.examples.signature.validation.CertInformationCollector
> > supports a single certificate as the alternative issuer.
> > It could support chains also:
> >
> > ---------------------------
> > Current code (lines 300-308):
> > try (InputStream in = certUrl.openStream())
> > {
> > X509Certificate altIssuerCert = (X509Certificate) certFactory
> > .generateCertificate(in);
> > addCertToCertificatesMap(altIssuerCert);
> >
> > certInfo.alternativeCertChain = new CertSignatureInformation();
> > traverseChain(altIssuerCert, certInfo.alternativeCertChain, maxDepth -
> > 1);
> > }
> > ------------------------------
> > Proposed update:
> > try (InputStream in = certUrl.openStream())
> > {
> > Collection<? extends java.security.cert.Certificate> altIssuerCerts =
> > certFactory.generateCertificates(in);
> > for (java.security.cert.Certificate c : altIssuerCerts) {
> > X509Certificate altIssuerCert = (X509Certificate) c;
> > addCertToCertificatesMap(altIssuerCert);
> >
> > certInfo.alternativeCertChain = new CertSignatureInformation();
> > traverseChain(altIssuerCert, certInfo.alternativeCertChain,
> > maxDepth - 1);
> > }
> > }
> > --------------------------
> >
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org
> For additional commands, e-mail: dev-help@pdfbox.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org
For additional commands, e-mail: dev-help@pdfbox.apache.org
Re: Enhancement to CertInformationCollector
Posted by Tilman Hausherr <TH...@t-online.de>.
Yes this sounds good, thanks. I'll add it soon.
Tilman
Am 31.01.2019 um 14:27 schrieb Tobias Brignol Petry:
> Hello,
>
> The current version of
> org.apache.pdfbox.examples.signature.validation.CertInformationCollector
> supports a single certificate as the alternative issuer.
> It could support chains also:
>
> ---------------------------
> Current code (lines 300-308):
> try (InputStream in = certUrl.openStream())
> {
> X509Certificate altIssuerCert = (X509Certificate) certFactory
> .generateCertificate(in);
> addCertToCertificatesMap(altIssuerCert);
>
> certInfo.alternativeCertChain = new CertSignatureInformation();
> traverseChain(altIssuerCert, certInfo.alternativeCertChain, maxDepth -
> 1);
> }
> ------------------------------
> Proposed update:
> try (InputStream in = certUrl.openStream())
> {
> Collection<? extends java.security.cert.Certificate> altIssuerCerts =
> certFactory.generateCertificates(in);
> for (java.security.cert.Certificate c : altIssuerCerts) {
> X509Certificate altIssuerCert = (X509Certificate) c;
> addCertToCertificatesMap(altIssuerCert);
>
> certInfo.alternativeCertChain = new CertSignatureInformation();
> traverseChain(altIssuerCert, certInfo.alternativeCertChain,
> maxDepth - 1);
> }
> }
> --------------------------
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pdfbox.apache.org
For additional commands, e-mail: dev-help@pdfbox.apache.org