Bugzilla Bug 354900: code cleanup -- consolidates some duplicate code,

checks for NULL pointers, and renames the SFTK_IS_SECURE_KEY_OBJECT macro.
r=glen.beasley,relyea.
Modified files: fipsaudt.c fipstokn.c
 Tag: NSS_3_11_BRANCH

2 files changed, 108 insertions, 76 deletions

diff --git a/security/nss/lib/softoken/fipsaudt.c b/security/nss/lib/softoken/fipsaudt.c
index dc4393083..d17496deb 100644
--- a/security/nss/lib/softoken/fipsaudt.c
+++ b/security/nss/lib/softoken/fipsaudt.c
@@ -42,6 +42,53 @@
 #include "prprf.h"
 #include "softoken.h"
 
+/*
+ * Print the value of the returned object handle in the output buffer
+ * on a successful return of the PKCS #11 function.  If the PKCS #11
+ * function failed or the pointer to object handle is NULL (which is
+ * the case for C_DeriveKey with CKM_TLS_KEY_AND_MAC_DERIVE), an empty
+ * string is stored in the output buffer.
+ *
+ * out: the output buffer
+ * outlen: the length of the output buffer
+ * argName: the name of the "pointer to object handle" argument
+ * phObject: the pointer to object handle
+ * rv: the return value of the PKCS #11 function
+ */
+static void sftk_PrintReturnedObjectHandle(char *out, PRUint32 outlen,
+    const char *argName, CK_OBJECT_HANDLE_PTR phObject, CK_RV rv)
+{
+    if ((rv == CKR_OK) && phObject) {
+	PR_snprintf(out, outlen,
+	    " *%s=0x%08lX", argName, (PRUint32)*phObject);
+    } else {
+	PORT_Assert(outlen != 0);
+	out[0] = '\0';
+    }
+}
+
+/*
+ * MECHANISM_BUFSIZE needs to be large enough for sftk_PrintMechanism,
+ * which uses <= 49 bytes.
+ */
+#define MECHANISM_BUFSIZE 64
+
+static void sftk_PrintMechanism(char *out, PRUint32 outlen,
+    CK_MECHANISM_PTR pMechanism)
+{
+    if (pMechanism) {
+	/*
+	 * If we change the format string, we need to make sure
+	 * MECHANISM_BUFSIZE is still large enough.  We allow
+	 * 20 bytes for %p on a 64-bit platform.
+	 */
+	PR_snprintf(out, outlen, "%p {mechanism=0x%08lX, ...}",
+	    pMechanism, (PRUint32)pMechanism->mechanism);
+    } else {
+	PR_snprintf(out, outlen, "%p", pMechanism);
+    }
+}
+
 void sftk_AuditCreateObject(CK_SESSION_HANDLE hSession,
     CK_ATTRIBUTE_PTR pTemplate, CK_ULONG ulCount,
     CK_OBJECT_HANDLE_PTR phObject, CK_RV rv)
@@ -50,12 +97,9 @@ void sftk_AuditCreateObject(CK_SESSION_HANDLE hSession,
     char shObject[32];
     NSSAuditSeverity severity = (rv == CKR_OK) ?
 	NSS_AUDIT_INFO : NSS_AUDIT_ERROR;
-    if (rv == CKR_OK) {
-	PR_snprintf(shObject, sizeof shObject, " *phObject=0x%08lX",
-	    (PRUint32)*phObject);
-    } else {
-	shObject[0] = '\0';
-    }
+
+    sftk_PrintReturnedObjectHandle(shObject, sizeof shObject,
+	"phObject", phObject, rv);
     PR_snprintf(msg, sizeof msg,
 	"C_CreateObject(hSession=0x%08lX, pTemplate=%p, ulCount=%lu, "
 	"phObject=%p)=0x%08lX%s",
@@ -72,12 +116,9 @@ void sftk_AuditCopyObject(CK_SESSION_HANDLE hSession,
     char shNewObject[32];
     NSSAuditSeverity severity = (rv == CKR_OK) ?
 	NSS_AUDIT_INFO : NSS_AUDIT_ERROR;
-    if (rv == CKR_OK) {
-	PR_snprintf(shNewObject, sizeof shNewObject,
-	    " *phNewObject=0x%08lX", (PRUint32)*phNewObject);
-    } else {
-	shNewObject[0] = '\0';
-    }
+
+    sftk_PrintReturnedObjectHandle(shNewObject, sizeof shNewObject,
+	"phNewObject", phNewObject, rv);
     PR_snprintf(msg, sizeof msg,
 	"C_CopyObject(hSession=0x%08lX, hObject=0x%08lX, "
 	"pTemplate=%p, ulCount=%lu, phNewObject=%p)=0x%08lX%s",
@@ -93,6 +134,7 @@ void sftk_AuditDestroyObject(CK_SESSION_HANDLE hSession,
     char msg[256];
     NSSAuditSeverity severity = (rv == CKR_OK) ?
 	NSS_AUDIT_INFO : NSS_AUDIT_ERROR;
+
     PR_snprintf(msg, sizeof msg,
 	"C_DestroyObject(hSession=0x%08lX, hObject=0x%08lX)=0x%08lX",
 	(PRUint32)hSession, (PRUint32)hObject, (PRUint32)rv);
@@ -105,6 +147,7 @@ void sftk_AuditGetObjectSize(CK_SESSION_HANDLE hSession,
     char msg[256];
     NSSAuditSeverity severity = (rv == CKR_OK) ?
 	NSS_AUDIT_INFO : NSS_AUDIT_ERROR;
+
     PR_snprintf(msg, sizeof msg,
 	"C_GetObjectSize(hSession=0x%08lX, hObject=0x%08lX, "
 	"pulSize=%p)=0x%08lX",
@@ -120,6 +163,7 @@ void sftk_AuditGetAttributeValue(CK_SESSION_HANDLE hSession,
     char msg[256];
     NSSAuditSeverity severity = (rv == CKR_OK) ?
 	NSS_AUDIT_INFO : NSS_AUDIT_ERROR;
+
     PR_snprintf(msg, sizeof msg,
 	"C_GetAttributeValue(hSession=0x%08lX, hObject=0x%08lX, "
 	"pTemplate=%p, ulCount=%lu)=0x%08lX",
@@ -135,6 +179,7 @@ void sftk_AuditSetAttributeValue(CK_SESSION_HANDLE hSession,
     char msg[256];
     NSSAuditSeverity severity = (rv == CKR_OK) ?
 	NSS_AUDIT_INFO : NSS_AUDIT_ERROR;
+
     PR_snprintf(msg, sizeof msg,
 	"C_SetAttributeValue(hSession=0x%08lX, hObject=0x%08lX, "
 	"pTemplate=%p, ulCount=%lu)=0x%08lX",
@@ -147,12 +192,15 @@ void sftk_AuditCryptInit(const char *opName, CK_SESSION_HANDLE hSession,
     CK_MECHANISM_PTR pMechanism, CK_OBJECT_HANDLE hKey, CK_RV rv)
 {
     char msg[256];
+    char mech[MECHANISM_BUFSIZE];
     NSSAuditSeverity severity = (rv == CKR_OK) ?
 	NSS_AUDIT_INFO : NSS_AUDIT_ERROR;
+
+    sftk_PrintMechanism(mech, sizeof mech, pMechanism);
     PR_snprintf(msg, sizeof msg,
-	"C_%sInit(hSession=0x%08lX, pMechanism->mechanism=0x%08lX, "
+	"C_%sInit(hSession=0x%08lX, pMechanism=%s, "
 	"hKey=0x%08lX)=0x%08lX",
-	opName, (PRUint32)hSession, (PRUint32)pMechanism->mechanism,
+	opName, (PRUint32)hSession, mech,
 	(PRUint32)hKey, (PRUint32)rv);
     sftk_LogAuditMessage(severity, msg);
 }
@@ -162,19 +210,17 @@ void sftk_AuditGenerateKey(CK_SESSION_HANDLE hSession,
     CK_ULONG ulCount, CK_OBJECT_HANDLE_PTR phKey, CK_RV rv)
 {
     char msg[256];
+    char mech[MECHANISM_BUFSIZE];
     char shKey[32];
     NSSAuditSeverity severity = (rv == CKR_OK) ?
 	NSS_AUDIT_INFO : NSS_AUDIT_ERROR;
-    if (rv == CKR_OK) {
-	PR_snprintf(shKey, sizeof shKey,
-	    " *phKey=0x%08lX", (PRUint32)*phKey);
-    } else {
-        shKey[0] = '\0';
-    }
+
+    sftk_PrintMechanism(mech, sizeof mech, pMechanism);
+    sftk_PrintReturnedObjectHandle(shKey, sizeof shKey, "phKey", phKey, rv);
     PR_snprintf(msg, sizeof msg,
-	"C_GenerateKey(hSession=0x%08lX, pMechanism->mechanism=0x%08lX, "
+	"C_GenerateKey(hSession=0x%08lX, pMechanism=%s, "
 	"pTemplate=%p, ulCount=%lu, phKey=%p)=0x%08lX%s",
-	(PRUint32)hSession, (PRUint32)pMechanism->mechanism,
+	(PRUint32)hSession, mech,
 	pTemplate, (PRUint32)ulCount, phKey, (PRUint32)rv, shKey);
     sftk_LogAuditMessage(severity, msg);
 }
@@ -186,24 +232,23 @@ void sftk_AuditGenerateKeyPair(CK_SESSION_HANDLE hSession,
     CK_OBJECT_HANDLE_PTR phPrivateKey, CK_RV rv)
 {
     char msg[512];
+    char mech[MECHANISM_BUFSIZE];
     char shPublicKey[32];
     char shPrivateKey[32];
     NSSAuditSeverity severity = (rv == CKR_OK) ?
 	NSS_AUDIT_INFO : NSS_AUDIT_ERROR;
-    if (rv == CKR_OK) {
-	PR_snprintf(shPublicKey, sizeof shPublicKey,
-	    " *phPublicKey=0x%08lX", (PRUint32)*phPublicKey);
-	PR_snprintf(shPrivateKey, sizeof shPrivateKey,
-	    " *phPrivateKey=0x%08lX", (PRUint32)*phPrivateKey);
-    } else {
-	shPublicKey[0] = shPrivateKey[0] = '\0';
-    }
+
+    sftk_PrintMechanism(mech, sizeof mech, pMechanism);
+    sftk_PrintReturnedObjectHandle(shPublicKey, sizeof shPublicKey,
+	"phPublicKey", phPublicKey, rv);
+    sftk_PrintReturnedObjectHandle(shPrivateKey, sizeof shPrivateKey,
+	"phPrivateKey", phPrivateKey, rv);
     PR_snprintf(msg, sizeof msg,
-	"C_GenerateKeyPair(hSession=0x%08lX, pMechanism->mechanism=0x%08lX, "
+	"C_GenerateKeyPair(hSession=0x%08lX, pMechanism=%s, "
 	"pPublicKeyTemplate=%p, ulPublicKeyAttributeCount=%lu, "
 	"pPrivateKeyTemplate=%p, ulPrivateKeyAttributeCount=%lu, "
 	"phPublicKey=%p, phPrivateKey=%p)=0x%08lX%s%s",
-	(PRUint32)hSession, (PRUint32)pMechanism->mechanism,
+	(PRUint32)hSession, mech,
 	pPublicKeyTemplate, (PRUint32)ulPublicKeyAttributeCount,
 	pPrivateKeyTemplate, (PRUint32)ulPrivateKeyAttributeCount,
 	phPublicKey, phPrivateKey, (PRUint32)rv, shPublicKey, shPrivateKey);
@@ -216,13 +261,16 @@ void sftk_AuditWrapKey(CK_SESSION_HANDLE hSession,
     CK_ULONG_PTR pulWrappedKeyLen, CK_RV rv)
 {
     char msg[256];
+    char mech[MECHANISM_BUFSIZE];
     NSSAuditSeverity severity = (rv == CKR_OK) ?
 	NSS_AUDIT_INFO : NSS_AUDIT_ERROR;
+
+    sftk_PrintMechanism(mech, sizeof mech, pMechanism);
     PR_snprintf(msg, sizeof msg,
-	"C_WrapKey(hSession=0x%08lX, hWrappingKey=0x%08lX, hKey=0x%08lX, "
-	"pWrappedKey=%p, pulWrappedKeyLen=%p)=0x%08lX",
-	(PRUint32)hSession, (PRUint32)hWrappingKey, (PRUint32)hKey,
-	pWrappedKey, pulWrappedKeyLen, (PRUint32)rv);
+	"C_WrapKey(hSession=0x%08lX, pMechanism=%s, hWrappingKey=0x%08lX, "
+	"hKey=0x%08lX, pWrappedKey=%p, pulWrappedKeyLen=%p)=0x%08lX",
+	(PRUint32)hSession, mech, (PRUint32)hWrappingKey,
+	(PRUint32)hKey, pWrappedKey, pulWrappedKeyLen, (PRUint32)rv);
     sftk_LogAuditMessage(severity, msg);
 }
 
@@ -233,20 +281,18 @@ void sftk_AuditUnwrapKey(CK_SESSION_HANDLE hSession,
     CK_OBJECT_HANDLE_PTR phKey, CK_RV rv)
 {
     char msg[256];
+    char mech[MECHANISM_BUFSIZE];
     char shKey[32];
     NSSAuditSeverity severity = (rv == CKR_OK) ?
 	NSS_AUDIT_INFO : NSS_AUDIT_ERROR;
-    if (rv == CKR_OK) {
-	PR_snprintf(shKey, sizeof shKey,
-	    " *phKey=0x%08lX", (PRUint32)*phKey);
-    } else {
-	shKey[0] = '\0';
-    }
+
+    sftk_PrintMechanism(mech, sizeof mech, pMechanism);
+    sftk_PrintReturnedObjectHandle(shKey, sizeof shKey, "phKey", phKey, rv);
     PR_snprintf(msg, sizeof msg,
-	"C_UnwrapKey(hSession=0x%08lX, pMechanism->mechanism=0x%08lX, "
+	"C_UnwrapKey(hSession=0x%08lX, pMechanism=%s, "
 	"hUnwrappingKey=0x%08lX, pWrappedKey=%p, ulWrappedKeyLen=%lu, "
 	"pTemplate=%p, ulAttributeCount=%lu, phKey=%p)=0x%08lX%s",
-	(PRUint32)hSession, (PRUint32)pMechanism->mechanism,
+	(PRUint32)hSession, mech,
 	(PRUint32)hUnwrappingKey, pWrappedKey, (PRUint32)ulWrappedKeyLen,
 	pTemplate, (PRUint32)ulAttributeCount, phKey, (PRUint32)rv, shKey);
     sftk_LogAuditMessage(severity, msg);
@@ -258,17 +304,14 @@ void sftk_AuditDeriveKey(CK_SESSION_HANDLE hSession,
     CK_OBJECT_HANDLE_PTR phKey, CK_RV rv)
 {
     char msg[512];
+    char mech[MECHANISM_BUFSIZE];
     char shKey[32];
     char sTlsKeys[128];
     NSSAuditSeverity severity = (rv == CKR_OK) ?
 	NSS_AUDIT_INFO : NSS_AUDIT_ERROR;
-    /* phKey is NULL for CKM_TLS_KEY_AND_MAC_DERIVE */
-    if ((rv == CKR_OK) && phKey) {
-	PR_snprintf(shKey, sizeof shKey,
-	    " *phKey=0x%08lX", (PRUint32)*phKey);
-    } else {
-	shKey[0] = '\0';
-    }
+
+    sftk_PrintMechanism(mech, sizeof mech, pMechanism);
+    sftk_PrintReturnedObjectHandle(shKey, sizeof shKey, "phKey", phKey, rv);
     if ((rv == CKR_OK) &&
 	(pMechanism->mechanism == CKM_TLS_KEY_AND_MAC_DERIVE)) {
 	CK_SSL3_KEY_MAT_PARAMS *param =
@@ -285,10 +328,10 @@ void sftk_AuditDeriveKey(CK_SESSION_HANDLE hSession,
 	sTlsKeys[0] = '\0';
     }
     PR_snprintf(msg, sizeof msg,
-	"C_DeriveKey(hSession=0x%08lX, pMechanism->mechanism=0x%08lX, "
+	"C_DeriveKey(hSession=0x%08lX, pMechanism=%s, "
 	"hBaseKey=0x%08lX, pTemplate=%p, ulAttributeCount=%lu, "
 	"phKey=%p)=0x%08lX%s%s",
-	(PRUint32)hSession, (PRUint32)pMechanism->mechanism,
+	(PRUint32)hSession, mech,
 	(PRUint32)hBaseKey, pTemplate,(PRUint32)ulAttributeCount,
 	phKey, (PRUint32)rv, shKey, sTlsKeys);
     sftk_LogAuditMessage(severity, msg);
@@ -300,6 +343,7 @@ void sftk_AuditDigestKey(CK_SESSION_HANDLE hSession,
     char msg[256];
     NSSAuditSeverity severity = (rv == CKR_OK) ?
 	NSS_AUDIT_INFO : NSS_AUDIT_ERROR;
+
     PR_snprintf(msg, sizeof msg,
 	"C_DigestKey(hSession=0x%08lX, hKey=0x%08lX)=0x%08lX",
 	(PRUint32)hSession, (PRUint32)hKey, (PRUint32)rv);
diff --git a/security/nss/lib/softoken/fipstokn.c b/security/nss/lib/softoken/fipstokn.c
index 81f1238a6..a53b9b047 100644
--- a/security/nss/lib/softoken/fipstokn.c
+++ b/security/nss/lib/softoken/fipstokn.c
@@ -303,11 +303,11 @@ static CK_FUNCTION_LIST sftk_fipsTable = {
     ((objClass) == CKO_PRIVATE_KEY) || \
     ((objClass) == CKO_SECRET_KEY))
 
-#define SFTK_IS_SECURE_KEY_OBJECT(objClass) \
+#define SFTK_IS_NONPUBLIC_KEY_OBJECT(objClass) \
     (((objClass) == CKO_PRIVATE_KEY) || ((objClass) == CKO_SECRET_KEY))
 
 static CK_RV
-fips_get_object_class(CK_SESSION_HANDLE hSession,
+sftk_get_object_class_and_fipsCheck(CK_SESSION_HANDLE hSession,
     CK_OBJECT_HANDLE hObject, CK_OBJECT_CLASS *pObjClass)
 {
     CK_RV rv;
@@ -316,6 +316,9 @@ fips_get_object_class(CK_SESSION_HANDLE hSession,
     class.pValue = pObjClass;
     class.ulValueLen = sizeof(*pObjClass);
     rv = NSC_GetAttributeValue(hSession, hObject, &class, 1);
+    if ((rv == CKR_OK) && SFTK_IS_NONPUBLIC_KEY_OBJECT(*pObjClass)) {
+	rv = sftk_fipsCheck();
+    }
     return rv;
 }
 
@@ -693,7 +696,7 @@ CK_RV FC_GetSlotInfo(CK_SLOT_ID slotID, CK_SLOT_INFO_PTR pInfo) {
     if (classptr == NULL) return CKR_TEMPLATE_INCOMPLETE;
 
     /* FIPS can't create keys from raw key material */
-    if (SFTK_IS_SECURE_KEY_OBJECT(*classptr)) {
+    if (SFTK_IS_NONPUBLIC_KEY_OBJECT(*classptr)) {
 	rv = CKR_ATTRIBUTE_VALUE_INVALID;
     } else {
 	rv = NSC_CreateObject(hSession,pTemplate,ulCount,phObject);
@@ -715,10 +718,7 @@ CK_RV FC_GetSlotInfo(CK_SLOT_ID slotID, CK_SLOT_INFO_PTR pInfo) {
     CK_RV rv;
     CK_OBJECT_CLASS objClass = CKO_NOT_A_KEY;
     SFTK_FIPSFATALCHECK();
-    rv = fips_get_object_class(hSession, hObject, &objClass);
-    if ((rv == CKR_OK) && SFTK_IS_SECURE_KEY_OBJECT(objClass)) {
-	rv = sftk_fipsCheck();
-    }
+    rv = sftk_get_object_class_and_fipsCheck(hSession, hObject, &objClass);
     if (rv == CKR_OK) {
 	rv = NSC_CopyObject(hSession,hObject,pTemplate,ulCount,phNewObject);
     }
@@ -736,10 +736,7 @@ CK_RV FC_GetSlotInfo(CK_SLOT_ID slotID, CK_SLOT_INFO_PTR pInfo) {
     CK_RV rv;
     CK_OBJECT_CLASS objClass = CKO_NOT_A_KEY;
     SFTK_FIPSFATALCHECK();
-    rv = fips_get_object_class(hSession, hObject, &objClass);
-    if ((rv == CKR_OK) && SFTK_IS_SECURE_KEY_OBJECT(objClass)) {
-	rv = sftk_fipsCheck();
-    }
+    rv = sftk_get_object_class_and_fipsCheck(hSession, hObject, &objClass);
     if (rv == CKR_OK) {
 	rv = NSC_DestroyObject(hSession,hObject);
     }
@@ -756,10 +753,7 @@ CK_RV FC_GetSlotInfo(CK_SLOT_ID slotID, CK_SLOT_INFO_PTR pInfo) {
     CK_RV rv;
     CK_OBJECT_CLASS objClass = CKO_NOT_A_KEY;
     SFTK_FIPSFATALCHECK();
-    rv = fips_get_object_class(hSession, hObject, &objClass);
-    if ((rv == CKR_OK) && SFTK_IS_SECURE_KEY_OBJECT(objClass)) {
-	rv = sftk_fipsCheck();
-    }
+    rv = sftk_get_object_class_and_fipsCheck(hSession, hObject, &objClass);
     if (rv == CKR_OK) {
 	rv = NSC_GetObjectSize(hSession, hObject, pulSize);
     }
@@ -776,10 +770,7 @@ CK_RV FC_GetSlotInfo(CK_SLOT_ID slotID, CK_SLOT_INFO_PTR pInfo) {
     CK_RV rv;
     CK_OBJECT_CLASS objClass = CKO_NOT_A_KEY;
     SFTK_FIPSFATALCHECK();
-    rv = fips_get_object_class(hSession, hObject, &objClass);
-    if ((rv == CKR_OK) && SFTK_IS_SECURE_KEY_OBJECT(objClass)) {
-	rv = sftk_fipsCheck();
-    }
+    rv = sftk_get_object_class_and_fipsCheck(hSession, hObject, &objClass);
     if (rv == CKR_OK) {
 	rv = NSC_GetAttributeValue(hSession,hObject,pTemplate,ulCount);
     }
@@ -796,10 +787,7 @@ CK_RV FC_GetSlotInfo(CK_SLOT_ID slotID, CK_SLOT_INFO_PTR pInfo) {
     CK_RV rv;
     CK_OBJECT_CLASS objClass = CKO_NOT_A_KEY;
     SFTK_FIPSFATALCHECK();
-    rv = fips_get_object_class(hSession, hObject, &objClass);
-    if ((rv == CKR_OK) && SFTK_IS_SECURE_KEY_OBJECT(objClass)) {
-	rv = sftk_fipsCheck();
-    }
+    rv = sftk_get_object_class_and_fipsCheck(hSession, hObject, &objClass);
     if (rv == CKR_OK) {
 	rv = NSC_SetAttributeValue(hSession,hObject,pTemplate,ulCount);
     }