GNOME.org
 

[gnome-keyring] pkcs11: Allow daemon to access value for credentials.

commit 6d8c3300097d2e3efece5bf1b2b0b853eedfb599
Author: Stef Walter <stefw collabora co uk>
Date:   Tue Jun 7 19:18:00 2011 +0000

    pkcs11: Allow daemon to access value for credentials.
    
     * This allows the daemon code to use the master passwords for
       keyring collections in various algorithms.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=652070

 pkcs11/gkm/gkm-credential.c         |   12 ++++++-
 pkcs11/gkm/gkm-module.c             |    6 +--
 pkcs11/gkm/gkm-session.c            |   43 ++++++++++++++-----------
 pkcs11/gkm/gkm-session.h            |    4 ++-
 pkcs11/gkm/tests/test-credential.c  |   61 +++++++++++++++++++++++++++++++++++
 pkcs11/rpc-layer/gkm-rpc-dispatch.c |    8 ++++
 6 files changed, 109 insertions(+), 25 deletions(-)
---
diff --git a/pkcs11/gkm/gkm-credential.c b/pkcs11/gkm/gkm-credential.c
index eec474c..3573f3f 100644
--- a/pkcs11/gkm/gkm-credential.c
+++ b/pkcs11/gkm/gkm-credential.c
@@ -156,6 +156,8 @@ gkm_credential_real_get_attribute (GkmObject *base, GkmSession *session, CK_ATTR
 {
 	GkmCredential *self = GKM_CREDENTIAL (base);
 	CK_OBJECT_HANDLE handle;
+	gconstpointer value;
+	gsize n_value;
 
 	switch (attr->type) {
 
@@ -170,7 +172,15 @@ gkm_credential_real_get_attribute (GkmObject *base, GkmSession *session, CK_ATTR
 		return gkm_attribute_set_ulong (attr, handle);
 
 	case CKA_VALUE:
-		return CKR_ATTRIBUTE_SENSITIVE;
+		if (gkm_session_is_for_application (session))
+			return CKR_ATTRIBUTE_SENSITIVE;
+		if (!self->pv->secret) {
+			value = NULL;
+			n_value = 0;
+		} else {
+			value = gkm_secret_get (self->pv->secret, &n_value);
+		}
+		return gkm_attribute_set_data (attr, value, n_value);
 	};
 
 	return GKM_OBJECT_CLASS (gkm_credential_parent_class)->get_attribute (base, session, attr);
diff --git a/pkcs11/gkm/gkm-module.c b/pkcs11/gkm/gkm-module.c
index fa2c1b9..5a9ff4c 100644
--- a/pkcs11/gkm/gkm-module.c
+++ b/pkcs11/gkm/gkm-module.c
@@ -1148,7 +1148,6 @@ gkm_module_C_OpenSession (GkmModule *self, CK_SLOT_ID id, CK_FLAGS flags, CK_VOI
 {
 	CK_G_APPLICATION_PTR app;
 	CK_SESSION_HANDLE handle;
-	gboolean read_only;
 	GkmSession *session;
 	Apartment *apt = NULL;
 
@@ -1189,9 +1188,8 @@ gkm_module_C_OpenSession (GkmModule *self, CK_SLOT_ID id, CK_FLAGS flags, CK_VOI
 
 	/* Make and register a new session */
 	handle = gkm_module_next_handle (self);
-	read_only = !(flags & CKF_RW_SESSION);
 	session = g_object_new (GKM_TYPE_SESSION, "slot-id", apt->slot_id, "apartment", apt->apt_id,
-	                        "read-only", read_only, "handle", handle, "module", self,
+	                        "flags", flags, "handle", handle, "module", self,
 	                        "manager", apt->session_manager, "logged-in", apt->logged_in, NULL);
 	apt->sessions = g_list_prepend (apt->sessions, session);
 
@@ -1348,7 +1346,7 @@ gkm_module_C_Login (GkmModule *self, CK_SESSION_HANDLE handle, CK_USER_TYPE user
 
 		/* Can't login as SO if read-only sessions exist */
 		for (l = apt->sessions; l; l = g_list_next (l)) {
-			if (gkm_session_get_read_only (l->data))
+			if (gkm_session_is_read_only (l->data))
 				return CKR_SESSION_READ_ONLY_EXISTS;
 		}
 
diff --git a/pkcs11/gkm/gkm-session.c b/pkcs11/gkm/gkm-session.c
index 9c68b0a..cf082cc 100644
--- a/pkcs11/gkm/gkm-session.c
+++ b/pkcs11/gkm/gkm-session.c
@@ -42,7 +42,7 @@ enum {
 	PROP_SLOT_ID,
 	PROP_APARTMENT,
 	PROP_HANDLE,
-	PROP_READ_ONLY,
+	PROP_FLAGS,
 	PROP_MANAGER,
 	PROP_LOGGED_IN
 };
@@ -58,7 +58,7 @@ struct _GkmSessionPrivate {
 	GkmStore *store;
 
 	CK_USER_TYPE logged_in;
-	gboolean read_only;
+	CK_ULONG flags;
 
 	CK_NOTIFY notify_callback;
 	CK_VOID_PTR application_ptr;
@@ -285,7 +285,7 @@ lookup_object_from_handle (GkmSession *self, CK_OBJECT_HANDLE handle,
 			if (!gkm_object_is_transient (object))
 				if (gkm_module_get_write_protected (self->pv->module))
 					return CKR_TOKEN_WRITE_PROTECTED;
-			if (self->pv->read_only)
+			if (gkm_session_is_read_only (self))
 				return CKR_SESSION_READ_ONLY;
 		}
 	}
@@ -381,7 +381,7 @@ gkm_session_init (GkmSession *self)
 {
 	self->pv = G_TYPE_INSTANCE_GET_PRIVATE (self, GKM_TYPE_SESSION, GkmSessionPrivate);
 	self->pv->objects = g_hash_table_new_full (g_direct_hash, g_direct_equal, NULL, gkm_util_dispose_unref);
-	self->pv->read_only = TRUE;
+	self->pv->flags = 0;
 
 	/* Create the store and register attributes */
 	self->pv->store = GKM_STORE (gkm_memory_store_new ());
@@ -462,8 +462,8 @@ gkm_session_set_property (GObject *obj, guint prop_id, const GValue *value,
 		self->pv->handle = g_value_get_ulong (value);
 		g_return_if_fail (self->pv->handle != 0);
 		break;
-	case PROP_READ_ONLY:
-		self->pv->read_only = g_value_get_boolean (value);
+	case PROP_FLAGS:
+		self->pv->flags = g_value_get_ulong (value);
 		break;
 	case PROP_LOGGED_IN:
 		gkm_session_set_logged_in (self, g_value_get_ulong (value));
@@ -496,8 +496,8 @@ gkm_session_get_property (GObject *obj, guint prop_id, GValue *value,
 	case PROP_HANDLE:
 		g_value_set_ulong (value, gkm_session_get_handle (self));
 		break;
-	case PROP_READ_ONLY:
-		g_value_set_boolean (value, gkm_session_get_read_only (self));
+	case PROP_FLAGS:
+		g_value_set_ulong (value, self->pv->flags);
 		break;
 	case PROP_LOGGED_IN:
 		g_value_set_ulong (value, gkm_session_get_logged_in (self));
@@ -542,9 +542,9 @@ gkm_session_class_init (GkmSessionClass *klass)
 	         g_param_spec_ulong ("apartment", "Apartment", "Apartment this session is opened on",
 	                             0, G_MAXULONG, 0, G_PARAM_READWRITE | G_PARAM_CONSTRUCT_ONLY));
 
-	g_object_class_install_property (gobject_class, PROP_READ_ONLY,
-	         g_param_spec_boolean ("read-only", "Read Only", "Whether a read-only session or not",
-	                               TRUE, G_PARAM_READWRITE | G_PARAM_CONSTRUCT_ONLY));
+	g_object_class_install_property(gobject_class, PROP_FLAGS,
+	         g_param_spec_ulong ("flags", "Flags", "Flags for the session",
+	                             0, G_MAXULONG, 0, G_PARAM_READWRITE | G_PARAM_CONSTRUCT_ONLY));
 
 	g_object_class_install_property (gobject_class, PROP_LOGGED_IN,
 	         g_param_spec_ulong ("logged-in", "Logged in", "Whether this session is logged in or not",
@@ -635,10 +635,17 @@ gkm_session_set_crypto_state (GkmSession *self, gpointer state,
 }
 
 gboolean
-gkm_session_get_read_only (GkmSession *self)
+gkm_session_is_read_only (GkmSession *self)
+{
+	g_return_val_if_fail (GKM_IS_SESSION (self), TRUE);
+	return (self->pv->flags & CKF_RW_SESSION) ? FALSE : TRUE;
+}
+
+gboolean
+gkm_session_is_for_application (GkmSession *self)
 {
 	g_return_val_if_fail (GKM_IS_SESSION (self), TRUE);
-	return self->pv->read_only;
+	return (self->pv->flags & CKF_G_APPLICATION_SESSION) ? TRUE : FALSE;
 }
 
 CK_RV
@@ -837,7 +844,7 @@ gkm_session_complete_object_creation (GkmSession *self, GkmTransaction *transact
 			gkm_transaction_fail (transaction, CKR_TOKEN_WRITE_PROTECTED);
 			return;
 		}
-		else if (self->pv->read_only) {
+		else if (gkm_session_is_read_only (self)) {
 			gkm_transaction_fail (transaction, CKR_SESSION_READ_ONLY);
 			return;
 		}
@@ -900,14 +907,12 @@ gkm_session_C_GetSessionInfo(GkmSession* self, CK_SESSION_INFO_PTR info)
 
 	info->slotID = self->pv->slot_id;
 	if (self->pv->logged_in == CKU_USER)
-		info->state = self->pv->read_only ? CKS_RO_USER_FUNCTIONS : CKS_RW_USER_FUNCTIONS;
+		info->state = gkm_session_is_read_only (self) ? CKS_RO_USER_FUNCTIONS : CKS_RW_USER_FUNCTIONS;
 	else if (self->pv->logged_in == CKU_SO)
 		info->state = CKS_RW_SO_FUNCTIONS;
 	else
-		info->state = self->pv->read_only ? CKS_RO_PUBLIC_SESSION : CKS_RW_PUBLIC_SESSION;
-	info->flags = CKF_SERIAL_SESSION;
-	if (!self->pv->read_only)
-		info->flags |= CKF_RW_SESSION;
+		info->state = gkm_session_is_read_only (self) ? CKS_RO_PUBLIC_SESSION : CKS_RW_PUBLIC_SESSION;
+	info->flags = CKF_SERIAL_SESSION | self->pv->flags;
 	info->ulDeviceError = 0;
 
 	return CKR_OK;
diff --git a/pkcs11/gkm/gkm-session.h b/pkcs11/gkm/gkm-session.h
index 01c69ae..f34ab1e 100644
--- a/pkcs11/gkm/gkm-session.h
+++ b/pkcs11/gkm/gkm-session.h
@@ -68,7 +68,9 @@ GkmModule*               gkm_session_get_module                         (GkmSess
 
 GkmManager*              gkm_session_get_manager                        (GkmSession *self);
 
-gboolean                 gkm_session_get_read_only                      (GkmSession *self);
+gboolean                 gkm_session_is_read_only                       (GkmSession *self);
+
+gboolean                 gkm_session_is_for_application                 (GkmSession *self);
 
 gulong                   gkm_session_get_logged_in                      (GkmSession *self);
 
diff --git a/pkcs11/gkm/tests/test-credential.c b/pkcs11/gkm/tests/test-credential.c
index 5a7b70e..ca94137 100644
--- a/pkcs11/gkm/tests/test-credential.c
+++ b/pkcs11/gkm/tests/test-credential.c
@@ -26,12 +26,15 @@
 #include "mock-module.h"
 #include "mock-locked-object.h"
 
+#include "egg/egg-testing.h"
+
 #include "gkm/gkm-attributes.h"
 #include "gkm/gkm-credential.h"
 #include "gkm/gkm-object.h"
 #include "gkm/gkm-secret.h"
 #include "gkm/gkm-session.h"
 #include "gkm/gkm-module.h"
+#include "gkm/gkm-test.h"
 
 #include "pkcs11i.h"
 
@@ -294,6 +297,62 @@ test_connect_object (Test *test, gconstpointer unused)
 	g_object_unref (cred);
 }
 
+static void
+test_value_is_accessible_to_daemon (Test *test, gconstpointer unused)
+{
+	GkmCredential *cred;
+	gchar buffer[20];
+	CK_ATTRIBUTE attr;
+	CK_RV rv;
+
+	rv = gkm_credential_create (test->module, NULL, NULL, (guchar*)"mock", 4, &cred);
+	g_assert (rv == CKR_OK);
+	g_assert (cred);
+
+	attr.type = CKA_VALUE;
+	attr.pValue = buffer;
+	attr.ulValueLen = sizeof (buffer);
+
+	rv = gkm_object_get_attribute (GKM_OBJECT (cred), test->session, &attr);
+	gkm_assert_cmprv (rv, ==, CKR_OK);
+	egg_assert_cmpmem ("mock", 4, ==, attr.pValue, attr.ulValueLen);
+
+	g_object_unref (cred);
+}
+
+static void
+test_value_is_inaccessible_to_applications (Test *test, gconstpointer unused)
+{
+	GkmCredential *cred;
+	CK_G_APPLICATION app;
+	gchar buffer[20];
+	CK_ATTRIBUTE attr;
+	CK_SESSION_HANDLE handle;
+	GkmSession *session;
+	CK_RV rv;
+
+	memset (&app, 0, sizeof (app));
+	rv = gkm_module_C_OpenSession (test->module, 1, CKF_SERIAL_SESSION | CKF_G_APPLICATION_SESSION, &app, NULL, &handle);
+	gkm_assert_cmprv (rv, ==, CKR_OK);
+
+	session = gkm_module_lookup_session (test->module, handle);
+	g_assert (session);
+
+	rv = gkm_credential_create (test->module, NULL, NULL, (guchar*)"mock", 4, &cred);
+	g_assert (rv == CKR_OK);
+	g_assert (cred);
+
+	attr.type = CKA_VALUE;
+	attr.pValue = buffer;
+	attr.ulValueLen = sizeof (buffer);
+
+	rv = gkm_object_get_attribute (GKM_OBJECT (cred), session, &attr);
+	gkm_assert_cmprv (rv, ==, CKR_ATTRIBUTE_SENSITIVE);
+
+	g_object_unref (cred);
+}
+
+
 int
 main (int argc, char **argv)
 {
@@ -309,6 +368,8 @@ main (int argc, char **argv)
 	g_test_add ("/gkm/credential/login_property", Test, NULL, setup, test_login_property, teardown);
 	g_test_add ("/gkm/credential/data", Test, NULL, setup, test_data, teardown);
 	g_test_add ("/gkm/credential/connect_object", Test, NULL, setup, test_connect_object, teardown);
+	g_test_add ("/gkm/credential/value_is_accessible_to_daemon", Test, NULL, setup, test_value_is_accessible_to_daemon, teardown);
+	g_test_add ("/gkm/credential/value_is_inaccessible_to_applications", Test, NULL, setup, test_value_is_inaccessible_to_applications, teardown);
 
 	return g_test_run ();
 }
diff --git a/pkcs11/rpc-layer/gkm-rpc-dispatch.c b/pkcs11/rpc-layer/gkm-rpc-dispatch.c
index 0748e1c..a32043d 100644
--- a/pkcs11/rpc-layer/gkm-rpc-dispatch.c
+++ b/pkcs11/rpc-layer/gkm-rpc-dispatch.c
@@ -950,6 +950,14 @@ rpc_C_OpenSession (CallState *cs)
 
 	/* Slot id becomes appartment so lower layers can tell clients apart. */
 
+	/*
+	 * IMPORTANT: When we open sessions on behalf of a client caller
+	 * we always specify CKF_G_APPLICATION_SESSION. This allows the module
+	 * to know whether they're talking to the daemon or a client app.
+	 *
+	 * This is a security feature.
+	 */
+
 	BEGIN_CALL (C_OpenSession);
 		IN_ULONG (slot_id);
 		IN_ULONG (flags);
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]