Skip to content

Commit

Permalink
Improve sign cove coverage by light refactoring of sign module
Browse files Browse the repository at this point in the history
  • Loading branch information
ars18wrw authored and Ubuntu committed Dec 27, 2021
1 parent 5d69064 commit db759b1
Show file tree
Hide file tree
Showing 14 changed files with 78 additions and 49 deletions.
10 changes: 3 additions & 7 deletions sign/src/main/java/com/itextpdf/signatures/CRLVerifier.java
Original file line number Diff line number Diff line change
Expand Up @@ -165,11 +165,7 @@ public X509CRL getCRL(X509Certificate signCert, X509Certificate issuerCert) {
}
LOGGER.info("Getting CRL from " + crlurl);
return (X509CRL) SignUtils.parseCrlFromStream(new URL(crlurl).openStream());
}
catch(IOException e) {
return null;
}
catch(GeneralSecurityException e) {
} catch (IOException | GeneralSecurityException e) {
return null;
}
}
Expand Down Expand Up @@ -201,12 +197,12 @@ public boolean isSignatureValid(X509CRL crl, X509Certificate crlIssuer) {
crl.verify(anchor.getPublicKey());
return true;
} catch (GeneralSecurityException e) {
continue;
// do nothing and continue
}
}
}
catch (GeneralSecurityException e) {
return false;
// do nothing and return false at the end
}
return false;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -156,13 +156,13 @@ public static String getOCSPURL(X509Certificate certificate) {
for (int i = 0; i < AccessDescriptions.size(); i++) {
ASN1Sequence AccessDescription = (ASN1Sequence) AccessDescriptions.getObjectAt(i);
if ( AccessDescription.size() != 2 ) {
continue;
// do nothing and continue
}
else if (AccessDescription.getObjectAt(0) instanceof ASN1ObjectIdentifier) {
ASN1ObjectIdentifier id = (ASN1ObjectIdentifier)AccessDescription.getObjectAt(0);
if (SecurityIDs.ID_OCSP.equals(id.getId())) {
ASN1Primitive description = (ASN1Primitive)AccessDescription.getObjectAt(1);
String AccessLocation = getStringFromGeneralName(description);
String AccessLocation = getStringFromGeneralName(description);
if (AccessLocation == null) {
return "" ;
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -151,12 +151,14 @@ public static List<VerificationException> verifyCertificates(Certificate[] certs
cert.verify(certStoreX509.getPublicKey());
return result;
} catch (Exception e) {
continue;
// do nothing and continue
}
} catch (Exception ex) {
// do nothing
}
}
} catch (Exception e) {
// do nothing
}
int j;
for (j = 0; j < certs.length; ++j) {
Expand All @@ -168,6 +170,7 @@ public static List<VerificationException> verifyCertificates(Certificate[] certs
cert.verify(certNext.getPublicKey());
break;
} catch (Exception e) {
// do nothing
}
}
if (j == certs.length) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,8 @@ public Collection<byte[]> getEncoded(X509Certificate checkCert, String url) {
ar.add(bout.toByteArray());
LOGGER.info("Added CRL found at: " + urlt);
} catch (Exception e) {
LOGGER.info(MessageFormatUtil.format(IoLogMessageConstant.INVALID_DISTRIBUTION_POINT, e.getMessage()));
LOGGER.info(MessageFormatUtil.format(IoLogMessageConstant.INVALID_DISTRIBUTION_POINT,
e.getMessage()));
}
}
return ar;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,8 @@ public static String normalizeDigestName(String algo) {
*/
public static String getAllowedDigest(String name) {
if (name == null) {
throw new IllegalArgumentException(SignExceptionMessageConstant.THE_NAME_OF_THE_DIGEST_ALGORITHM_IS_NULL);
throw new IllegalArgumentException(
SignExceptionMessageConstant.THE_NAME_OF_THE_DIGEST_ALGORITHM_IS_NULL);
}
return allowedDigests.get(name.toUpperCase());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ public static KeyStore loadCacertsKeyStore(String provider) {
fin.close();
}
} catch (Exception ex) {
// do nothing
}
}
}
Expand Down
3 changes: 1 addition & 2 deletions sign/src/main/java/com/itextpdf/signatures/LtvVerifier.java
Original file line number Diff line number Diff line change
Expand Up @@ -401,8 +401,7 @@ protected PdfPKCS7 coversWholeDocument() throws GeneralSecurityException {
if (pkcs7.verifySignatureIntegrityAndAuthenticity()) {
LOGGER.info("The signed document has not been modified.");
return pkcs7;
}
else {
} else {
throw new VerificationException((Certificate) null, "The document was altered after the final signature was applied.");
}
}
Expand Down
3 changes: 1 addition & 2 deletions sign/src/main/java/com/itextpdf/signatures/OCSPVerifier.java
Original file line number Diff line number Diff line change
Expand Up @@ -274,8 +274,7 @@ && isSignatureValid(ocspResp, cert)) {
throw new VerificationException(issuerCert, "Authorized OCSP responder certificate was revoked.");
}
} else {
Logger logger = LoggerFactory.getLogger(OCSPVerifier.class);
logger.error("Authorized OCSP responder certificate revocation status cannot be checked");
LOGGER.error("Authorized OCSP responder certificate revocation status cannot be checked");
// TODO DEVSIX-5207 throw exception starting from iText version 7.2, but only after OCSPVerifier
// would allow explicit setting revocation check end points/provide revocation data
}
Expand Down
13 changes: 8 additions & 5 deletions sign/src/main/java/com/itextpdf/signatures/PdfPKCS7.java
Original file line number Diff line number Diff line change
Expand Up @@ -866,8 +866,8 @@ public byte[] getEncodedPKCS7(byte[] secondDigest, PdfSigner.CryptoStandard sigt
//
v = new ASN1EncodableVector();
for (Object element : certs) {
ASN1InputStream tempstream
= new ASN1InputStream(new ByteArrayInputStream(((X509Certificate) element).getEncoded()));
ASN1InputStream tempstream = new ASN1InputStream(
new ByteArrayInputStream(((X509Certificate) element).getEncoded()));
v.add(tempstream.readObject());
}

Expand Down Expand Up @@ -1322,7 +1322,8 @@ void findCRL(ASN1Sequence seq) {
try {
crls = new ArrayList<>();
for (int k = 0; k < seq.size(); ++k) {
ByteArrayInputStream ar = new ByteArrayInputStream(seq.getObjectAt(k).toASN1Primitive().getEncoded(ASN1Encoding.DER));
ByteArrayInputStream ar = new ByteArrayInputStream(
seq.getObjectAt(k).toASN1Primitive().getEncoded(ASN1Encoding.DER));
X509CRL crl = (X509CRL) SignUtils.parseCrlFromStream(ar);
crls.add(crl);
}
Expand Down Expand Up @@ -1363,7 +1364,8 @@ public boolean isRevocationValid() {
CertificateID cid = sr.getCertID();
X509Certificate sigcer = getSigningCertificate();
X509Certificate isscer = cs[1];
CertificateID tis = SignUtils.generateCertificateId(isscer, sigcer.getSerialNumber(), cid.getHashAlgOID());
CertificateID tis = SignUtils.generateCertificateId(isscer, sigcer.getSerialNumber(),
cid.getHashAlgOID());
return tis.equals(cid);
} catch (Exception ignored) {
}
Expand Down Expand Up @@ -1398,8 +1400,9 @@ private void findOcsp(ASN1Sequence seq) throws IOException {
seq = (ASN1Sequence) tag.getObject();
ret = false;
break;
} else
} else {
return;
}
}
}
if (ret)
Expand Down
21 changes: 13 additions & 8 deletions sign/src/main/java/com/itextpdf/signatures/PdfSigner.java
Original file line number Diff line number Diff line change
Expand Up @@ -507,8 +507,9 @@ public void setFieldLockDict(PdfSigFieldLock fieldLock) {
* @throws IOException if some I/O problem occurs
* @throws GeneralSecurityException if some problem during apply security algorithms occurs
*/
public void signDetached(IExternalDigest externalDigest, IExternalSignature externalSignature, Certificate[] chain, Collection<ICrlClient> crlList, IOcspClient ocspClient,
ITSAClient tsaClient, int estimatedSize, CryptoStandard sigtype) throws IOException, GeneralSecurityException {
public void signDetached(IExternalDigest externalDigest, IExternalSignature externalSignature, Certificate[] chain,
Collection<ICrlClient> crlList, IOcspClient ocspClient, ITSAClient tsaClient, int estimatedSize,
CryptoStandard sigtype) throws IOException, GeneralSecurityException {
signDetached(externalDigest, externalSignature, chain, crlList, ocspClient, tsaClient, estimatedSize, sigtype,
(SignaturePolicyIdentifier) null);
}
Expand All @@ -531,8 +532,9 @@ public void signDetached(IExternalDigest externalDigest, IExternalSignature exte
* @throws IOException if some I/O problem occurs
* @throws GeneralSecurityException if some problem during apply security algorithms occurs
*/
public void signDetached(IExternalDigest externalDigest, IExternalSignature externalSignature, Certificate[] chain, Collection<ICrlClient> crlList, IOcspClient ocspClient,
ITSAClient tsaClient, int estimatedSize, CryptoStandard sigtype, SignaturePolicyInfo signaturePolicy) throws IOException, GeneralSecurityException {
public void signDetached(IExternalDigest externalDigest, IExternalSignature externalSignature, Certificate[] chain,
Collection<ICrlClient> crlList, IOcspClient ocspClient, ITSAClient tsaClient, int estimatedSize,
CryptoStandard sigtype, SignaturePolicyInfo signaturePolicy) throws IOException, GeneralSecurityException {
signDetached(externalDigest, externalSignature, chain, crlList, ocspClient, tsaClient, estimatedSize, sigtype,
signaturePolicy.toSignaturePolicyIdentifier());
}
Expand All @@ -555,8 +557,9 @@ public void signDetached(IExternalDigest externalDigest, IExternalSignature exte
* @throws IOException if some I/O problem occurs
* @throws GeneralSecurityException if some problem during apply security algorithms occurs
*/
public void signDetached(IExternalDigest externalDigest, IExternalSignature externalSignature, Certificate[] chain, Collection<ICrlClient> crlList, IOcspClient ocspClient,
ITSAClient tsaClient, int estimatedSize, CryptoStandard sigtype, SignaturePolicyIdentifier signaturePolicy) throws IOException, GeneralSecurityException {
public void signDetached(IExternalDigest externalDigest, IExternalSignature externalSignature, Certificate[] chain,
Collection<ICrlClient> crlList, IOcspClient ocspClient, ITSAClient tsaClient, int estimatedSize,
CryptoStandard sigtype, SignaturePolicyIdentifier signaturePolicy) throws IOException, GeneralSecurityException {
if (closed) {
throw new PdfException(SignExceptionMessageConstant.THIS_INSTANCE_OF_PDF_SIGNER_ALREADY_CLOSED);
}
Expand Down Expand Up @@ -628,8 +631,9 @@ public void signDetached(IExternalDigest externalDigest, IExternalSignature exte

byte[] encodedSig = sgn.getEncodedPKCS7(hash, sigtype, tsaClient, ocspList, crlBytes);

if (estimatedSize < encodedSig.length)
if (estimatedSize < encodedSig.length) {
throw new IOException("Not enough space");
}

byte[] paddedSig = new byte[estimatedSize];
System.arraycopy(encodedSig, 0, paddedSig, 0, encodedSig.length);
Expand Down Expand Up @@ -1071,7 +1075,8 @@ protected PdfSigFieldLock createNewSignatureFormField(PdfAcroForm acroForm, Stri
*/
protected InputStream getRangeStream() throws IOException {
RandomAccessSourceFactory fac = new RandomAccessSourceFactory();
return new RASInputStream(fac.createRanged(getUnderlyingSource(), range));
IRandomAccessSource randomAccessSource = fac.createRanged(getUnderlyingSource(), range);
return new RASInputStream(randomAccessSource);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ public List<VerificationOK> verify(X509Certificate signCert, X509Certificate iss
result.addAll(super.verify(signCert, issuerCert, signDate));
return result;
} catch (GeneralSecurityException e) {
continue;
// do nothing and continue
}
}
result.addAll(super.verify(signCert, issuerCert, signDate));
Expand Down
42 changes: 29 additions & 13 deletions sign/src/main/java/com/itextpdf/signatures/SignUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ This file is part of the iText (R) project.
import org.bouncycastle.cert.ocsp.OCSPException;
import org.bouncycastle.cert.ocsp.OCSPReq;
import org.bouncycastle.cert.ocsp.OCSPReqBuilder;
import org.bouncycastle.cms.SignerInformationVerifier;
import org.bouncycastle.cms.jcajce.JcaSimpleSignerInfoVerifierBuilder;
import org.bouncycastle.jce.X509Principal;
import org.bouncycastle.jce.provider.X509CertParser;
Expand Down Expand Up @@ -165,15 +166,20 @@ static InputStream getHttpResponse(URL urlt) throws IOException {
return (InputStream) con.getContent();
}

static CertificateID generateCertificateId(X509Certificate issuerCert, BigInteger serialNumber, AlgorithmIdentifier digestAlgorithmIdentifier) throws OperatorCreationException, CertificateEncodingException, OCSPException {
static CertificateID generateCertificateId(X509Certificate issuerCert, BigInteger serialNumber,
AlgorithmIdentifier digestAlgorithmIdentifier)
throws OperatorCreationException, CertificateEncodingException, OCSPException {
return new CertificateID(
new JcaDigestCalculatorProviderBuilder().build().get(digestAlgorithmIdentifier),
new JcaX509CertificateHolder(issuerCert), serialNumber);
}

static CertificateID generateCertificateId(X509Certificate issuerCert, BigInteger serialNumber, ASN1ObjectIdentifier identifier) throws OperatorCreationException, CertificateEncodingException, OCSPException {
static CertificateID generateCertificateId(X509Certificate issuerCert, BigInteger serialNumber,
ASN1ObjectIdentifier identifier)
throws OperatorCreationException, CertificateEncodingException, OCSPException {
return new CertificateID(
new JcaDigestCalculatorProviderBuilder().build().get(new AlgorithmIdentifier(identifier, DERNull.INSTANCE)),
new JcaDigestCalculatorProviderBuilder().build().get(
new AlgorithmIdentifier(identifier, DERNull.INSTANCE)),
new JcaX509CertificateHolder(issuerCert), serialNumber);
}

Expand Down Expand Up @@ -206,20 +212,27 @@ static InputStream getHttpResponseForOcspRequest(byte[] request, URL urlt) throw
return (InputStream) con.getContent();
}

static boolean isSignatureValid(BasicOCSPResp validator, Certificate certStoreX509, String provider) throws OperatorCreationException, OCSPException {
if (provider == null) provider = "BC";
static boolean isSignatureValid(BasicOCSPResp validator, Certificate certStoreX509, String provider)
throws OperatorCreationException, OCSPException {
if (provider == null) {
provider = "BC";
}
return validator.isSignatureValid(
new JcaContentVerifierProviderBuilder().setProvider(provider).build(certStoreX509.getPublicKey()));
}

static void isSignatureValid(TimeStampToken validator, X509Certificate certStoreX509, String provider) throws OperatorCreationException, TSPException {
static void isSignatureValid(TimeStampToken validator, X509Certificate certStoreX509, String provider)
throws OperatorCreationException, TSPException {
if (provider == null) {
provider = "BC";
}
validator.validate(new JcaSimpleSignerInfoVerifierBuilder().setProvider(provider).build(certStoreX509));
SignerInformationVerifier verifier = new JcaSimpleSignerInfoVerifierBuilder().setProvider(provider)
.build(certStoreX509);
validator.validate(verifier);
}

static boolean checkIfIssuersMatch(CertificateID certID, X509Certificate issuerCert) throws CertificateEncodingException, IOException, OCSPException {
static boolean checkIfIssuersMatch(CertificateID certID, X509Certificate issuerCert)
throws CertificateEncodingException, IOException, OCSPException {
return certID.matchesIssuer(
new X509CertificateHolder(issuerCert.getEncoded()), new BcDigestCalculatorProvider());
}
Expand All @@ -236,6 +249,7 @@ static Iterable<X509Certificate> getCertsFromOcspResponse(BasicOCSPResp ocspResp
try {
certs.add(converter.getCertificate(certHolder));
} catch (Exception ex) {
// do nothing
}
}
return certs;
Expand Down Expand Up @@ -264,7 +278,8 @@ static class TsaResponse {
InputStream tsaResponseStream;
}

static TsaResponse getTsaResponseForUserRequest(String tsaUrl, byte[] requestBytes, String tsaUsername, String tsaPassword) throws IOException {
static TsaResponse getTsaResponseForUserRequest(String tsaUrl, byte[] requestBytes, String tsaUsername,
String tsaPassword) throws IOException {
URL url = new URL(tsaUrl);
URLConnection tsaConnection;
try {
Expand Down Expand Up @@ -312,10 +327,9 @@ static TsaResponse getTsaResponseForUserRequest(String tsaUrl, byte[] requestByt
// TODO DEVSIX-2534
@Deprecated
static boolean hasUnsupportedCriticalExtension(X509Certificate cert) {
if ( cert == null ) {
if (cert == null) {
throw new IllegalArgumentException("X509Certificate can't be null.");
}

if (cert.hasUnsupportedCriticalExtension()) {
for (String oid : cert.getCriticalExtensionOIDs()) {
if (OID.X509Extensions.SUPPORTED_CRITICAL_EXTENSIONS.contains(oid)) {
Expand All @@ -336,7 +350,9 @@ static Calendar getTimeStampDate(TimeStampToken timeStampToken) {

static Signature getSignatureHelper(String algorithm, String provider)
throws NoSuchProviderException, NoSuchAlgorithmException {
return provider == null ? Signature.getInstance(algorithm) : Signature.getInstance(algorithm, provider);
return provider == null
? Signature.getInstance(algorithm)
: Signature.getInstance(algorithm, provider);
}

static boolean verifyCertificateSignature(X509Certificate certificate, PublicKey issuerPublicKey, String provider) {
Expand Down Expand Up @@ -392,7 +408,7 @@ private void tryToGetNextCertificate() {
break;
}
} catch (KeyStoreException e) {
continue;
// do nothing and continue
}
}
}
Expand Down
Loading

0 comments on commit db759b1

Please sign in to comment.