144913 - verify Certificates that exist in database

Status: RESOLVED FIXED

(JSS Graveyard :: Library, defect)

Description

[glen beasley]

Request for verifiying Certificates with condition that they exist in database.
Another request for verifiying certificates that do not exist in the database
will be a separate bug.

Jamie, I am sure this is not what your talking about for the code review but I
thought I should create a bug anyways. 

changes to three files: jss.def CryptoManager.c and CryptoManager.Java
 
>     public final static class CertUsage {
>       private int usage;
>       private CertUsage() {};
>       private CertUsage(int usage) {
>           this.usage = usage;
>       }
>         public int getUsage() {
>           return usage;
>       }
>       // certUsage, these must be kept in sync with nss/lib/certdb/certt.h
>         public static final CertUsage SSLClient = new CertUsage(0);
>         public static final CertUsage SSLServer = new CertUsage(1);
>         public static final CertUsage SSLServerWithStepUp = new CertUsage(2);
>         public static final CertUsage SSLCA = new CertUsage(3);
>         public static final CertUsage EmailSigner = new CertUsage(4);
>         public static final CertUsage EmailRecipient = new CertUsage(5);
>         public static final CertUsage ObjectSigner = new CertUsage(6);
>         public static final CertUsage UserCertImport = new CertUsage(7);
>         public static final CertUsage VerifyCA = new CertUsage(8);
>         public static final CertUsage ProtectedObjectSigner = new
CertUsage(9);
>         public static final CertUsage StatusResponder = new CertUsage(10);
>         public static final CertUsage AnyCA = new CertUsage(11);
>     }
> 
1213c1237
<      *      found matching the given certificate.
---
>      *      found matching the given certificate.      
1347a1372,1400
> 
>     /////////////////////////////////////////////////////////////
>     // isCertValid
>     /////////////////////////////////////////////////////////////
>     /**
>      * Verify a certificate that exists in the given cert database, 
>      * check if is valid and that we trust the issuer. Verify time
>      * against Now.  
>      * @param nickname The nickname of the certificate to verify.
>      * @param checkSig verify the signature of the certificate
>      * @param certUsage see exposed certUsage defines to verify Certificate 
>      * @return true for success; false otherwise 
>      *      
>      * @exception InvalidNicknameException If the nickname is null 
>      * @exception ObjectNotFoundException If no certificate could be found
>      *      with the given nickname.
>      */
> 
>     public boolean isCertValid(String nickname, boolean checkSig, int
certUsage) 
>          throws ObjectNotFoundException,
>               InvalidNicknameException
>     {
>       if (nickname==null) {
>            throw new InvalidNicknameException("Nickname must be non-null");
>       }
>       return verifyCertNowNative(nickname, checkSig, certUsage);
>     }
> 
>     private native boolean verifyCertNowNative(String nickname, boolean
checkSig, int cUsage) throws ObjectNotFoundException; 

and cryptoManager.c

> /***********************************************************************
>  * CryptoManager.verifyCertNowNative
>  *
>  * Returns JNI_TRUE if success, JNI_FALSE otherwise
>  */
> JNIEXPORT jboolean JNICALL
> Java_org_mozilla_jss_CryptoManager_verifyCertNowNative(JNIEnv *env, jobject
self, jstring nickString, jboolean checkSig, jint cUsage) {
>     SECStatus         rv    = SECFailure;
>     SECCertUsage      certUsage;
>     CERTCertificate   *cert=NULL;
>     char *nickname=NULL;
> 
>     nickname = (char *) (*env)->GetStringUTFChars(env, nickString, NULL);
>     if( nickname == NULL ) {
>          goto finish;
>     }
>     certUsage = cUsage; 
>     cert = CERT_FindCertByNickname(CERT_GetDefaultCertDB(), nickname);
>     
>     if (cert == NULL) {
>       JSS_throw(env, OBJECT_NOT_FOUND_EXCEPTION);
>       goto finish;
> 
>     } else {
>       rv = CERT_VerifyCertNow(CERT_GetDefaultCertDB(), cert, 
>              checkSig, certUsage, NULL );
>     }
> 
> finish: 
>     if(nickname != NULL) {
>       (*env)->ReleaseStringUTFChars(env, nickString, nickname);
>     }
>     if(cert != NULL) {
>        CERT_DestroyCertificate(cert);
>     }
>     if( rv == SECSuccess) {
>         return JNI_TRUE;
>     } else {
>         return JNI_FALSE;
>     }
> }

Example usage: 

>       if (cm.isCertValid(nickname, true,
CryptoManager.CertUsage.SSLClient.getUsage()) == true) {
>            System.out.println("verified" +
CryptoManager.CertUsage.SSLClient.getUsage());
>       } else {
>          System.out.println("not valid"); 
>       }

Comment 1

[Jamie Nicolson]

These changes look great Glen, just two more comments below. The way to include
patches in bugzilla is to first capture a context diff with "cvs diff -u
>patch", and then attach that patch file to the bug with the "Create a New
Attachment" link above.

1. The point of the CertUsage class is that it should be transparent to users.
They shouldn't have to call the getUsage() method. The certUsage argument to
isCertValid() should be a CertUsage instead of an int. Then you pass
certUsage.getUsage() into the native function. The example then becomes:

if (cm.isCertValid(nickname, true,CryptoManager.CertUsage.SSLClient) == true) {
    System.out.println("verified" + CryptoManager.CertUsage.SSLClient);
} else {
    System.out.println("not valid"); 
}

The second line will only work if you add a toString() method to the CertUsage
class. You could make CertUsage.getUsage() have package-level access, since
users don't need to call it.

2. Can you put the native code into PK11Finder.c instead of CryptoManager.c? I
reserve CryptoManager.c for initialization type stuff, while all the certificate
and miscellaneous functions go in PK11Finder.c.

Comment 2

[glen beasley]

check in isCertValid method
Checking in CryptoManager.java;
/cvsroot/mozilla/security/jss/org/mozilla/jss/CryptoManager.java,v  <--
CryptoManager.java
new revision: 1.6; previous revision: 1.5
done
Checking in PK11Finder.c;
/cvsroot/mozilla/security/jss/org/mozilla/jss/PK11Finder.c,v  <--  PK11Finder.c
new revision: 1.2; previous revision: 1.1
Checking in jss.def;
/cvsroot/mozilla/security/jss/lib/jss.def,v  <--  jss.def
new revision: 1.14; previous revision: 1.13

Comment 3

[Jamie Nicolson]

These changes look good. A few comments though :)

1) The convention (unfortunately not always followed) for JSS code is that no
tab characters should be used--only spaces. It seems that most of
CryptoManager.java is already infected with tabs. I'll clean it up.

2) Please limit line length to 80 characters, i.e., no line wraps if possible.

3) The new entry in jss.def should go toward the end of the file, in the JSS 3.2
section. I'm not sure what will go wrong if its in an earlier section--possibly
backwards compatibility will be broken.