SubjectCertificateNotRevokedValidator is not thread safe

[heikih]

We experience stacktrace:

eu.webeid.security.validator.certvalidators.SubjectCertificateNotRevokedValidator.validateCertificateNotRevoked(SubjectCertificateNotRevokedValidator.java:93) ~[authtoken-validation-3.0.1.jar!/:?]

Caused by: java.lang.ArrayIndexOutOfBoundsException: Index 4 out of bounds for length 4","at 
org.bouncycastle.crypto.digests.GeneralDigest.update(Unknown Source) ~[bcprov-jdk18on-1.78.jar!/:?]","at 
org.bouncycastle.crypto.digests.GeneralDigest.finish(Unknown Source) ~[bcprov-jdk18on-1.78.jar!/:?]","at 
org.bouncycastle.crypto.digests.SHA1Digest.doFinal(Unknown Source) ~[bcprov-jdk18on-1.78.jar!/:?]","at 
org.bouncycastle.crypto.io.DigestOutputStream.getDigest(Unknown Source) ~[bcprov-jdk18on-1.78.jar!/:?]","at 
eu.webeid.security.validator.ocsp.Digester.getDigest(Digester.java:98) ~[authtoken-validation-3.0.1.jar!/:?]","at 
org.bouncycastle.cert.ocsp.CertificateID.createCertID(Unknown Source) ~[bcpkix-jdk18on-1.78.jar!/:?]","at 
org.bouncycastle.cert.ocsp.CertificateID.<init>(Unknown Source) ~[bcpkix-jdk18on-1.78.jar!/:?]","at 
eu.webeid.security.validator.certvalidators.SubjectCertificateNotRevokedValidator.getCertificateId(SubjectCertificateNotRevokedValidator.java:192) ~[authtoken-validation-3.0.1.jar!/:?]","at 
eu.webeid.security.validator.certvalidators.SubjectCertificateNotRevokedValidator.validateCertificateNotRevoked(SubjectCertificateNotRevokedValidator.java:93) ~[authtoken-validation-3.0.1.jar!/:?]

After first exception all new requests will fail with similar java.lang.ArrayIndexOutOfBoundsException.

Happens when two threads are trying to call getCertificateId method exactly same time.

Underlying reason seems to be usage of single DigestCalculator class:

private static final DigestCalculator DIGEST_CALCULATOR = Digester.sha1();

that ends up with single non threadsafe SHA1Digest class for all threads. If 2 threads are running on same time then internal buffer can be corrupted there.

[mrts]

Thanks for reporting this issue and apologies for the trouble!

Here's a fix where each thread gets its own DigestCalculator instance: #69

Please let us know if this helps and looks good to you.

[heikih]

Fix looks good to me. Do you plan to publish a new version with this fix soon?

[mrts]

Yes, either this or next week.

[mrts]

Can you please confirm if the fix works for you now?

[heikih]

I think it will work, cant 100% confirm as we dont have test for it and have experienced it only twice in production with heavy load.