Re: [opensc-devel] [PATCH] Fix OpenSC PKCS#11 object grouping

2009-10-06 Thread Aktiv Co. Aleksey Samsonov
Hello,

Pierre Ossman:
> On Mon, 5 Oct 2009 11:28:12 +0300
> Martin Paljak  wrote:
> 
>> On 05.10.2009, at 11:01, Pierre Ossman wrote:
>>> New attempt, this time against r3756 (r18006 was our internal repo,
>>> for
>>> those curious :)), as an attachment and without a signature on the
>>> mail. Hopefully everyone can read it this time.
>> Applies and works for me.
>>
> 
> Glad to hear it. Does that also mean it will get merged in trunk?
> 
>>> Oh yeah, I also forgot to mention that this patch also adds some more
>>> debug output. I found it helpful to see how the library chooses to
>>> associate objects, even though it currently only prints the index
>>> number.
>> Maybe you can improve it so that it would log object/auth IDs? This
>> would facilitate better debugging by looking at pkcs15-tool -D and
>> then pkcs#11 debug log?
>>
> 
> Sure. Included patch gives this debug output:

Thanks!
Committed in revision 3769.

___
opensc-devel mailing list
opensc-devel@lists.opensc-project.org
http://www.opensc-project.org/mailman/listinfo/opensc-devel


Re: [opensc-devel] [PATCH] Fix OpenSC PKCS#11 object grouping

2009-10-05 Thread Pierre Ossman
On Mon, 5 Oct 2009 11:28:12 +0300
Martin Paljak  wrote:

> 
> On 05.10.2009, at 11:01, Pierre Ossman wrote:
> > New attempt, this time against r3756 (r18006 was our internal repo,  
> > for
> > those curious :)), as an attachment and without a signature on the
> > mail. Hopefully everyone can read it this time.
> Applies and works for me.
> 

Glad to hear it. Does that also mean it will get merged in trunk?

> 
> > Oh yeah, I also forgot to mention that this patch also adds some more
> > debug output. I found it helpful to see how the library chooses to
> > associate objects, even though it currently only prints the index
> > number.
> Maybe you can improve it so that it would log object/auth IDs? This  
> would facilitate better debugging by looking at pkcs15-tool -D and  
> then pkcs#11 debug log?
> 

Sure. Included patch gives this debug output:

[opensc-pkcs11] framework-pkcs15.c:559:pkcs15_bind_related_objects: Looking for 
objects related to object 0
[opensc-pkcs11] framework-pkcs15.c:467:__pkcs15_prkey_bind_related: Object is a 
private key and has id 45
[opensc-pkcs11] framework-pkcs15.c:559:pkcs15_bind_related_objects: Looking for 
objects related to object 1
[opensc-pkcs11] framework-pkcs15.c:467:__pkcs15_prkey_bind_related: Object is a 
private key and has id 46
[opensc-pkcs11] framework-pkcs15.c:559:pkcs15_bind_related_objects: Looking for 
objects related to object 2
[opensc-pkcs11] framework-pkcs15.c:509:__pkcs15_cert_bind_related: Object is a 
certificate and has id 45
[opensc-pkcs11] framework-pkcs15.c:538:__pkcs15_cert_bind_related: Associating 
object 0 as private key
[opensc-pkcs11] framework-pkcs15.c:559:pkcs15_bind_related_objects: Looking for 
objects related to object 3
[opensc-pkcs11] framework-pkcs15.c:559:pkcs15_bind_related_objects: Looking for 
objects related to object 4
[opensc-pkcs11] framework-pkcs15.c:509:__pkcs15_cert_bind_related: Object is a 
certificate and has id 46
[opensc-pkcs11] framework-pkcs15.c:538:__pkcs15_cert_bind_related: Associating 
object 1 as private key
[opensc-pkcs11] framework-pkcs15.c:559:pkcs15_bind_related_objects: Looking for 
objects related to object 5
[opensc-pkcs11] framework-pkcs15.c:559:pkcs15_bind_related_objects: Looking for 
objects related to object 6
[opensc-pkcs11] framework-pkcs15.c:509:__pkcs15_cert_bind_related: Object is a 
certificate and has id 45
[opensc-pkcs11] framework-pkcs15.c:538:__pkcs15_cert_bind_related: Associating 
object 0 as private key
[opensc-pkcs11] framework-pkcs15.c:528:__pkcs15_cert_bind_related: Associating 
object 10 (id 47) as issuer
[opensc-pkcs11] framework-pkcs15.c:559:pkcs15_bind_related_objects: Looking for 
objects related to object 7
[opensc-pkcs11] framework-pkcs15.c:559:pkcs15_bind_related_objects: Looking for 
objects related to object 8
[opensc-pkcs11] framework-pkcs15.c:509:__pkcs15_cert_bind_related: Object is a 
certificate and has id 46
[opensc-pkcs11] framework-pkcs15.c:538:__pkcs15_cert_bind_related: Associating 
object 1 as private key
[opensc-pkcs11] framework-pkcs15.c:528:__pkcs15_cert_bind_related: Associating 
object 10 (id 47) as issuer
[opensc-pkcs11] framework-pkcs15.c:559:pkcs15_bind_related_objects: Looking for 
objects related to object 9
[opensc-pkcs11] framework-pkcs15.c:559:pkcs15_bind_related_objects: Looking for 
objects related to object 10
[opensc-pkcs11] framework-pkcs15.c:509:__pkcs15_cert_bind_related: Object is a 
certificate and has id 47
[opensc-pkcs11] framework-pkcs15.c:559:pkcs15_bind_related_objects: Looking for 
objects related to object 11

Note that several certificates share id, which means that id isn't
enough to distinguish them (which also means that the opensc tools are
unable to properly use this card since they use ids heavily :/).

Rgds
-- 
Pierre OssmanOpenSource-based Thin Client Technology
System Developer Telephone: +46-13-21 46 00
Cendio ABWeb: http://www.cendio.com
Index: src/pkcs11/framework-pkcs15.c
===
--- src/pkcs11/framework-pkcs15.c	(revision 18042)
+++ src/pkcs11/framework-pkcs15.c	(working copy)
@@ -463,6 +463,9 @@
 	sc_pkcs15_id_t *id = &pk->prv_info->id;
 	unsigned int i;
 
+	sc_debug(context, "Object is a private key and has id %s",
+	 sc_pkcs15_print_id(id));
+
 	for (i = 0; i < fw_data->num_objects; i++) {
 		struct pkcs15_any_object *obj = fw_data->objects[i];
 
@@ -502,21 +505,27 @@
 	sc_pkcs15_id_t *id = &cert->cert_info->id;
 	unsigned int i;
 
+	sc_debug(context, "Object is a certificate and has id %s",
+	 sc_pkcs15_print_id(id));
+
 	/* Loop over all objects to see if we find the certificate of
 	 * the issuer and the associated private key */
 	for (i = 0; i < fw_data->num_objects; i++) {
 		struct pkcs15_any_object *obj = fw_data->objects[i];
 
 		if (is_cert(obj) && obj != (struct pkcs15_any_object *) cert) {
+			struct pkcs15_cert_object *cert2;
 			struct sc_pkcs15_cert *c2;
 
-			c2 = ((struct pkcs15_cert_obje

Re: [opensc-devel] [PATCH] Fix OpenSC PKCS#11 object grouping

2009-10-05 Thread Martin Paljak

On 05.10.2009, at 11:01, Pierre Ossman wrote:
> New attempt, this time against r3756 (r18006 was our internal repo,  
> for
> those curious :)), as an attachment and without a signature on the
> mail. Hopefully everyone can read it this time.
Applies and works for me.


> Oh yeah, I also forgot to mention that this patch also adds some more
> debug output. I found it helpful to see how the library chooses to
> associate objects, even though it currently only prints the index
> number.
Maybe you can improve it so that it would log object/auth IDs? This  
would facilitate better debugging by looking at pkcs15-tool -D and  
then pkcs#11 debug log?


Thanks,
-- 
Martin Paljak
http://martin.paljak.pri.ee
+372.515.6495




___
opensc-devel mailing list
opensc-devel@lists.opensc-project.org
http://www.opensc-project.org/mailman/listinfo/opensc-devel


Re: [opensc-devel] [PATCH] Fix OpenSC PKCS#11 object grouping

2009-10-05 Thread Pierre Ossman
On Sat, 3 Oct 2009 11:00:16 +0300
Martin Paljak  wrote:

> Hi,
> 
> I can't use it:
> 
> $ patch -p0 < ../pkcs11.diff
> patching file src/pkcs11/framework-pkcs15.c
> patch:  malformed patch at line 10: struct pkcs15_prkey_object {
> 
> Please provide a new patch against trunk (which is not @ r18006) as an  
> attachment.
> 

New attempt, this time against r3756 (r18006 was our internal repo, for
those curious :)), as an attachment and without a signature on the
mail. Hopefully everyone can read it this time.

Oh yeah, I also forgot to mention that this patch also adds some more
debug output. I found it helpful to see how the library chooses to
associate objects, even though it currently only prints the index
number.

Rgds
-- 
Pierre OssmanOpenSource-based Thin Client Technology
System Developer Telephone: +46-13-21 46 00
Cendio ABWeb: http://www.cendio.com
Index: src/pkcs11/framework-pkcs15.c
===
--- src/pkcs11/framework-pkcs15.c	(revision 3756)
+++ src/pkcs11/framework-pkcs15.c	(working copy)
@@ -82,6 +82,7 @@
 #define cert_p15obj		base.p15_object
 #define cert_pubkey		base.related_pubkey
 #define cert_issuer		base.related_cert
+#define cert_prvkey		base.related_privkey
 
 struct pkcs15_prkey_object {
 	struct pkcs15_any_object	base;
@@ -91,7 +92,6 @@
 #define prv_flags		base.base.flags
 #define prv_p15obj		base.p15_object
 #define prv_pubkey		base.related_pubkey
-#define prv_cert		base.related_cert
 #define prv_next		base.related_privkey
 
 struct pkcs15_pubkey_object {
@@ -102,7 +102,7 @@
 };
 #define pub_flags		base.base.flags
 #define pub_p15obj		base.p15_object
-#define pub_cert		base.related_cert
+#define pub_genfrom		base.related_cert
 
 #define __p15_type(obj)		(((obj) && (obj)->p15_object)? ((obj)->p15_object->type) : (unsigned int)-1)
 #define is_privkey(obj)		(__p15_type(obj) == SC_PKCS15_TYPE_PRKEY_RSA)
@@ -346,7 +346,7 @@
 	} else
 		obj2->pub_data = NULL; /* will copy from cert when cert is read */
 
-	obj2->pub_cert = object;
+	obj2->pub_genfrom = object;
 	object->cert_pubkey = obj2;
 
 	if (cert_object != NULL)
@@ -484,18 +484,12 @@
 *pp = (struct pkcs15_prkey_object *) obj;
 			}
 		} else
-		if (is_cert(obj) && !pk->prv_cert) {
-			struct pkcs15_cert_object *cert;
-			
-			cert = (struct pkcs15_cert_object *) obj;
-			if (sc_pkcs15_compare_id(&cert->cert_info->id, id))
-pk->prv_cert = cert;
-		} else
 		if (is_pubkey(obj) && !pk->prv_pubkey) {
 			struct pkcs15_pubkey_object *pubkey;
 			
 			pubkey = (struct pkcs15_pubkey_object *) obj;
 			if (sc_pkcs15_compare_id(&pubkey->pub_info->id, id)) {
+sc_debug(context, "Associating object %d as public key", i);
 pk->prv_pubkey = pubkey;
 if (pk->prv_info->modulus_length == 0)
 	pk->prv_info->modulus_length = pubkey->pub_info->modulus_length;
@@ -507,25 +501,37 @@
 static void
 __pkcs15_cert_bind_related(struct pkcs15_fw_data *fw_data, struct pkcs15_cert_object *cert)
 {
-	struct sc_pkcs15_cert *c1 = cert->cert_data, *c2;
+	struct sc_pkcs15_cert *c1 = cert->cert_data;
+	sc_pkcs15_id_t *id = &cert->cert_info->id;
 	unsigned int i;
 
-	/* Loop over all certificates see if we find the certificate of
-	 * the issuer */
+	/* Loop over all objects to see if we find the certificate of
+	 * the issuer and the associated private key */
 	for (i = 0; i < fw_data->num_objects; i++) {
 		struct pkcs15_any_object *obj = fw_data->objects[i];
 
-		if (!is_cert(obj) || obj == (struct pkcs15_any_object *) cert)
-			continue;
+		if (is_cert(obj) && obj != (struct pkcs15_any_object *) cert) {
+			struct sc_pkcs15_cert *c2;
 
-		c2 = ((struct pkcs15_cert_object *) obj)->cert_data;
+			c2 = ((struct pkcs15_cert_object *) obj)->cert_data;
 
-		if (!c1 || !c2 || !c1->issuer_len || !c2->subject_len)
-			continue;
-		if (c1->issuer_len == c2->subject_len
-		 && !memcmp(c1->issuer, c2->subject, c1->issuer_len)) {
-			cert->cert_issuer = (struct pkcs15_cert_object *) obj;
-			return;
+			if (!c1 || !c2 || !c1->issuer_len || !c2->subject_len)
+continue;
+			if (c1->issuer_len == c2->subject_len
+			 && !memcmp(c1->issuer, c2->subject, c1->issuer_len)) {
+sc_debug(context, "Associating object %d as issuer", i);
+cert->cert_issuer = (struct pkcs15_cert_object *) obj;
+return;
+			}
+		} else
+		if (is_privkey(obj) && !cert->cert_prvkey) {
+			struct pkcs15_prkey_object *pk;
+			
+			pk = (struct pkcs15_prkey_object *) obj;
+			if (sc_pkcs15_compare_id(&pk->prv_info->id, id)) {
+sc_debug(context, "Associating object %d as private key", i);
+cert->cert_prvkey = pk;
+			}
 		}
 	}
 }
@@ -543,6 +549,9 @@
 
 		if (obj->base.flags & SC_PKCS11_OBJECT_HIDDEN)
 			continue;
+
+		sc_debug(context, "Looking for objects related to object %d", i);
+
 		if (is_privkey(obj)) {
 			__pkcs15_prkey_bind_related(fw_data, (struct pkcs15_prkey_object *) obj);
 		} else if (is_cert(obj)) {
@@ -609,6 +618,9 @@
 		  struct pkcs15_any_object *obj,

Re: [opensc-devel] [PATCH] Fix OpenSC PKCS#11 object grouping

2009-10-03 Thread Martin Paljak
Hi,

I can't use it:

$ patch -p0 < ../pkcs11.diff
patching file src/pkcs11/framework-pkcs15.c
patch:  malformed patch at line 10: struct pkcs15_prkey_object {

Please provide a new patch against trunk (which is not @ r18006) as an  
attachment.


Thanks,
Martin

On 02.10.2009, at 16:29, Pierre Ossman wrote:

> ===
> --- src/pkcs11/framework-pkcs15.c (revision 18006)
> +++ src/pkcs11/framework-pkcs15.c (working copy)
> @@ -82,6 +82,7 @@
> #define cert_p15obj   base.p15_object
> #define cert_pubkey   base.related_pubkey
> #define cert_issuer   base.related_cert
> +#define cert_prvkey  base.related_privkey
>
> struct pkcs15_prkey_object {
>   struct pkcs15_any_objectbase;
> @@ -91,7 +92,6 @@
> #define prv_flags base.base.flags
> #define prv_p15objbase.p15_object
> #define prv_pubkeybase.related_pubkey
> -#define prv_cert base.related_cert
> #define prv_next  base.related_privkey
>
> struct pkcs15_pubkey_object {
> @@ -102,7 +102,7 @@
> };
> #define pub_flags base.base.flags
> #define pub_p15objbase.p15_object
> -#define pub_cert base.related_cert
> +#define pub_genfrom  base.related_cert
>
> #define __p15_type(obj)   (((obj) && (obj)->p15_object)? ((obj)- 
> >p15_object->type) : (unsigned int)-1)
> #define is_privkey(obj)   (__p15_type(obj) == 
> SC_PKCS15_TYPE_PRKEY_RSA)
> @@ -343,7 +343,7 @@
>   } else
>   obj2->pub_data = NULL; /* will copy from cert when cert is read 
> */
>
> - obj2->pub_cert = object;
> + obj2->pub_genfrom = object;
>   object->cert_pubkey = obj2;
>
>   if (cert_object != NULL)
> @@ -481,18 +481,12 @@
>   *pp = (struct pkcs15_prkey_object *) obj;
>   }
>   } else
> - if (is_cert(obj) && !pk->prv_cert) {
> - struct pkcs15_cert_object *cert;
> - 
> - cert = (struct pkcs15_cert_object *) obj;
> - if (sc_pkcs15_compare_id(&cert->cert_info->id, id))
> - pk->prv_cert = cert;
> - } else
>   if (is_pubkey(obj) && !pk->prv_pubkey) {
>   struct pkcs15_pubkey_object *pubkey;
>   
>   pubkey = (struct pkcs15_pubkey_object *) obj;
>   if (sc_pkcs15_compare_id(&pubkey->pub_info->id, id)) {
> + sc_debug(context, "Associating object %d as 
> public key", i);
>   pk->prv_pubkey = pubkey;
>   if (pk->prv_info->modulus_length == 0)
>   pk->prv_info->modulus_length = 
> pubkey->pub_info->modulus_length;
> @@ -504,25 +498,37 @@
> static void
> __pkcs15_cert_bind_related(struct pkcs15_fw_data *fw_data, struct  
> pkcs15_cert_object *cert)
> {
> - struct sc_pkcs15_cert *c1 = cert->cert_data, *c2;
> + struct sc_pkcs15_cert *c1 = cert->cert_data;
> + sc_pkcs15_id_t *id = &cert->cert_info->id;
>   unsigned int i;
>
> - /* Loop over all certificates see if we find the certificate of
> -  * the issuer */
> + /* Loop over all objects to see if we find the certificate of
> +  * the issuer and the associated private key */
>   for (i = 0; i < fw_data->num_objects; i++) {
>   struct pkcs15_any_object *obj = fw_data->objects[i];
>
> - if (!is_cert(obj) || obj == (struct pkcs15_any_object *) cert)
> - continue;
> + if (is_cert(obj) && obj != (struct pkcs15_any_object *) cert) {
> + struct sc_pkcs15_cert *c2;
>
> - c2 = ((struct pkcs15_cert_object *) obj)->cert_data;
> + c2 = ((struct pkcs15_cert_object *) obj)->cert_data;
>
> - if (!c1 || !c2 || !c1->issuer_len || !c2->subject_len)
> - continue;
> - if (c1->issuer_len == c2->subject_len
> -  && !memcmp(c1->issuer, c2->subject, c1->issuer_len)) {
> - cert->cert_issuer = (struct pkcs15_cert_object *) obj;
> - return;
> + if (!c1 || !c2 || !c1->issuer_len || !c2->subject_len)
> + continue;
> + if (c1->issuer_len == c2->subject_len
> +  && !memcmp(c1->issuer, c2->subject, c1->issuer_len)) {
> + sc_debug(context, "Associating object %d as 
> issuer", i);
> + cert->cert_issuer = (struct pkcs15_cert_object 
> *) obj;
> + return;
> + }
> + } else
> + if (is_privkey(obj) && !cert->cert_prvkey) {
> + struct pkcs15_prkey_object *pk;
> +  

[opensc-devel] [PATCH] Fix OpenSC PKCS#11 object grouping

2009-10-02 Thread Pierre Ossman
The logic in the current PKCS#11 library could not handle more advanced
cards where it would incorrectly group objects into slots. This patch
fixes two such issues:

 - A key pair can be used for multiple certificates, but a certificate
   can never use more than one key pair. Unfortunately the code was
   designed for the completely opposite scenario (most likely out of
   convenience). I.e. the code would only put the first certificate it
   found together with its private key. Any later certificates with the
   same key would end up in the wrong slot.

 - Some PKCS#15 object have an auth_id even though they are not
   private. The current code always uses the auth_id though, which can
   result in incorrect grouping.

The code removes the connection priv_key->cert and sets up
cert->priv_key instead. The pkcs15_prkey_get_attribute() routine needs
access to one of the associated certs though, but the new code iterates
rather than maintaining the priv_key->cert link as keeping the link
could trick people into reintroducing some of the buggy behaviour.
---

===
--- src/pkcs11/framework-pkcs15.c   (revision 18006)
+++ src/pkcs11/framework-pkcs15.c   (working copy)
@@ -82,6 +82,7 @@
 #define cert_p15objbase.p15_object
 #define cert_pubkeybase.related_pubkey
 #define cert_issuerbase.related_cert
+#define cert_prvkeybase.related_privkey
 
 struct pkcs15_prkey_object {
struct pkcs15_any_objectbase;
@@ -91,7 +92,6 @@
 #define prv_flags  base.base.flags
 #define prv_p15obj base.p15_object
 #define prv_pubkey base.related_pubkey
-#define prv_cert   base.related_cert
 #define prv_next   base.related_privkey
 
 struct pkcs15_pubkey_object {
@@ -102,7 +102,7 @@
 };
 #define pub_flags  base.base.flags
 #define pub_p15obj base.p15_object
-#define pub_cert   base.related_cert
+#define pub_genfrombase.related_cert
 
 #define __p15_type(obj)(((obj) && (obj)->p15_object)? 
((obj)->p15_object->type) : (unsigned int)-1)
 #define is_privkey(obj)(__p15_type(obj) == 
SC_PKCS15_TYPE_PRKEY_RSA)
@@ -343,7 +343,7 @@
} else
obj2->pub_data = NULL; /* will copy from cert when cert is read 
*/
 
-   obj2->pub_cert = object;
+   obj2->pub_genfrom = object;
object->cert_pubkey = obj2;
 
if (cert_object != NULL)
@@ -481,18 +481,12 @@
*pp = (struct pkcs15_prkey_object *) obj;
}
} else
-   if (is_cert(obj) && !pk->prv_cert) {
-   struct pkcs15_cert_object *cert;
-   
-   cert = (struct pkcs15_cert_object *) obj;
-   if (sc_pkcs15_compare_id(&cert->cert_info->id, id))
-   pk->prv_cert = cert;
-   } else
if (is_pubkey(obj) && !pk->prv_pubkey) {
struct pkcs15_pubkey_object *pubkey;

pubkey = (struct pkcs15_pubkey_object *) obj;
if (sc_pkcs15_compare_id(&pubkey->pub_info->id, id)) {
+   sc_debug(context, "Associating object %d as 
public key", i);
pk->prv_pubkey = pubkey;
if (pk->prv_info->modulus_length == 0)
pk->prv_info->modulus_length = 
pubkey->pub_info->modulus_length;
@@ -504,25 +498,37 @@
 static void
 __pkcs15_cert_bind_related(struct pkcs15_fw_data *fw_data, struct 
pkcs15_cert_object *cert)
 {
-   struct sc_pkcs15_cert *c1 = cert->cert_data, *c2;
+   struct sc_pkcs15_cert *c1 = cert->cert_data;
+   sc_pkcs15_id_t *id = &cert->cert_info->id;
unsigned int i;
 
-   /* Loop over all certificates see if we find the certificate of
-* the issuer */
+   /* Loop over all objects to see if we find the certificate of
+* the issuer and the associated private key */
for (i = 0; i < fw_data->num_objects; i++) {
struct pkcs15_any_object *obj = fw_data->objects[i];
 
-   if (!is_cert(obj) || obj == (struct pkcs15_any_object *) cert)
-   continue;
+   if (is_cert(obj) && obj != (struct pkcs15_any_object *) cert) {
+   struct sc_pkcs15_cert *c2;
 
-   c2 = ((struct pkcs15_cert_object *) obj)->cert_data;
+   c2 = ((struct pkcs15_cert_object *) obj)->cert_data;
 
-   if (!c1 || !c2 || !c1->issuer_len || !c2->subject_len)
-   continue;
-   if (c1->issuer_len == c2->subject_len
-&& !memcmp(c1->issuer, c2->subject, c1->issuer_len)) {
-   cert->cert_issuer = (st