gnome-keyring r1456 - in trunk: . daemon/pkcs11 pkcs11/plex-layer pkcs11/rpc-layer

[nnielsen svn gnome org]

Author: nnielsen
Date: Sat Jan 10 03:15:56 2009
New Revision: 1456
URL: http://svn.gnome.org/viewvc/gnome-keyring?rev=1456&view=rev

Log:
	* daemon/pkcs11/gkr-pkcs11-auth-ep.c:
	* pkcs11/plex-layer/gck-plex-layer.c:
	* pkcs11/rpc-layer/gck-rpc-dispatch.c:
	* pkcs11/rpc-layer/gck-rpc-message.c:
	* pkcs11/rpc-layer/gck-rpc-module.c: Fix corner cases and error code
	problems highlighted by the p11-tests tool.


Modified:
   trunk/ChangeLog
   trunk/daemon/pkcs11/gkr-pkcs11-auth-ep.c
   trunk/pkcs11/plex-layer/gck-plex-layer.c
   trunk/pkcs11/rpc-layer/gck-rpc-dispatch.c
   trunk/pkcs11/rpc-layer/gck-rpc-message.c
   trunk/pkcs11/rpc-layer/gck-rpc-module.c

Modified: trunk/daemon/pkcs11/gkr-pkcs11-auth-ep.c
==============================================================================
--- trunk/daemon/pkcs11/gkr-pkcs11-auth-ep.c	(original)
+++ trunk/daemon/pkcs11/gkr-pkcs11-auth-ep.c	Sat Jan 10 03:15:56 2009
@@ -545,7 +545,7 @@
 		}
 	}
 	
-	return CKR_OK;
+	return rv;
 }
 
 static CK_RV
@@ -684,7 +684,7 @@
 		}
 	}
 	
-	return CKR_OK;
+	return rv;
 }
 
 static CK_RV
@@ -736,7 +736,7 @@
 		}
 	}
 	
-	return CKR_OK;
+	return rv;
 }
 
 static CK_RV
@@ -770,7 +770,7 @@
 		}
 	}
 	
-	return CKR_OK;
+	return rv;
 }
 
 static CK_RV

Modified: trunk/pkcs11/plex-layer/gck-plex-layer.c
==============================================================================
--- trunk/pkcs11/plex-layer/gck-plex-layer.c	(original)
+++ trunk/pkcs11/plex-layer/gck-plex-layer.c	Sat Jan 10 03:15:56 2009
@@ -341,7 +341,8 @@
 	
 	MAP_SLOT_DOWN (id, map);
 	rv = (map.funcs->C_OpenSession) (id, flags, user_data, callback, handle);
-	MAP_SESSION_UP (map, *handle);
+	if (rv == CKR_OK)
+		MAP_SESSION_UP (map, *handle);
 	
 	return rv;
 }

Modified: trunk/pkcs11/rpc-layer/gck-rpc-dispatch.c
==============================================================================
--- trunk/pkcs11/rpc-layer/gck-rpc-dispatch.c	(original)
+++ trunk/pkcs11/rpc-layer/gck-rpc-dispatch.c	Sat Jan 10 03:15:56 2009
@@ -232,6 +232,34 @@
 }
 
 static CK_RV
+proto_write_byte_array (CallState *cs, CK_BYTE_PTR array, CK_ULONG len, CK_RV ret)
+{
+	assert (cs);
+
+	/* 
+	 * When returning an byte array, in many cases we need to pass
+	 * an invalid array along with a length, which signifies CKR_BUFFER_TOO_SMALL.
+	 */
+	
+	switch (ret) {
+	case CKR_BUFFER_TOO_SMALL:
+		array = NULL;
+		/* fall through */
+	case CKR_OK:
+		break;
+		
+	/* Pass all other errors straight through */
+	default:
+		return ret;
+	};
+	
+	if (!gck_rpc_message_write_byte_array (cs->resp, array, len))
+		return PREP_ERROR;
+
+	return CKR_OK;
+}
+
+static CK_RV
 proto_read_ulong_buffer (CallState *cs, CK_ULONG_PTR* buffer, CK_ULONG* n_buffer)
 {
 	GckRpcMessage *msg;
@@ -264,6 +292,33 @@
 	return CKR_OK;
 }
 
+static CK_RV
+proto_write_ulong_array (CallState *cs, CK_ULONG_PTR array, CK_ULONG len, CK_RV ret)
+{
+	assert (cs);
+
+	/* 
+	 * When returning an ulong array, in many cases we need to pass
+	 * an invalid array along with a length, which signifies CKR_BUFFER_TOO_SMALL.
+	 */
+	
+	switch (ret) {
+	case CKR_BUFFER_TOO_SMALL:
+		array = NULL;
+		/* fall through */
+	case CKR_OK:
+		break;
+		
+	/* Pass all other errors straight through */
+	default:
+		return ret;
+	};
+	
+	if (!gck_rpc_message_write_ulong_array (cs->resp, array, len))
+		return PREP_ERROR;
+
+	return CKR_OK;
+}
 
 static CK_RV
 proto_read_attribute_buffer (CallState *cs, CK_ATTRIBUTE_PTR* result, CK_ULONG* n_result)
@@ -650,12 +705,12 @@
 		_ret = PREP_ERROR;
 
 #define OUT_BYTE_ARRAY(array, len) \
-	if (_ret == CKR_OK && !gck_rpc_message_write_byte_array (cs->resp, array, len)) \
-		_ret = PREP_ERROR;
+	/* Note how we filter return codes */ \
+	_ret = proto_write_byte_array (cs, array, len, _ret);
 
 #define OUT_ULONG_ARRAY(array, len) \
-	if (_ret == CKR_OK && !gck_rpc_message_write_ulong_array (cs->resp, array, len)) \
-		_ret = PREP_ERROR;
+	/* Note how we filter return codes */ \
+	_ret = proto_write_ulong_array (cs, array, len, _ret);
 
 #define OUT_ATTRIBUTE_ARRAY(array, len) \
 	/* Note how we filter return codes */ \

Modified: trunk/pkcs11/rpc-layer/gck-rpc-message.c
==============================================================================
--- trunk/pkcs11/rpc-layer/gck-rpc-message.c	(original)
+++ trunk/pkcs11/rpc-layer/gck-rpc-message.c	Sat Jan 10 03:15:56 2009
@@ -346,7 +346,6 @@
 gck_rpc_message_write_byte_array (GckRpcMessage *msg, CK_BYTE_PTR arr, CK_ULONG num)
 {
 	assert (msg);
-	assert (!num || arr);
 
 	/* Make sure this is in the right order */
 	assert (!msg->signature || gck_rpc_message_verify_part (msg, "ay"));

Modified: trunk/pkcs11/rpc-layer/gck-rpc-module.c
==============================================================================
--- trunk/pkcs11/rpc-layer/gck-rpc-module.c	(original)
+++ trunk/pkcs11/rpc-layer/gck-rpc-module.c	Sat Jan 10 03:15:56 2009
@@ -756,27 +756,17 @@
 	if (!gkr_buffer_get_byte (&msg->buffer, msg->parsed, &msg->parsed, &valid))
 		return PARSE_ERROR;
 	
-	/* If not valid, then just the length is encoded */
+	/* If not valid, then just the length is encoded, this can signify CKR_BUFFER_TOO_SMALL */
 	if (!valid) {
 		if (!gkr_buffer_get_uint32 (&msg->buffer, msg->parsed, &msg->parsed, &vlen))
 			return PARSE_ERROR;
 		
-		if (arr) {
-			
-			/*
-			 * This should never happen in normal operation. It denotes a goof up 
-			 * on the other side of our RPC. We should be sending an empty buffer 
-			 * only in the case where there's no array to be filled, which is what 
-			 * indicates the other side to reply with an invalid array.
-			 */
-
-			warning (("received an invalid array, but caller expected filled"));
-			return PARSE_ERROR;
-		}
-		
-		/* Just return the length */
 		*len = vlen;
-		return CKR_OK;
+		
+		if (arr)
+			return CKR_BUFFER_TOO_SMALL;
+		else
+			return CKR_OK;
 	} 
 
 	/* Get the actual bytes */
@@ -820,22 +810,12 @@
 
 	*len = num;
 
+	/* If not valid, then just the length is encoded, this can signify CKR_BUFFER_TOO_SMALL */
 	if (!valid) {
-
-		if (arr) {
-			
-			/*
-			 * This should never happen in normal operation. It denotes a goof up 
-			 * on the other side of our RPC. We should be sending an empty buffer 
-			 * only in the case where there's no array to be filled, which is what 
-			 * indicates the other side to reply with an invalid array.
-			 */
-
-			warning (("received an invalid array, but caller expected filled"));
-			return PARSE_ERROR;
-		}
-
-		return CKR_OK;
+		if (arr) 
+			return CKR_BUFFER_TOO_SMALL;
+		else
+			return CKR_OK;
 	}
 
 	if (max < num)
@@ -1026,7 +1006,7 @@
 	
 #define IN_BYTE_ARRAY(arr, len) \
 	if (len != 0 && arr == NULL) \
-		_ret = CKR_ARGUMENTS_BAD; \
+		{ _ret = CKR_ARGUMENTS_BAD; goto _cleanup; } \
 	if (!gck_rpc_message_write_byte_array (_cs->req, arr, len)) \
 		{ _ret = CKR_HOST_MEMORY; goto _cleanup; }
 
@@ -1038,7 +1018,7 @@
 	
 #define IN_ULONG_ARRAY(arr, len) \
 	if (len != 0 && arr == NULL) \
-		_ret = CKR_ARGUMENTS_BAD; \
+		{ _ret = CKR_ARGUMENTS_BAD; goto _cleanup; }\
 	if (!gck_rpc_message_write_ulong_array (_cs->req, arr, len)) \
 		{ _ret = CKR_HOST_MEMORY; goto _cleanup; }
 
@@ -1062,7 +1042,7 @@
 
 #define IN_MECHANISM(val) \
 	if (val == NULL) \
-		_ret = CKR_ARGUMENTS_BAD; \
+		{ _ret = CKR_ARGUMENTS_BAD; goto _cleanup; } \
 	_ret = proto_write_mechanism (_cs->req, val); \
 	if (_ret != CKR_OK) goto _cleanup;