Nils Larsch wrote:

Stef Hoeben wrote:

Hi,

there seem to be 2 problems:

- protect_certificates = false by default -> nasty security bug IMHO, this is fixed in profile.c


It's certainly a bug but I don't think this is a real security problem
(unless you can you describe a practical attack using this bug). If the
CDF isn't protected you can at most delete or replace [references to]
certificates (of course this can be a denial of service attack but it
shouldn't affect security schemes using certifcates + private keys).

Guess the security problem is that anyone can add a bogus root cert and
'register' it in the CDF. If an applications then trusts such a root cert, you
can no longer be sure who signed a mail, if it's realy the bank's web server
you're connected to... (unless you look deeper into it then most people do)
I don't know which apps effectively trust root certs marked trusted on a
smart card, but for the ones that do there is indeed a problem.

(BTW: the certs themselves are also left unprotected)

PS: The flex.profile sets this value to true, but setting it to false seems to work fine for me
        (so I propose to remove it -> OK???)


how many pins did you test ?

3, and with 1 key under each of them + corresponding cert chain.

But it seems the Delete ACL is set to NONE for the cryptoflex card, so it's still possible to delete the files and then create a new, bogus one -> this is another
security issue, IMHO
===> What to do with it??


- no reference to the user PIN is given with "pkcs15-init -X", which causes
 sc_pkcs15_init_fixup_file() to set the corresponding ACs to NONE (!)
It looks rather nasty but I'm afraid of shacking it up so I just added a reference
 to the first user PIN in case it's not set -> comments?


why not the so-pin (if present) ?

Hm, it's indeed not there (although it doesn't seem to be necessary..)
But adding a set_so_pin_from_card() doesn't seem to hurt, so perhaps
it's better to do so (?)

@@ -3408,6 +3431,51 @@
     return sc_pkcs15init_fixup_acls(profile, file, &so_acl, &user_acl);
 }
+static const char * acl_to_str(const sc_acl_entry_t *e)
+{


hmm, this function isn't used in your patch.

Oops, forgotten to remove that debug code...
Patch added again.

If it's OK like this, I'll commit it and wait a bit hoping for someone to test; and then
check 0.9.6 and 0.10.1 as well?

Cheers,
Stef

Index: profile.c
===================================================================
--- profile.c   (revision 2881)
+++ profile.c   (working copy)
@@ -255,6 +255,7 @@
                return NULL;
        pro->p15_spec = p15card = sc_pkcs15_card_new();
 
+       pro->protect_certificates = 1;
        pro->pkcs15.do_last_update = 1;
 
        if (p15card) {
Index: pkcs15-lib.c
===================================================================
--- pkcs15-lib.c        (revision 2881)
+++ pkcs15-lib.c        (working copy)
@@ -1675,25 +1675,33 @@
        if ((label = args->label) == NULL)
                label = "Certificate";
 
+       /* Set the SO PIN reference from card */
+       if ((r = set_so_pin_from_card(p15card, profile)) < 0)
+               return r;
+
        /* Select an ID if the user didn't specify one, otherwise
         * make sure it's unique */
        if ((r = select_id(p15card, SC_PKCS15_TYPE_CERT, &args->id, NULL, NULL, 
NULL)) < 0)
                return r;
 
-       /* If there is a private key corresponding to the ID given
-        * by the user, make sure $PIN references the pin protecting
-        * this key
-        */
-       if (args->id.len != 0
-        && profile->protect_certificates
-        && sc_pkcs15_find_prkey_by_id(p15card, &args->id, &object) == 0) {
-               r = set_user_pin_from_authid(p15card, profile, 
&object->auth_id);
-               if (r < 0) {
-                       sc_error(p15card->card->ctx,
-                                     "Failed to assign user pin reference "
-                                     "(copied from private key auth_id)\n");
-                       return r;
+       if (profile->protect_certificates) {
+               /* If there is a private key corresponding to the ID given
+                * by the user, make sure $PIN references the pin protecting
+                * this key
+                */
+               r = -1;
+               if (args->id.len != 0
+                && sc_pkcs15_find_prkey_by_id(p15card, &args->id, &object) == 
0) {
+                       r = set_user_pin_from_authid(p15card, profile, 
&object->auth_id);
+                       if (r < 0) {
+                               sc_error(p15card->card->ctx,
+                                             "Failed to assign user pin 
reference "
+                                             "(copied from private key 
auth_id)\n");
+                               return r;
+                       }
                }
+               if (r == -1) /* User pin ref not yet set */
+                       set_user_pin_from_authid(p15card, profile, NULL);
        }
 
        object = sc_pkcs15init_new_object(SC_PKCS15_TYPE_CERT_X509, label, 
NULL, NULL);
@@ -3098,7 +3106,9 @@
 
 /*
  * If the user specified an auth_id, select the corresponding
- * PIN entry and set the reference data
+ * PIN entry and set the reference data.
+ * If auth_id is NULL, then get the first user PIN found, this
+ * is usefull for the 'onepin' profile option.
  */
 static int
 set_user_pin_from_authid(struct sc_pkcs15_card *p15card,
@@ -3109,6 +3119,23 @@
        struct sc_pkcs15_object *objp;
        int             r;
 
+       if (auth_id == NULL) {
+               int i;
+               struct sc_pkcs15_object *p15objects[5];
+               r = sc_pkcs15_get_objects(p15card, SC_PKCS15_TYPE_AUTH_PIN, 
p15objects, 5);
+               if (r < 0)
+                       return r;
+               for (i = 0; i < r; i++) {
+                       sc_pkcs15_pin_info_t *pininfo = (sc_pkcs15_pin_info_t 
*) p15objects[i]->data;
+                       if (!(pininfo->flags & SC_PKCS15_PIN_FLAG_SO_PIN)) {
+                               auth_id = &pininfo->auth_id;
+                               break;
+                       }
+               }
+               if (i >= r)
+                       return SC_ERROR_OBJECT_NOT_FOUND;
+       }
+
        if (auth_id->len == 0)
                return 0;
 
Index: flex.profile
===================================================================
--- flex.profile        (revision 2881)
+++ flex.profile        (working copy)
@@ -6,12 +6,6 @@
     pin-encoding       = ascii-numeric;
     pin-pad-char       = 0x00;
     pin-domains                = yes;
-
-    # This profile does not PIN-protect certificates
-    # stored on the card. If you enable this, you MUST
-    # adjust the sizes of the pin-domain and key-dir DFs
-    # accordingly.
-    protect-certificates = no;
 }
 
 # Define reasonable limits for PINs and PUK
_______________________________________________
opensc-devel mailing list
opensc-devel@lists.opensc-project.org
http://www.opensc-project.org/mailman/listinfo/opensc-devel

Reply via email to