[PATCH] agent: pksign result conversion to sexp to upper layer

NIIBE Yutaka gniibe at fsij.org
Wed Feb 27 13:05:17 CET 2013


Thank you for reviewing the patch.

On 2013-02-27 at 09:48 +0100, Werner Koch wrote:
> On Mon, 25 Feb 2013 01:47, gniibe at fsij.org said:
> 
> > This patch moves the conversion/building of SEXP structure from
> > agent_card_pksign to agent_pksign_do.
> 
> I see no problem with this change.

OK.  If it will be needed to move this to scdaemon, we will be able to
do that later.  As move to scdaemon will introduce a protocol change,
I think that it's good to defer decision.

> Please change to 
> 
>          int is_RSA = 0;
>          int is_ECDSA = 0;
> 
> as per GNU coding standards.
> 
> > +	if (r_buflen > len/2)
> > +	  xfree (r_buf);
> > +
> > +	if (s_buflen > len/2)
> > +	  xfree (s_buf);
> 
> These conditions are not easy to understand.  What about using an
> explicit flag telling whether they have been allocated or to use an
> alias variable initialized to NULL which may then always be freed?

Done.

Now it's like this:

+      else if (is_ECDSA)
+        {
+          unsigned char *r_buf_allocated = NULL;
+          unsigned char *s_buf_allocated = NULL;
+          unsigned char *r_buf, *s_buf;
+          int r_buflen, s_buflen;
+
+          r_buflen = s_buflen = len/2;
+
+          if (*buf & 0x80)
+            {
+              r_buflen++;
+              r_buf_allocated = xtrymalloc (r_buflen);
+              if (!r_buf_allocated)
+                goto leave;
+
+              r_buf = r_buf_allocated;
+              memcpy (r_buf + 1, buf, len/2);
+              *r_buf = 0;
+            }
+          else
+            r_buf = buf;
+
+          if (*(buf + len/2) & 0x80)
+            {
+              s_buflen++;
+              s_buf_allocated = xtrymalloc (s_buflen);
+              if (!s_buf_allocated)
+                {
+                  xfree (r_buf_allocated);
+                  goto leave;
+                }
+
+              s_buf = s_buf_allocated;
+              memcpy (s_buf + 1, buf + len/2, len/2);
+              *s_buf = 0;
+            }
+          else
+            s_buf = buf + len/2;
+
+          rc = gcry_sexp_build (&s_sig, NULL, "(sig-val(ecdsa(r%b)(s%b)))",
+                                r_buflen, r_buf,
+                                s_buflen, s_buf);
+          xfree (r_buf_allocated);
+          xfree (s_buf_allocated);
+        }


I'll commit this tomorrow after testing.  I'm out of town today.
-- 





More information about the Gnupg-devel mailing list