Quantcast

[GNUPG HEAD] sqlite3 compatibility issue

classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GNUPG HEAD] sqlite3 compatibility issue

Ineiev
Hello,

GnuPG doesn't build with sqlite 3.7.9 because it has
no sqlite3_errstr ().  I wonder if it could be worked around like

diff --git a/g10/gpgsql.c b/g10/gpgsql.c
index 5b75569..3bade5c 100644
--- a/g10/gpgsql.c
+++ b/g10/gpgsql.c
@@ -64,7 +64,8 @@ gpgsql_stepx (sqlite3 *db,
               const char *sql, ...)
 {
   int rc;
-  int err = 0;
+  int err = SQLITE_OK;
+  const char *errstr = NULL;
   sqlite3_stmt *stmt = NULL;
 
   va_list va;
@@ -161,7 +162,7 @@ gpgsql_stepx (sqlite3 *db,
               log_fatal ("Bad value for parameter type %d.\n", t);
             }
 
-          if (err)
+          if (err != SQLITE_OK)
             {
               log_fatal ("Error binding parameter %d\n", i);
               goto out;
@@ -203,6 +204,7 @@ gpgsql_stepx (sqlite3 *db,
             /* Out of memory.  */
             {
               err = SQLITE_NOMEM;
+              errstr = "out of memory";
               break;
             }
         }
@@ -211,6 +213,7 @@ gpgsql_stepx (sqlite3 *db,
         /* A non-zero result means to abort.  */
         {
           err = SQLITE_ABORT;
+          errstr = "callback requested query abort";
           break;
         }
     }
@@ -218,33 +221,26 @@ gpgsql_stepx (sqlite3 *db,
  out:
   xfree (azColName);
 
+  if (err != SQLITE_OK && errmsg && !errstr)
+    errstr = sqlite3_errmsg (db);
+
   if (stmtp)
     rc = sqlite3_reset (stmt);
   else
     rc = sqlite3_finalize (stmt);
-  if (rc == SQLITE_OK && err)
-    /* Local error.  */
-    {
-      rc = err;
-      if (errmsg)
-        {
-          const char *e = sqlite3_errstr (err);
-          size_t l = strlen (e) + 1;
-          *errmsg = sqlite3_malloc (l);
-          if (! *errmsg)
-            log_fatal ("Out of memory.\n");
-          memcpy (*errmsg, e, l);
-        }
-    }
-  else if (rc != SQLITE_OK && errmsg)
-    /* Error reported by sqlite.  */
+
+  if (rc != SQLITE_OK)
+    errstr = sqlite3_errmsg (db);
+  else
+    rc = err;
+
+  if (rc != SQLITE_OK && errmsg)
     {
-      const char * e = sqlite3_errmsg (db);
-      size_t l = strlen (e) + 1;
+      size_t l = strlen (errstr) + 1;
       *errmsg = sqlite3_malloc (l);
       if (! *errmsg)
         log_fatal ("Out of memory.\n");
-      memcpy (*errmsg, e, l);
+      memcpy (*errmsg, errstr, l);
     }
 
   return rc;

_______________________________________________
Gnupg-devel mailing list
[hidden email]
http://lists.gnupg.org/mailman/listinfo/gnupg-devel

signature.asc (499 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [GNUPG HEAD] sqlite3 compatibility issue

Daniel Kahn Gillmor-7
On Thu 2017-05-18 08:50:21 -0400, Ineiev wrote:

> GnuPG doesn't build with sqlite 3.7.9 because it has
> no sqlite3_errstr ().

sqlite3 3.7.9 is positively ancient.  even debian oldstable has 3.7.13.

why should we support such an old version of this library?  I'd imagine
there are sqlite3 security problems that have been fixed since the
release of 3.7.9 (though i haven't dug in that project's archives to
verify).

I'd rather not add compatibility cruft just to support people who
are still running a broken library and really should upgrade.

    --dkg

_______________________________________________
Gnupg-devel mailing list
[hidden email]
http://lists.gnupg.org/mailman/listinfo/gnupg-devel
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [GNUPG HEAD] sqlite3 compatibility issue

Andreas Stieger-2
In reply to this post by Ineiev
Hi,


On 05/18/2017 02:50 PM, Ineiev wrote:

> GnuPG doesn't build with sqlite 3.7.9 because it has
> no sqlite3_errstr ().  I wonder if it could be worked around like
> [...]
>
>
> @@ -218,33 +221,26 @@ gpgsql_stepx (sqlite3 *db,
>   out:
>    xfree (azColName);
>  
> +  if (err != SQLITE_OK && errmsg && !errstr)
> +    errstr = sqlite3_errmsg (db);
> +
>    if (stmtp)
>      rc = sqlite3_reset (stmt);
>    else
>      rc = sqlite3_finalize (stmt);
> -  if (rc == SQLITE_OK && err)
> -    /* Local error.  */
> -    {
> -      rc = err;
> -      if (errmsg)
> -        {
> -          const char *e = sqlite3_errstr (err);
> -          size_t l = strlen (e) + 1;
> -          *errmsg = sqlite3_malloc (l);
> -          if (! *errmsg)
> -            log_fatal ("Out of memory.\n");
> -          memcpy (*errmsg, e, l);
> -        }
> -    }
> -  else if (rc != SQLITE_OK && errmsg)
> -    /* Error reported by sqlite.  */
> +
> +  if (rc != SQLITE_OK)
> +    errstr = sqlite3_errmsg (db);
> +  else
> +    rc = err;
> +
> +  if (rc != SQLITE_OK && errmsg)
>      {
> -      const char * e = sqlite3_errmsg (db);
> -      size_t l = strlen (e) + 1;
> +      size_t l = strlen (errstr) + 1;
>        *errmsg = sqlite3_malloc (l);
>        if (! *errmsg)
>          log_fatal ("Out of memory.\n");
> -      memcpy (*errmsg, e, l);
> +      memcpy (*errmsg, errstr, l);
>      }
>  
>    return rc;

Too complex. Why not use AC_CHECK_FUNCS to check for the availability of
the function, and conditionally define a stub function matching the
signature of sqlite3_errstr()?

Andreas

--
Andreas Stieger <[hidden email]>
Project Manager Security
SUSE Linux GmbH, GF: Felix Imend├Ârffer, Jane Smithard, Graham Norton,
HRB 21284 (AG N├╝rnberg)



_______________________________________________
Gnupg-devel mailing list
[hidden email]
http://lists.gnupg.org/mailman/listinfo/gnupg-devel
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [GNUPG HEAD] sqlite3 compatibility issue

Ineiev
In reply to this post by Daniel Kahn Gillmor-7
On Thu, May 18, 2017 at 08:59:06AM -0400, Daniel Kahn Gillmor wrote:
> On Thu 2017-05-18 08:50:21 -0400, Ineiev wrote:
>
> > GnuPG doesn't build with sqlite 3.7.9 because it has
> > no sqlite3_errstr ().
>
> sqlite3 3.7.9 is positively ancient.  even debian oldstable has 3.7.13.

3.7.13 doesn't seem to have sqlite3_errstr (), either.

> why should we support such an old version of this library?

3.7.9 is what comes with Trisquel 6.0, and Trisquel 6.0 is still
supported by Trisquel maintainers.  I believe there must be
other distros who provide at least equally old libraries.

> I'd imagine
> there are sqlite3 security problems that have been fixed since the
> release of 3.7.9 (though i haven't dug in that project's archives to
> verify).

I guess they either backported the fixes, or there have been
no real ones.

Thank you!

_______________________________________________
Gnupg-devel mailing list
[hidden email]
http://lists.gnupg.org/mailman/listinfo/gnupg-devel

signature.asc (499 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [GNUPG HEAD] sqlite3 compatibility issue

Ineiev
In reply to this post by Andreas Stieger-2
On Thu, May 18, 2017 at 03:19:05PM +0200, Andreas Stieger wrote:
>
> Too complex. Why not use AC_CHECK_FUNCS to check for the availability of
> the function, and conditionally define a stub function matching the
> signature of sqlite3_errstr()?

I think that would be more complex. my patch only changes one file,
it doesn't use conditional compilation, and it actually reduces code
size. with AC_CHECK_FUNCS, the number of lines would likely increase.

_______________________________________________
Gnupg-devel mailing list
[hidden email]
http://lists.gnupg.org/mailman/listinfo/gnupg-devel

signature.asc (499 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [GNUPG HEAD] sqlite3 compatibility issue

Werner Koch
On Thu, 18 May 2017 18:19, [hidden email] said:

> I think that would be more complex. my patch only changes one file,
> it doesn't use conditional compilation, and it actually reduces code
> size. with AC_CHECK_FUNCS, the number of lines would likely increase.

Nope.  We use or will use SQLite also at other places and thus a generic
solution would be the way to go.  However, as of now you can build GnuPG
without SQLite and thus I don't see why we should add backward
compatibility for a niche OS.

As a workaround you could link statically to a decent SQLite version.


Shalom-Salam,

   Werner

--
Die Gedanken sind frei.  Ausnahmen regelt ein Bundesgesetz.

_______________________________________________
Gnupg-devel mailing list
[hidden email]
http://lists.gnupg.org/mailman/listinfo/gnupg-devel

attachment0 (233 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [GNUPG HEAD] sqlite3 compatibility issue

Ineiev
On Fri, May 19, 2017 at 08:21:00AM +0200, Werner Koch wrote:

> On Thu, 18 May 2017 18:19, [hidden email] said:
>
> > I think that would be more complex. my patch only changes one file,
> > it doesn't use conditional compilation, and it actually reduces code
> > size. with AC_CHECK_FUNCS, the number of lines would likely increase.
>
> Nope.  We use or will use SQLite also at other places and thus a generic
> solution would be the way to go.  However, as of now you can build GnuPG
> without SQLite and thus I don't see why we should add backward
> compatibility for a niche OS.
Thank you; do you see why configure could detect old versions
and suggest the user that they won't do?

_______________________________________________
Gnupg-devel mailing list
[hidden email]
http://lists.gnupg.org/mailman/listinfo/gnupg-devel

signature.asc (499 bytes) Download Attachment
Loading...