1308026 - JSS certificate validation does not pass up exact error from NSS

Status: RESOLVED FIXED

(JSS Graveyard :: Library, defect)

Comment 1

[Elio Maldonado]

Attachment: pass up exact jss certificate validation errors from nss V1

Downstream patch by Endi Sukma Dewata not yet ready for review.

Comment 2

[Elio Maldonado]

Attachment: pass up exact jss certificate validation errors from nss - V2

Regenerated the patch from Endi S. Dewata" <[e-mail]> for current experimental sources, 
================= Test Results
JSSTEST_SUITE: 31 / 31
JSSTEST_RATE: 100 %
Test Status: SUCCESS
Not ready for review until all subsequent patches get testested.

Comment 3

[Elio Maldonado]

Attachment: pass up exact jss certificate validation errors from nss - V3

Patch was updated for the current state of the sources.

Comment 4

[Elio Maldonado]

Comment on attachment 8844661 [details] [diff] [review]
pass up exact jss certificate validation errors from nss - V3

Review of attachment 8844661 [details] [diff] [review]:
-----------------------------------------------------------------

::: org/mozilla/jss/CryptoManager.c
@@ +321,5 @@
> +        jstring ocspResponderCertNickname,
> +        jboolean initializeJavaOnly,
> +        jboolean PKIXVerify,
> +        jboolean noCertDB,
> +        jboolean noModDB, 

nitpick: should remove trailing white space

::: org/mozilla/jss/CryptoManager.java
@@ +1575,5 @@
>       *      with the given nickname.
>       */
>      public boolean isCertValid(String nickname, boolean checkSig,
>              CertificateUsage certificateUsage)
> +        throws ObjectNotFoundException, InvalidNicknameException, CertificateException

Notice that the function now also throws CertificateException

::: org/mozilla/jss/PK11Finder.c
@@ +1566,5 @@
> +        self, nickString, checkSig, required_certificateUsage);
> +    return JNI_TRUE;
> +    /* NOTE: No way of knowing the result the function we are calling
> +     * as it was the result type type was changed from boolean to void.
> +     */

Please notice this question.

@@ +1593,5 @@
> +    SECCertificateUsage      currUsage = 0x0000;  /* unexposed for now */
> +    SECStatus                rv = SECFailure;
> +    CERTCertificate          *cert = NULL;
> +    char                     *nickname = NULL;
> +    

nitpick: should get rid of white space

Comment 5

[Elio Maldonado]

(In reply to Elio Maldonado from comment #4)
> Comment on attachment 8844661 [details] [diff] [review]
> pass up exact jss certificate validation errors from nss
> 
> Review of attachment 8844661 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: org/mozilla/jss/CryptoManager.c
> @@ +321,5 @@
> > +        jstring ocspResponderCertNickname,
> > +        jboolean initializeJavaOnly,
> > +        jboolean PKIXVerify,
> > +        jboolean noCertDB,
> > +        jboolean noModDB, 
> 
> nitpick: should remove trailing white space

Besides I meant to bring to your attention that
initializeAllNative2 now has more parameters that were added to it by upstream folks.

Comment 6

[Elio Maldonado]

Attachment: pass up exact jss certificate validation errors from nss - V4

This version has changes requested by Endi in a private review.

Comment 7

[Elio Maldonado]

Attachment: pass up exact jss certificate validation errors from nss - V4

This is the one I meant to attach.

Comment 8

[Endi S. Dewata]

Comment on attachment 8845094 [details] [diff] [review]
pass up exact jss certificate validation errors from nss - V4

Just one minor issue, in PK11Finder.c the parameter "required_rertificateUsage" should be renamed to "required_certificateUsage" for consistency.

Please use this commit description:

A new CryptoManager.verifyCertificate() method has been added as
an alternative to isCertValid(). If the specified certificate is
invalid, the new method will throw a CertificateException containing
the actual NSS error code and a description instead of just a boolean
value returned by the old method. The exception itself will also
help locate the error for troubleshooting.

Comment 9

[Elio Maldonado]

Pushed: https://hg.mozilla.org/projects/jss/rev/79ae2566dda0f04b6e4138e35a8f0a7511dcfe24

Comment 10

[Elio Maldonado]

Historical Note:
# RHBZ 1074208 - pass up exact JSS certificate validation errors from NSS
# https://bugzilla.redhat.com/show_bug.cgi?id=1074208
# author: [e-mail]
# Upstream: https://bugzilla.mozilla.org/show_bug.cgi?id=1308026
Patch30:        jss-VerifyCertificate-enhancement.patch

Comment 11

[Endi S. Dewata]

There's an additional patch for this bug posted in comment #11 in bug #1308029.