Page MenuHome GnuPG
Create Task
Maniphest T5150

scd: For NetKey cards READKEY with keygrip fails
Closed, ResolvedPublic
Actions

Description

This is with master. This does actually happen when trying to generate an OpenPGP public key for NetKey card keys with gpg --quick-gen-key --yes nks@example.net card, but it's easier/faster to reproduce as follows (the first few SCD commands may not be necessary):
Plug in card reader (I have an IDENTIV SPR332 V2) and insert NetKey card. Then run

$ gpg-connect-agent "SCD SERIALNO" /bye
gpg-connect-agent: no running gpg-agent - starting '/opt/gnupg/master/bin/gpg-agent'
gpg-connect-agent: waiting for the agent to come up ... (5s)
gpg-connect-agent: connection to the agent established
S SERIALNO 89490173300026616520
OK

$ gpg-connect-agent "SCD LEARN --force" /bye
S READER 04E6:E003:51271834208013:0
S SERIALNO 89490173300026616520
S APPTYPE nks
S APPVERSION 3
S CHV-STATUS -4+0+-4+0
S KEYPAIRINFO 39400430E38BB96F105B740A7119FE113578B59D NKS-NKS3.4531 sa -
S CERTINFO 101 NKS-NKS3.C000
S CERTINFO 110 NKS-NKS3.B000
S KEYPAIRINFO 42C3CA6F9D7A725A59DDE06B16B61E88C62777C4 NKS-NKS3.45B1 e -
S CERTINFO 101 NKS-NKS3.C200
S KEYPAIRINFO 20E7CB1D5299669CABF29B103C692AB34CB03528 NKS-NKS3.45B2 e -
S CERTINFO 101 NKS-NKS3.C201
S KEYPAIRINFO A69B0D3796EE33E4426E5CE4B6BEEE5F1209FBA4 NKS-SIGG.4531 se -
S CERTINFO 101 NKS-SIGG.C000
OK

$ gpg-connect-agent "SCD READKEY --info-only -- NKS-NKS3.4531" /bye
S KEYPAIRINFO 39400430E38BB96F105B740A7119FE113578B59D NKS-NKS3.4531 - - rsa2048
OK

$ gpg-connect-agent "SCD READKEY --info-only -- 39400430E38BB96F105B740A7119FE113578B59D" /bye
ERR 100663414 Invalid ID <SCD>

While trying to debug this I noticed a few things.

In cmd_readkey() (scd/command.c) a check for rc seems to be missing which causes the above bogus error message "Invalid ID". I propose the following patch:

diff --git a/scd/command.c b/scd/command.c
index c1cded6b1..47912adfe 100644
--- a/scd/command.c
+++ b/scd/command.c
@@ -735,6 +739,8 @@ cmd_readkey (assuan_context_t ctx, char *line)
     }
   else
     rc = gpg_error (GPG_ERR_NO_SECKEY);
+  if (rc)
+    goto leave;
 
   if (opt_nokey)
     ;

After applying this patch the above fails with

ERR 100663313 No secret key <SCD>

Further debugging revealed that filelist in do_with_keygrip() (scd/app-nks.c) seems to contain the fourth key (A69B0D3796EE33E4426E5CE4B6BEEE5F1209FBA4) twice, but not the first key (39400430E38BB96F105B740A7119FE113578B59D). I've added

log_info ("do_with_keygrip (app-nks.c): keygripstr: %s", keygripstr);

after

err = keygripstr_from_pk_file (app, filelist[idx].fid,
                               filelist[idx].iskeypair, keygripstr,
                               NULL, NULL);

and I got the following debug output:

2020-11-20 14:54:49 scdaemon[10384] do_with_keygrip (app-nks.c): keygripstr: A69B0D3796EE33E4426E5CE4B6BEEE5F1209FBA4
2020-11-20 14:54:49 scdaemon[10384] do_with_keygrip (app-nks.c): keygripstr: 42C3CA6F9D7A725A59DDE06B16B61E88C62777C4
2020-11-20 14:54:49 scdaemon[10384] do_with_keygrip (app-nks.c): keygripstr: 20E7CB1D5299669CABF29B103C692AB34CB03528
2020-11-20 14:54:49 scdaemon[10384] nks: switching to SigG
2020-11-20 14:54:50 scdaemon[10384] do_with_keygrip (app-nks.c): keygripstr: A69B0D3796EE33E4426E5CE4B6BEEE5F1209FBA4

At this point I stopped looking further into the code.

BTW, READKEY with the keygrip of the second key also fails, but somewhat differently. It fails with error log app_readcert failed: Invalid ID.

With 2.2.23 gpg --quick-gen-key --yes nks@example.net card works except that after entering the PIN it fails with "Conditions of use not satisfied" maybe because "the NullPIN has not yet been changed".

Revisions and Commits

rG GnuPG
rG3c1acb7b9fa4 scd:nks: Support READKEY with keygrip and for "NKS-IDLM" keyref.
rG1f2823e0beee scd:nks: Add support of KEYGRIP for do_readcert.
rGc9eb4c063231 scd:nks: Fix caching keygrip (more).
rGf1bd7369a754 scd,nks: Fix caching keygrip.
rG3b3926308813 scd:nks: Support READKEY with keygrip and for "NKS-IDLM" keyref.
rG4020cd9d6562 scd:nks: Add support of KEYGRIP for do_readcert.
rG87d2c579cc38 scd:nks: Fix caching keygrip (more).
rG920154370834 scd,nks: Fix caching keygrip.
rGc3a20c88fb30 scd: Fix an error return for READKEY.

Related Objects
Search...

Event Timeline

• ikloecker created this task.Nov 20 2020, 3:36 PM
• werner added a project: gnupg (gpg23).Nov 20 2020, 6:31 PM
• gniibe added a commit: rGc3a20c88fb30: scd: Fix an error return for READKEY..Nov 25 2020, 3:45 AM
• gniibe added a subscriber: • gniibe.Nov 25 2020, 3:48 AM

For the first issue, I pushed the change in rGc3a20c88fb30: scd: Fix an error return for READKEY..

For the second issue, I think that the logic to identify a key is wrong for the cache.
Please try:

diff --git a/scd/app-nks.c b/scd/app-nks.c
index caf686316..7ed3be75f 100644
--- a/scd/app-nks.c
+++ b/scd/app-nks.c
@@ -292,7 +292,7 @@ keygripstr_from_pk_file (app_t app, int pkfid, int cfid, char *r_gripstr,
   struct fid_cache_s *ci;
 
   for (ci = app->app_local->fid_cache; ci; ci = ci->next)
-    if (ci->fid && ci->fid == pkfid)
+    if (ci->fid && ((cfid && ci->fid == cfid) || (!cfid && ci->fid == pkfid)))
       {
         if (!ci->got_keygrip)
           return gpg_error (GPG_ERR_NOT_FOUND);
@@ -455,7 +455,8 @@ keygripstr_from_pk_file (app_t app, int pkfid, int cfid, char *r_gripstr,
 
       /* FIXME: We need to implement not_found caching.  */
       for (ci = app->app_local->fid_cache; ci; ci = ci->next)
-        if (ci->fid && ci->fid == pkfid)
+        if (ci->fid
+            && ((cfid && ci->fid == cfid) || (!cfid && ci->fid == pkfid)))
           {
             /* Update the keygrip.  */
             memcpy (ci->keygripstr, r_gripstr, 2*KEYGRIP_LEN+1);
@@ -476,7 +477,7 @@ keygripstr_from_pk_file (app_t app, int pkfid, int cfid, char *r_gripstr,
             ; /* Out of memory - it is a cache, so we ignore it.  */
           else
             {
-              ci->fid = pkfid;
+              ci->fid = cfid? cfid : pkfid;
               memcpy (ci->keygripstr, r_gripstr, 2*KEYGRIP_LEN+1);
               ci->algo = algo;
               ci->got_keygrip = 1;
• gniibe triaged this task as Normal priority.Nov 25 2020, 3:48 AM
• gniibe added a commit: rG920154370834: scd,nks: Fix caching keygrip..Nov 26 2020, 1:38 AM
• gniibe added a comment.Nov 26 2020, 1:38 AM

Applied and push the change above in rG920154370834: scd,nks: Fix caching keygrip..

• werner added a project: backport.Nov 26 2020, 7:55 AM
• werner added a subscriber: • werner.Nov 27 2020, 5:54 PM

Regarding a backport I think that I will eventually backport all app-*c to stable by source copying them. We have a quite stable internal API and thus it is easier to keep at least the card specific code in sync. I did some local work in this directory some time ago.

• ikloecker added a comment.Nov 30 2020, 10:19 AM

Seems to work now. I'm not sure whether I should close this issue because it's marked for backport.

• ikloecker mentioned this in T5161: scd: For NetKey cards KEYINFO with keygrip fails.Nov 30 2020, 10:57 AM
• gniibe added a commit: rG87d2c579cc38: scd:nks: Fix caching keygrip (more)..Dec 3 2020, 8:25 AM
• ikloecker reassigned this task from • ikloecker to • gniibe.Dec 9 2020, 12:19 PM

I'm not sure why I thought that it would work now. With current master I get

$ gpg-connect-agent "SCD READKEY --info-only -- 39400430E38BB96F105B740A7119FE113578B59D" /bye
ERR 100663414 Invalid ID <SCD>

After looking into it, it's obvious why it doesn't work.

do_readkey() in app-nks.c only supports reading the key with the special id $IFDAUTHKEY (for app version 3 or later):

/* We use a generic name to retrieve PK.AUT.IFD-SPK.  */
if (!strcmp (keyid, "$IFDAUTHKEY") && app->appversion >= 3)
  ;
else /* Return the error code expected by cmd_readkey.  */
  return gpg_error (GPG_ERR_UNSUPPORTED_OPERATION);

Because of the previous, do_readkey() in command.c falls back to certificate reading:

/* Fall back to certificate reading.  */
rc = app_readcert (card, ctrl, line, &cert, &ncert);

But do_readcert() in app-nks.c only supports reading certificates specified by key ref:

if (!strncmp (certid, "NKS-NKS3.", 9))
  nks_app_id = NKS_APP_NKS;
else if (!strncmp (certid, "NKS-ESIGN.", 10))
  nks_app_id = NKS_APP_ESIGN;
else if (!strncmp (certid, "NKS-SIGG.", 9))
  nks_app_id = NKS_APP_SIGG;
else if (!strncmp (certid, "NKS-DF01.", 9))
  nks_app_id = NKS_APP_NKS;
else if (!strncmp (certid, "NKS-IDLM.", 9))
  nks_app_id = NKS_APP_IDLM;
else
  return gpg_error (GPG_ERR_INV_ID);

There are (at least) two solutions:

  1. If possible, support other keys than the special PK.AUT.IFD-SPK (aka $IFDAUTHKEY) in do_readkey() in app-nks.c.
  2. Support reading certificates specified by key grip in do_readcert() in app-nks.c.
    1. Alternatively, map the key grip to the key ref and then call the other functions with the key ref instead of the key grip. For example, in cmd_readkey() in command.c there is already a lookup by key grip to get the correct card. Maybe this lookup could also return the key ref for the key grip. To me this feels like a good, clean solution because then the card-specific code in app-nks.c (and the other app-*.c) does not need to support keys specified by key grip.
• gniibe added a comment.Dec 10 2020, 2:37 AM

Thanks a lot for your time to locate the problem. I took the approach of #2.

With little (mostly no) knowledge of NKS card, I think I fixed this issue.
Please note that access by KEYGRIP is relatively new feature.
I think that we need to check KEYGRIP support for other cards.

• gniibe added a commit: rG4020cd9d6562: scd:nks: Add support of KEYGRIP for do_readcert..Dec 10 2020, 2:41 AM
• ikloecker closed this task as Resolved.Dec 10 2020, 10:39 AM
In T5150#140039, @gniibe wrote:

With little (mostly no) knowledge of NKS card, I think I fixed this issue.

It works. Thanks.

I checked this also for OpenPGP cards and PIV cards. For both READKEY with keygrip works.

• gniibe added a comment.Dec 11 2020, 1:13 AM

Reading the code again, I think that some configuration of NKS card doesn't work well, when it has no certificates but keys (e.g. IDLM config).
I'm going to fix do_readkey as well (the approach #1).

• gniibe added a commit: rG3b3926308813: scd:nks: Support READKEY with keygrip and for "NKS-IDLM" keyref..Dec 11 2020, 6:12 AM
• werner added a commit: rGf1bd7369a754: scd,nks: Fix caching keygrip..Oct 20 2022, 12:23 PM
• werner added a commit: rGc9eb4c063231: scd:nks: Fix caching keygrip (more)..
• werner added a commit: rG1f2823e0beee: scd:nks: Add support of KEYGRIP for do_readcert..
• werner added a commit: rG3c1acb7b9fa4: scd:nks: Support READKEY with keygrip and for "NKS-IDLM" keyref..