Mailing List Archive

[PATCH gnupg 1/2] sm: Support generation of card-based ECDSA CSR.
* sm/call-agent.c (gpgsm_scd_pksign): Identify type of signing key
and format resulting S-expression accordingly.
* sm/misc.c (transform_sigval): Support ECDSA signatures.
--

Current GpgSM implementation assumes card-based keys are RSA keys.
This patch introduces support for ECDSA keys.

By itself this patch is not sufficient, we also need support
from libksba.

GnuPG-bug-id: 4092
Signed-off-by: Damien Goutte-Gattat <dgouttegattat@incenp.org>
---
sm/call-agent.c | 57 +++++++++++++++++++++++++++++++------------
sm/misc.c | 75 +++++++++++++++++++++++++++++++++++++++++++--------------
2 files changed, 98 insertions(+), 34 deletions(-)

diff --git a/sm/call-agent.c b/sm/call-agent.c
index 20d879fa4..6ac715fab 100644
--- a/sm/call-agent.c
+++ b/sm/call-agent.c
@@ -334,7 +334,7 @@ gpgsm_scd_pksign (ctrl_t ctrl, const char *keyid, const char *desc,
unsigned char *digest, size_t digestlen, int digestalgo,
unsigned char **r_buf, size_t *r_buflen )
{
- int rc, i;
+ int rc, i, pkalgo;
char *p, line[ASSUAN_LINELENGTH];
membuf_t data;
size_t len;
@@ -342,6 +342,7 @@ gpgsm_scd_pksign (ctrl_t ctrl, const char *keyid, const char *desc,
unsigned char *sigbuf;
size_t sigbuflen;
struct default_inq_parm_s inq_parm;
+ gcry_sexp_t sig;

(void)desc;

@@ -366,6 +367,23 @@ gpgsm_scd_pksign (ctrl_t ctrl, const char *keyid, const char *desc,
if (digestlen*2 + 50 > DIM(line))
return gpg_error (GPG_ERR_GENERAL);

+ /* Get the key type from the scdaemon. */
+ snprintf (line, DIM(line), "SCD READKEY %s", keyid);
+ init_membuf (&data, 1024);
+ rc = assuan_transact (agent_ctx, line,
+ put_membuf_cb, &data, NULL, NULL, NULL, NULL);
+ if (rc)
+ {
+ xfree (get_membuf (&data, &len));
+ return rc;
+ }
+
+ p = get_membuf (&data, &len);
+ pkalgo = get_pk_algo_from_canon_sexp (p, len);
+ xfree (p);
+ if (!pkalgo)
+ return gpg_error (GPG_ERR_WRONG_PUBKEY_ALGO);
+
p = stpcpy (line, "SCD SETDATA " );
for (i=0; i < digestlen ; i++, p += 2 )
sprintf (p, "%02X", digest[i]);
@@ -386,24 +404,31 @@ gpgsm_scd_pksign (ctrl_t ctrl, const char *keyid, const char *desc,
}
sigbuf = get_membuf (&data, &sigbuflen);

- /* Create an S-expression from it which is formatted like this:
- "(7:sig-val(3:rsa(1:sSIGBUFLEN:SIGBUF)))" Fixme: If a card ever
- creates non-RSA keys we need to change things. */
- *r_buflen = 21 + 11 + sigbuflen + 4;
- p = xtrymalloc (*r_buflen);
- *r_buf = (unsigned char*)p;
- if (!p)
+ switch(pkalgo)
{
- xfree (sigbuf);
- return 0;
+ case GCRY_PK_RSA:
+ rc = gcry_sexp_build (&sig, NULL, "(sig-val(rsa(s%b)))",
+ sigbuflen, sigbuf);
+ break;
+
+ case GCRY_PK_ECC:
+ rc = gcry_sexp_build (&sig, NULL, "(sig-val(ecdsa(r%b)(s%b)))",
+ sigbuflen/2, sigbuf,
+ sigbuflen/2, sigbuf + sigbuflen/2);
+ break;
+
+ default:
+ rc = gpg_error (GPG_ERR_WRONG_PUBKEY_ALGO);
+ break;
}
- p = stpcpy (p, "(7:sig-val(3:rsa(1:s" );
- sprintf (p, "%u:", (unsigned int)sigbuflen);
- p += strlen (p);
- memcpy (p, sigbuf, sigbuflen);
- p += sigbuflen;
- strcpy (p, ")))");
xfree (sigbuf);
+ if (rc)
+ return rc;
+
+ rc = make_canon_sexp (sig, r_buf, r_buflen);
+ gcry_sexp_release (sig);
+ if (rc)
+ return rc;

assert (gcry_sexp_canon_len (*r_buf, *r_buflen, NULL, NULL));
return 0;
diff --git a/sm/misc.c b/sm/misc.c
index 6d047763b..9bf528513 100644
--- a/sm/misc.c
+++ b/sm/misc.c
@@ -109,13 +109,16 @@ transform_sigval (const unsigned char *sigval, size_t sigvallen, int mdalgo,
gpg_error_t err;
const unsigned char *buf, *tok;
size_t buflen, toklen;
- int depth, last_depth1, last_depth2;
+ int depth, last_depth1, last_depth2, pkalgo;
int is_pubkey = 0;
- const unsigned char *rsa_s = NULL;
- size_t rsa_s_len = 0;
+ const unsigned char *rsa_s, *ecc_r, *ecc_s;
+ size_t rsa_s_len, ecc_r_len, ecc_s_len;
const char *oid;
gcry_sexp_t sexp;

+ rsa_s = ecc_r = ecc_s = NULL;
+ rsa_s_len = ecc_r_len = ecc_s_len = 0;
+
*r_newsigval = NULL;
if (r_newsigvallen)
*r_newsigvallen = 0;
@@ -137,7 +140,13 @@ transform_sigval (const unsigned char *sigval, size_t sigvallen, int mdalgo,
return err;
if ((err = parse_sexp (&buf, &buflen, &depth, &tok, &toklen)))
return err;
- if (!tok || toklen != 3 || memcmp ("rsa", tok, toklen))
+ if (!tok)
+ return gpg_error (GPG_ERR_WRONG_PUBKEY_ALGO);
+ if (toklen == 3 && !memcmp ("rsa", tok, 3))
+ pkalgo = GCRY_PK_RSA;
+ else if (toklen == 5 && !memcmp ("ecdsa", tok, 5))
+ pkalgo = GCRY_PK_ECC;
+ else
return gpg_error (GPG_ERR_WRONG_PUBKEY_ALGO);

last_depth1 = depth;
@@ -150,16 +159,27 @@ transform_sigval (const unsigned char *sigval, size_t sigvallen, int mdalgo,
return err;
if (tok && toklen == 1)
{
- const unsigned char **mpi;
- size_t *mpi_len;
+ const unsigned char **mpi = NULL;
+ size_t *mpi_len = NULL;

switch (*tok)
{
- case 's': mpi = &rsa_s; mpi_len = &rsa_s_len; break;
+ case 's':
+ if (pkalgo == GCRY_PK_RSA)
+ {
+ mpi = &rsa_s;
+ mpi_len = &rsa_s_len;
+ }
+ else if (pkalgo == GCRY_PK_ECC)
+ {
+ mpi = &ecc_s;
+ mpi_len = &ecc_s_len;
+ }
+ break;
+
+ case 'r': mpi = &ecc_r; mpi_len = &ecc_r_len; break;
default: mpi = NULL; mpi_len = NULL; break;
}
- if (mpi && *mpi)
- return gpg_error (GPG_ERR_DUP_VALUE);

if ((err = parse_sexp (&buf, &buflen, &depth, &tok, &toklen)))
return err;
@@ -182,33 +202,52 @@ transform_sigval (const unsigned char *sigval, size_t sigvallen, int mdalgo,
return err;

/* Map the hash algorithm to an OID. */
- switch (mdalgo)
+ switch (mdalgo | (pkalgo << 8))
{
- case GCRY_MD_SHA1:
+ case GCRY_MD_SHA1 | (GCRY_PK_RSA << 8):
oid = "1.2.840.113549.1.1.5"; /* sha1WithRSAEncryption */
break;

- case GCRY_MD_SHA256:
+ case GCRY_MD_SHA256 | (GCRY_PK_RSA << 8):
oid = "1.2.840.113549.1.1.11"; /* sha256WithRSAEncryption */
break;

- case GCRY_MD_SHA384:
+ case GCRY_MD_SHA384 | (GCRY_PK_RSA << 8):
oid = "1.2.840.113549.1.1.12"; /* sha384WithRSAEncryption */
break;

- case GCRY_MD_SHA512:
+ case GCRY_MD_SHA512 | (GCRY_PK_RSA << 8):
oid = "1.2.840.113549.1.1.13"; /* sha512WithRSAEncryption */
break;

+ case GCRY_MD_SHA224 | (GCRY_PK_ECC << 8):
+ oid = "1.2.840.10045.4.3.1"; /* ecdsa-with-sha224 */
+ break;
+
+ case GCRY_MD_SHA256 | (GCRY_PK_ECC << 8):
+ oid = "1.2.840.10045.4.3.2"; /* ecdsa-with-sha256 */
+ break;
+
+ case GCRY_MD_SHA384 | (GCRY_PK_ECC << 8):
+ oid = "1.2.840.10045.4.3.3"; /* ecdsa-with-sha384 */
+ break;
+
+ case GCRY_MD_SHA512 | (GCRY_PK_ECC << 8):
+ oid = "1.2.840.10045.4.3.4"; /* ecdsa-with-sha512 */
+ break;
+
default:
return gpg_error (GPG_ERR_DIGEST_ALGO);
}

- if (rsa_s && !is_pubkey)
- err = gcry_sexp_build (&sexp, NULL, "(sig-val(%s(s%b)))",
- oid, (int)rsa_s_len, rsa_s);
- else
+ if (is_pubkey)
err = gcry_sexp_build (&sexp, NULL, "(sig-val(%s))", oid);
+ else if (pkalgo == GCRY_PK_RSA)
+ err = gcry_sexp_build (&sexp, NULL, "(sig-val(%s(s%b)))", oid,
+ (int)rsa_s_len, rsa_s);
+ else if (pkalgo == GCRY_PK_ECC)
+ err = gcry_sexp_build (&sexp, NULL, "(sig-val(%s(r%b)(s%b)))", oid,
+ (int)ecc_r_len, ecc_r, (int)ecc_s_len, ecc_s);
if (err)
return err;
err = make_canon_sexp (sexp, r_newsigval, r_newsigvallen);
--
2.14.5


_______________________________________________
Gnupg-devel mailing list
Gnupg-devel@gnupg.org
http://lists.gnupg.org/mailman/listinfo/gnupg-devel
Re: [PATCH gnupg 1/2] sm: Support generation of card-based ECDSA CSR. [ In reply to ]
Hello,

Damien Goutte-Gattat wrote:
> * sm/call-agent.c (gpgsm_scd_pksign): Identify type of signing key
> and format resulting S-expression accordingly.
> * sm/misc.c (transform_sigval): Support ECDSA signatures.
> --
>
> Current GpgSM implementation assumes card-based keys are RSA keys.
> This patch introduces support for ECDSA keys.
>
> By itself this patch is not sufficient, we also need support
> from libksba.

Comparing agent_pksign_do in gnupg/agent/pksign.c and your
gpgsm_scd_pksign, I think that it's not yet perfect.

(1) There are three cases; RSA, ECDSA, and EdDSA. It's good that we can
support all.


(2) The format of signature by card in agent_pksign_do is the one of
libgcrypt. In the agent_pksign_do function, for ECDSA, it checks
the MSB, and put the prefix 0x00 in this case. It should be same
for GpgSM.


Well, shall we apply your patch first, and then proceed to more changes
for EdDSA and 0x00-for-MSB? Or, are you going to submit updated patch?
Whichever is OK. Let me know.
--

_______________________________________________
Gnupg-devel mailing list
Gnupg-devel@gnupg.org
http://lists.gnupg.org/mailman/listinfo/gnupg-devel
Re: [PATCH gnupg 1/2] sm: Support generation of card-based ECDSA CSR. [ In reply to ]
Hi,

On Thu, Feb 14, 2019 at 09:55:59AM +0900, NIIBE Yutaka wrote:
> (1) There are three cases; RSA, ECDSA, and EdDSA. It's good that we can
> support all.

Agreed. I have another patch in the works to add support for EdDSA.


> (2) The format of signature by card in agent_pksign_do is the one of
> libgcrypt. In the agent_pksign_do function, for ECDSA, it checks
> the MSB, and put the prefix 0x00 in this case. It should be same
> for GpgSM.

I can look into that, but I am not sure I understand everything here.

Looking at the agent_pksign_do function, I can see that this MSB
checking is performed for RSA and ECDSA signatures, but not for EdDSA.

(1) Is it normal not to do this check for EdDSA?

(2) Currently gpgsm_scd_pksign does not perform this check for RSA
(that’s actually why I didn’t think it was needed for ECDSA as
well, and didn’t implement it in my patch). Again, is that
normal? Or should we add this check also for RSA?


> Well, shall we apply your patch first, and then proceed to more changes
> for EdDSA

Yes, that was kind of my plan: First add ECDSA support, then build upon
that to add EdDSA support afterward. I will post the patch for EdDSA
soon.


> and 0x00-for-MSB?

Likewise, I can submit another patch later for this issue, once I will
be sure I know what I am doing and why I am doing it. :)

Thanks,

- Damien