secret-store: Support the xdg:schema attribute correctly

 * libsecret uses that attribute to store the 'schema' describing
   the other attributes.
 * The old way of having a special 'Type' dbus argument, or
   CKA_G_SCHEMA pkcs#11 attribute is deprecated ... to be more
   inline with the Secret Service spec.

5 files changed, 136 insertions, 44 deletions

diff --git a/pkcs11/secret-store/gkm-secret-fields.c b/pkcs11/secret-store/gkm-secret-fields.c
index 279f2165..8145b895 100644
--- a/pkcs11/secret-store/gkm-secret-fields.c
+++ b/pkcs11/secret-store/gkm-secret-fields.c
@@ -145,7 +145,9 @@ gkm_secret_fields_new (void)
 }
 
 CK_RV
-gkm_secret_fields_parse (CK_ATTRIBUTE_PTR attr, GHashTable **fields)
+gkm_secret_fields_parse (CK_ATTRIBUTE_PTR attr,
+                         GHashTable **fields,
+                         gchar **schema_name)
 {
 	GHashTable *result;
 	const gchar *name;
@@ -201,48 +203,62 @@ gkm_secret_fields_parse (CK_ATTRIBUTE_PTR attr, GHashTable **fields)
 		g_hash_table_replace (result, g_strndup (name, n_name), g_strndup (value, n_value));
 	}
 
+	if (schema_name)
+		*schema_name = g_strdup (g_hash_table_lookup (result, "xdg:schema"));
+
 	*fields = result;
 	return CKR_OK;
 }
 
-static void
-each_field_append (gpointer key, gpointer value, gpointer user_data)
-{
-	GString *result = user_data;
-	g_string_append (result, key);
-	g_string_append_c (result, '\0');
-	g_string_append (result, value);
-	g_string_append_c (result, '\0');
-}
-
-static void
-each_field_length (gpointer key, gpointer value, gpointer user_data)
-{
-	gsize *length = user_data;
-	*length += strlen (key);
-	*length += strlen (value);
-	*length += 2;
-}
-
 CK_RV
-gkm_secret_fields_serialize (CK_ATTRIBUTE_PTR attr, GHashTable *fields)
+gkm_secret_fields_serialize (CK_ATTRIBUTE_PTR attr,
+                             GHashTable *fields,
+                             const gchar *schema_name)
 {
+	GHashTableIter iter;
+	gboolean saw_schema;
+	gpointer key;
+	gpointer value;
 	GString *result;
-	gsize length;
 	CK_RV rv;
 
-	g_assert (attr);
-	g_assert (fields);
+	g_assert (attr != NULL);
+	g_assert (fields != NULL);
 
 	if (!attr->pValue) {
-		length = 0;
-		g_hash_table_foreach (fields, each_field_length, &length);
-		attr->ulValueLen = length;
+		attr->ulValueLen = 0;
+		g_hash_table_iter_init (&iter, fields);
+		while (g_hash_table_iter_next (&iter, &key, &value)) {
+			if (g_str_equal (key, "xdg:schema"))
+				saw_schema = TRUE;
+			attr->ulValueLen += strlen (key);
+			attr->ulValueLen += strlen (value);
+			attr->ulValueLen += 2;
+		}
+		if (schema_name && !saw_schema) {
+			attr->ulValueLen += strlen ("xdg:schema");
+			attr->ulValueLen += strlen (schema_name);
+			attr->ulValueLen += 2;
+		}
 		return CKR_OK;
 	}
 
 	result = g_string_sized_new (256);
-	g_hash_table_foreach (fields, each_field_append, result);
+	g_hash_table_iter_init (&iter, fields);
+	while (g_hash_table_iter_next (&iter, &key, &value)) {
+		if (g_str_equal (key, "xdg:schema"))
+			saw_schema = TRUE;
+		g_string_append (result, key);
+		g_string_append_c (result, '\0');
+		g_string_append (result, value);
+		g_string_append_c (result, '\0');
+	}
+	if (schema_name && !saw_schema) {
+		g_string_append (result, "xdg:schema");
+		g_string_append_c (result, '\0');
+		g_string_append (result, schema_name);
+		g_string_append_c (result, '\0');
+	}
 
 	rv = gkm_attribute_set_data (attr, result->str, result->len);
 	g_string_free (result, TRUE);
diff --git a/pkcs11/secret-store/gkm-secret-fields.h b/pkcs11/secret-store/gkm-secret-fields.h
index aef79458..ed28f447 100644
--- a/pkcs11/secret-store/gkm-secret-fields.h
+++ b/pkcs11/secret-store/gkm-secret-fields.h
@@ -65,10 +65,12 @@ const gchar*    gkm_secret_fields_get                         (GHashTable *field
                                                                const gchar *name);
 
 CK_RV           gkm_secret_fields_parse                       (CK_ATTRIBUTE_PTR attr,
-                                                               GHashTable **fields);
+                                                               GHashTable **fields,
+                                                               gchar **schema_name);
 
 CK_RV           gkm_secret_fields_serialize                   (CK_ATTRIBUTE_PTR attr,
-                                                               GHashTable *fields);
+                                                               GHashTable *fields,
+                                                               const gchar *schema_name);
 
 gboolean        gkm_secret_fields_match                       (GHashTable *haystack,
                                                                GHashTable *needle);
diff --git a/pkcs11/secret-store/gkm-secret-item.c b/pkcs11/secret-store/gkm-secret-item.c
index d03c4a83..35698da1 100644
--- a/pkcs11/secret-store/gkm-secret-item.c
+++ b/pkcs11/secret-store/gkm-secret-item.c
@@ -235,7 +235,7 @@ gkm_secret_item_real_get_attribute (GkmObject *base, GkmSession *session, CK_ATT
 	case CKA_G_FIELDS:
 		if (!self->fields)
 			return gkm_attribute_set_data (attr, NULL, 0);
-		return gkm_secret_fields_serialize (attr, self->fields);
+		return gkm_secret_fields_serialize (attr, self->fields, self->schema);
 
 	case CKA_G_SCHEMA:
 		return gkm_attribute_set_string (attr, self->schema);
@@ -252,6 +252,7 @@ gkm_secret_item_real_set_attribute (GkmObject *base, GkmSession *session,
 	const gchar *identifier;
 	GkmSecretData *sdata;
 	GHashTable *fields;
+	gchar *schema_name;
 	GkmSecret *secret;
 	gchar *schema;
 	CK_RV rv;
@@ -281,11 +282,14 @@ gkm_secret_item_real_set_attribute (GkmObject *base, GkmSession *session,
 		return;
 
 	case CKA_G_FIELDS:
-		rv = gkm_secret_fields_parse (attr, &fields);
-		if (rv != CKR_OK)
+		rv = gkm_secret_fields_parse (attr, &fields, &schema_name);
+		if (rv != CKR_OK) {
 			gkm_transaction_fail (transaction, rv);
-		else
+		} else {
 			begin_set_fields (self, transaction, fields);
+			if (schema_name)
+				begin_set_schema (self, transaction, schema_name);
+		}
 		return;
 
 	case CKA_G_SCHEMA:
diff --git a/pkcs11/secret-store/gkm-secret-search.c b/pkcs11/secret-store/gkm-secret-search.c
index 02eb9e99..3f7a77d7 100644
--- a/pkcs11/secret-store/gkm-secret-search.c
+++ b/pkcs11/secret-store/gkm-secret-search.c
@@ -194,7 +194,7 @@ factory_create_search (GkmSession *session, GkmTransaction *transaction,
 	}
 
 	/* Parse the fields, into our internal representation */
-	rv = gkm_secret_fields_parse (attr, &fields);
+	rv = gkm_secret_fields_parse (attr, &fields, NULL);
 	gkm_attribute_consume (attr);
 	if (rv != CKR_OK) {
 		gkm_transaction_fail (transaction, rv);
@@ -307,7 +307,7 @@ gkm_secret_search_get_attribute (GkmObject *base, GkmSession *session, CK_ATTRIB
 			return gkm_attribute_set_empty (attr);
 		return gkm_attribute_set_string (attr, self->collection_id);
 	case CKA_G_FIELDS:
-		return gkm_secret_fields_serialize (attr, self->fields);
+		return gkm_secret_fields_serialize (attr, self->fields, NULL);
 	case CKA_G_MATCHED:
 		return attribute_set_handles (self->objects, attr);
 	}
diff --git a/pkcs11/secret-store/tests/test-secret-fields.c b/pkcs11/secret-store/tests/test-secret-fields.c
index 96bb6071..9e85eda3 100644
--- a/pkcs11/secret-store/tests/test-secret-fields.c
+++ b/pkcs11/secret-store/tests/test-secret-fields.c
@@ -74,11 +74,12 @@ static void
 test_parse (void)
 {
 	CK_ATTRIBUTE attr = { CKA_G_FIELDS, "one\0value1\0two\0value2\0three\0value3\0", 35 };
+	gchar *schema_name;
 	GHashTable *fields;
 	const gchar *value;
 	CK_RV rv;
 
-	rv = gkm_secret_fields_parse (&attr, &fields);
+	rv = gkm_secret_fields_parse (&attr, &fields, &schema_name);
 	g_assert (rv == CKR_OK);
 
 	g_assert_cmpuint (g_hash_table_size (fields), ==, 3);
@@ -89,6 +90,34 @@ test_parse (void)
 	value = g_hash_table_lookup (fields, "three");
 	g_assert_cmpstr (value, ==, "value3");
 
+	g_assert (schema_name == NULL);
+
+	g_hash_table_unref (fields);
+}
+
+static void
+test_parse_schema (void)
+{
+	CK_ATTRIBUTE attr = { CKA_G_FIELDS, "one\0value1\0two\0valu\0xdg:schema\0xxx\0", 35 };
+	gchar *schema_name;
+	GHashTable *fields;
+	const gchar *value;
+	CK_RV rv;
+
+	rv = gkm_secret_fields_parse (&attr, &fields, &schema_name);
+	g_assert (rv == CKR_OK);
+
+	g_assert_cmpuint (g_hash_table_size (fields), ==, 3);
+	value = g_hash_table_lookup (fields, "one");
+	g_assert_cmpstr (value, ==, "value1");
+	value = g_hash_table_lookup (fields, "two");
+	g_assert_cmpstr (value, ==, "valu");
+	value = g_hash_table_lookup (fields, "xdg:schema");
+	g_assert_cmpstr (value, ==, "xxx");
+
+	g_assert_cmpstr (schema_name, ==, "xxx");
+
+	g_free (schema_name);
 	g_hash_table_unref (fields);
 }
 
@@ -99,7 +128,7 @@ test_parse_empty (void)
 	GHashTable *fields;
 	CK_RV rv;
 
-	rv = gkm_secret_fields_parse (&attr, &fields);
+	rv = gkm_secret_fields_parse (&attr, &fields, NULL);
 	g_assert (rv == CKR_OK);
 
 	g_assert_cmpuint (g_hash_table_size (fields), == , 0);
@@ -114,7 +143,7 @@ test_parse_null_invalid (void)
 	GHashTable *fields;
 	CK_RV rv;
 
-	rv = gkm_secret_fields_parse (&attr, &fields);
+	rv = gkm_secret_fields_parse (&attr, &fields, NULL);
 	g_assert (rv == CKR_ATTRIBUTE_VALUE_INVALID);
 }
 
@@ -125,7 +154,7 @@ test_parse_missing_value (void)
 	GHashTable *fields;
 	CK_RV rv;
 
-	rv = gkm_secret_fields_parse (&attr, &fields);
+	rv = gkm_secret_fields_parse (&attr, &fields, NULL);
 	g_assert (rv == CKR_ATTRIBUTE_VALUE_INVALID);
 }
 
@@ -136,7 +165,7 @@ test_parse_missing_terminator (void)
 	GHashTable *fields;
 	CK_RV rv;
 
-	rv = gkm_secret_fields_parse (&attr, &fields);
+	rv = gkm_secret_fields_parse (&attr, &fields, NULL);
 	g_assert (rv == CKR_ATTRIBUTE_VALUE_INVALID);
 }
 
@@ -147,7 +176,7 @@ test_parse_not_utf8 (void)
 	GHashTable *fields;
 	CK_RV rv;
 
-	rv = gkm_secret_fields_parse (&attr, &fields);
+	rv = gkm_secret_fields_parse (&attr, &fields, NULL);
 	g_assert (rv == CKR_ATTRIBUTE_VALUE_INVALID);
 }
 
@@ -162,7 +191,7 @@ test_serialize (void)
 	fields = gkm_secret_fields_new ();
 	gkm_secret_fields_add (fields, "one", "value1");
 
-	rv = gkm_secret_fields_serialize (&attr, fields);
+	rv = gkm_secret_fields_serialize (&attr, fields, NULL);
 	g_assert (rv == CKR_OK);
 	g_assert (attr.ulValueLen == 11);
 	g_assert (memcmp (buffer, "one\0value1\0", 11) == 0);
@@ -171,6 +200,44 @@ test_serialize (void)
 }
 
 static void
+test_serialize_schema (void)
+{
+	gchar buffer[32];
+	CK_ATTRIBUTE attr = { CKA_G_FIELDS, buffer, 32 };
+	GHashTable *fields;
+	CK_RV rv;
+
+	fields = gkm_secret_fields_new ();
+	gkm_secret_fields_add (fields, "one", "value1");
+
+	rv = gkm_secret_fields_serialize (&attr, fields, "xxx");
+	g_assert (rv == CKR_OK);
+	g_assert_cmpint (attr.ulValueLen, ==, 26);
+	g_assert (memcmp (buffer, "one\0value1\0xdg:schema\0xxx\0", 26) == 0);
+
+	g_hash_table_unref (fields);
+}
+
+static void
+test_serialize_schema_already (void)
+{
+	gchar buffer[32];
+	CK_ATTRIBUTE attr = { CKA_G_FIELDS, buffer, 32 };
+	GHashTable *fields;
+	CK_RV rv;
+
+	fields = gkm_secret_fields_new ();
+	gkm_secret_fields_add (fields, "xdg:schema", "yyy");
+
+	rv = gkm_secret_fields_serialize (&attr, fields, "xxx");
+	g_assert (rv == CKR_OK);
+	g_assert (attr.ulValueLen == 15);
+	g_assert (memcmp (buffer, "xdg:schema\0yyy\0", 15) == 0);
+
+	g_hash_table_unref (fields);
+}
+
+static void
 test_serialize_length (void)
 {
 	CK_ATTRIBUTE attr = { CKA_G_FIELDS, NULL, 0 };
@@ -180,7 +247,7 @@ test_serialize_length (void)
 	fields = gkm_secret_fields_new ();
 	gkm_secret_fields_add (fields, "one", "value1");
 
-	rv = gkm_secret_fields_serialize (&attr, fields);
+	rv = gkm_secret_fields_serialize (&attr, fields, NULL);
 	g_assert (rv == CKR_OK);
 	g_assert (attr.ulValueLen == 11);
 
@@ -416,12 +483,15 @@ main (int argc, char **argv)
 	g_test_add_func ("/secret-store/fields/boxed", test_boxed);
 	g_test_add_func ("/secret-store/fields/add_get_values", test_add_get_values);
 	g_test_add_func ("/secret-store/fields/parse", test_parse);
+	g_test_add_func ("/secret-store/fields/parse_schema", test_parse_schema);
 	g_test_add_func ("/secret-store/fields/parse_empty", test_parse_empty);
 	g_test_add_func ("/secret-store/fields/parse_null_invalid", test_parse_null_invalid);
 	g_test_add_func ("/secret-store/fields/parse_missing_value", test_parse_missing_value);
 	g_test_add_func ("/secret-store/fields/parse_missing_terminator", test_parse_missing_terminator);
 	g_test_add_func ("/secret-store/fields/parse_not_utf8", test_parse_not_utf8);
 	g_test_add_func ("/secret-store/fields/serialize", test_serialize);
+	g_test_add_func ("/secret-store/fields/serialize_schema", test_serialize_schema);
+	g_test_add_func ("/secret-store/fields/serialize_schema_already", test_serialize_schema_already);
 	g_test_add_func ("/secret-store/fields/serialize_length", test_serialize_length);
 	g_test_add_func ("/secret-store/fields/add_get_compat_uint32", test_add_get_compat_uint32);
 	g_test_add_func ("/secret-store/fields/get_compat_uint32_fail", test_get_compat_uint32_fail);