SKS-Keyserver / sks-keyserver

OpenPGP keyserver
GNU General Public License v2.0
135 stars 15 forks source link

DB_DBT_READONLY should not be set on data DBT. #82

Closed christmart closed 3 years ago

christmart commented 4 years ago

sks-keyserver from commit 9e9d504d10d8203e46494f196568dcdf7443b914 build for Debian buster results in errors like

Fatal database error: Bdb.DBError("BDB0620 DB_DBT_READONLY should not be set on data DBT.")

and eventually

Fatal database error: Bdb.DBError("BDB0110 Log sequence error: page LSN 2 35017101; previous LSN 2 33875510 add_keys_merge failed: Sys.Break Key addition failed: Sys.Break

and an sks abort.

commenting line 66 in bdb_stubs.c

dbt->flags = DB_DBT_READONLY

stops these errors and keeps sks from crashing.

Maybe you should not use dbt_from_string an all key and data values with always setting DB_DBT_READONLY.

ArcticB commented 3 years ago

Hello, I'm facing the same issue when I try to drop GPG keys:

Jan 04 11:16:35 *** sks[2891]: 2021-01-04 11:16:35 Handling DeleteKey
Jan 04 11:16:35 *** sks[2891]: 2021-01-04 11:16:35 Fatal database error: Bdb.DBError("BDB0620 DB_DBT_READONLY should not be set on data DBT.")
Jan 04 11:16:35 *** sks[2891]: 2021-01-04 11:16:35 <command handler> callback interrupted by break

I thought that it was due to my chroot but it's not because I get this error also on a simple fresh install.

Debian package version: sks/unstable,now 1.1.6+git20200620.9e9d504-1+b1 amd64

ygrek commented 3 years ago

ftr this bug was introduced in #77 fixed as proposed, thanks!

kmkaplan commented 3 years ago

I have tracked down the error to caml_cursor_init_both.

This is the line causing errors. According to Berkeley DB's documentation:

In all other ways, this (DB_GET_BOTH) flag is identical to the DB_SET flag [...] and return the datum associated with the given key.

Replacing the line with

   zerob(&data, sizeof(DBT));
   data.data = String_val(vdata);
   data.size = string_length(vdata);

seems to solve the issue.

christmart commented 3 years ago
   zerob(&data, sizeof(DBT));
   data.data = String_val(vdata);
   data.size = string_length(vdata);

But isn't this essentialy the same solution only limiting it on data? You just ommit setting the flag for data, like removing the #ifdef in ygrek's patch.

kmkaplan commented 3 years ago

Yes, exactly. I found that the DB_DBT_READONLY was wrong only in this case. For me this solves the problem. You still have BDB0620 errors with it (or with removing all DB_DBT_READONLY)?

ygrek commented 3 years ago

DB_DBT_READONLY is only needed if keys are subject to possible mutations, but this is not the case in sks db usage. So we can just simplify code by not using it anywhere (at first I wanted to differentiate key and data assignments). Nothing needs to be done further I believe.

christmart commented 3 years ago

For me this solves the problem. You still have BDB0620 errors with it (or with removing all DB_DBT_READONLY)?

No DBD0620 errors.