Hi,
On Tue, 3 Mar 2020 20:45:17 +0100
Martijn van Duren <openbsd+t...@list.imperialat.at> wrote:
> I don't agree with this diff, even though you're right with it being
> broken. Right now the regress test uses it/tries to use it and even
> though someone trying to run regress on an actual production machine
> somewhat deserves to get shot in the foot, I don't think we should
> make that our goal.
>
> Also, I can see this being a helpful feature for people doing some
> development work (e.g. testing with delegations etc without setting
> up a vm-environment).
>
> Could you please look into fixing the actual issue?
this patch fixes only the two issues. I de-const'd "char * datadir" in
ldapd to get rid of the new warning.
Best regards
Robert
diff 1813335e849f285a868ea3d474b4704812b1843e /usr/src
blob - c8564c5543f518a720e049c559556f87edda6b8a
file + usr.sbin/ldapctl/ldapctl.c
--- usr.sbin/ldapctl/ldapctl.c
+++ usr.sbin/ldapctl/ldapctl.c
@@ -150,7 +150,7 @@ index_namespace(struct namespace *ns, const char *data
log_info("indexing namespace %s", ns->suffix);
- if (asprintf(&path, "%s/%s_data.db", DATADIR, ns->suffix) == -1)
+ if (asprintf(&path, "%s/%s_data.db", datadir, ns->suffix) == -1)
return -1;
data_db = btree_open(path, BT_NOSYNC | BT_REVERSEKEY, 0644);
free(path);
blob - 797a36f89f33233c2d9655a307ec5e727b9fbaa1
file + usr.sbin/ldapd/ldapd.c
--- usr.sbin/ldapd/ldapd.c
+++ usr.sbin/ldapd/ldapd.c
@@ -55,7 +55,7 @@ static pid_t start_child(enum ldapd_process, char *,
struct ldapd_stats stats;
pid_t ldape_pid;
-const char *datadir = DATADIR;
+char *datadir = DATADIR;
void
usage(void)
@@ -415,7 +415,7 @@ static pid_t
start_child(enum ldapd_process p, char *argv0, int fd, int debug,
int verbose, char *csockpath, char *conffile)
{
- char *argv[9];
+ char *argv[11];
int argc = 0;
pid_t pid;
@@ -459,7 +459,11 @@ start_child(enum ldapd_process p, char *argv0, int fd,
argv[argc++] = "-f";
argv[argc++] = conffile;
}
-
+ if (datadir) {
+ argv[argc++] = "-r";
+ argv[argc++] = datadir;
+ }
+
argv[argc++] = NULL;
execvp(argv0, argv);
blob - 2259f57bde3c2918a8b200747c194e5768ae40f1
file + usr.sbin/ldapd/namespace.c
--- usr.sbin/ldapd/namespace.c
+++ usr.sbin/ldapd/namespace.c
@@ -29,7 +29,7 @@
#include "ldapd.h"
#include "log.h"
-extern const char *datadir;
+extern char *datadir;
/* Maximum number of requests to queue per namespace during compaction.
* After this many requests, we return LDAP_BUSY.
>
> martijn@
>
> On 3/2/20 8:00 PM, Robert Klein wrote:
> > Hi,
> >
> > in ldapd and ldapctl the "-r directory" command line argument does
> > not work:
> >
> > ldapd fork/execs itself but the directory command line argument is
> > not given to the execvp call which then uses the default data
> > directory.
> >
> > ldapctl has a thinko/typo which causes the data_db to be always
> > opened in the default directory for indexing.
> >
> > The patch below removes the command line argument and corresponding
> > global variable and instead uses a configuration file directive
> > "datadir". Incidentally removes the global 'datadir' variable used
> > before.
> >
> > Best regards
> > Robert
> >
> >
> > -----------------------------------------------
> > commit 6a71c90c19b5dc5850305c15696af3f14d26c168 (master)
> > from: Robert Klein <rokl...@roklein.de>
> > date: Mon Mar 2 18:50:12 2020 UTC
> >
> > fix ldapd/ldapctl data directory handling
> >
> > -r directive didn't work as intended in both
> > ldapd and ldapctl.
> >
> > this patch replaces command line argument '-r directory'
> > with a configuration file directive 'datadir'.
> >
> > diff f62b80b397c780e8273b5cc67d544b951c4c9d1f
> > 35bb29194b2067dab925ec4566dfb82f9bf34d65 blob -
> > 48cff01b435bca0deebf28f55e6f71675c5752f2 blob +
> > 231af2ddbbee2da446d2f03c9c48e679e216e993 ---
> > usr.sbin/ldapctl/ldapctl.8 +++ usr.sbin/ldapctl/ldapctl.8
> > @@ -24,7 +24,6 @@
> > .Nm ldapctl
> > .Op Fl v
> > .Op Fl f Ar file
> > -.Op Fl r Ar directory
> > .Op Fl s Ar socket
> > .Ar command
> > .Op Ar argument ...
> > @@ -42,11 +41,6 @@ Use
> > .Ar file
> > as the configuration file, instead of the default
> > .Pa /etc/ldapd.conf .
> > -.It Fl r Ar directory
> > -Store and read database files in
> > -.Ar directory ,
> > -instead of the default
> > -.Pa /var/db/ldap .
> > .It Fl s Ar socket
> > Use
> > .Ar socket
> > blob - c8564c5543f518a720e049c559556f87edda6b8a
> > blob + 6830a603d153794390faafbc9aadc8ef0bd985c0
> > --- usr.sbin/ldapctl/ldapctl.c
> > +++ usr.sbin/ldapctl/ldapctl.c
> > @@ -58,10 +58,10 @@ void show_stats(struct imsg
> > *imsg); void show_dbstats(const char *prefix,
> > struct btree_stat *st); void show_nsstats(struct
> > imsg *imsg); int compact_db(const char *path);
> > -int compact_namespace(struct namespace *ns, const
> > char *datadir); -int compact_namespaces(const char
> > *datadir); -int index_namespace(struct namespace
> > *ns, const char *datadir); -int
> > index_namespaces(const char *datadir); +int
> > compact_namespace(struct namespace *ns); +int
> > compact_namespaces(void); +int
> > index_namespace(struct namespace *ns); +int
> > index_namespaces(void); int
> > ssl_load_certfile(struct ldapd_config *, const char *, u_int8_t);
> > __dead void
> > @@ -70,7 +70,7 @@ usage(void)
> > extern char *__progname;
> >
> > fprintf(stderr,
> > - "usage: %s [-v] [-f file] [-r directory] [-s socket] "
> > + "usage: %s [-v] [-f file] [-s socket] "
> > "command [argument ...]\n",
> > __progname);
> > exit(1);
> > @@ -97,11 +97,11 @@ compact_db(const char *path)
> > }
> >
> > int
> > -compact_namespace(struct namespace *ns, const char *datadir)
> > +compact_namespace(struct namespace *ns)
> > {
> > char *path;
> >
> > - if (asprintf(&path, "%s/%s_data.db", datadir, ns->suffix)
> > == -1)
> > + if (asprintf(&path, "%s/%s_data.db", conf->datadir,
> > ns->suffix) == -1) return -1;
> > if (compact_db(path) != 0) {
> > log_warn("%s", path);
> > @@ -110,7 +110,7 @@ compact_namespace(struct namespace *ns, const
> > char *da }
> > free(path);
> >
> > - if (asprintf(&path, "%s/%s_indx.db", datadir, ns->suffix)
> > == -1)
> > + if (asprintf(&path, "%s/%s_indx.db", conf->datadir,
> > ns->suffix) == -1) return -1;
> > if (compact_db(path) != 0) {
> > log_warn("%s", path);
> > @@ -123,14 +123,14 @@ compact_namespace(struct namespace *ns, const
> > char *da }
> >
> > int
> > -compact_namespaces(const char *datadir)
> > +compact_namespaces()
> > {
> > struct namespace *ns;
> >
> > TAILQ_FOREACH(ns, &conf->namespaces, next) {
> > if (SLIST_EMPTY(&ns->referrals))
> > continue;
> > - if (compact_namespace(ns, datadir) != 0)
> > + if (compact_namespace(ns) != 0)
> > return -1;
> > }
> >
> > @@ -138,7 +138,7 @@ compact_namespaces(const char *datadir)
> > }
> >
> > int
> > -index_namespace(struct namespace *ns, const char *datadir)
> > +index_namespace(struct namespace *ns)
> > {
> > struct btval key, val;
> > struct btree *data_db, *indx_db;
> > @@ -150,14 +150,14 @@ index_namespace(struct namespace *ns, const
> > char *data
> > log_info("indexing namespace %s", ns->suffix);
> >
> > - if (asprintf(&path, "%s/%s_data.db", DATADIR, ns->suffix)
> > == -1)
> > + if (asprintf(&path, "%s/%s_data.db", conf->datadir,
> > ns->suffix) == -1) return -1;
> > data_db = btree_open(path, BT_NOSYNC | BT_REVERSEKEY,
> > 0644); free(path);
> > if (data_db == NULL)
> > return -1;
> >
> > - if (asprintf(&path, "%s/%s_indx.db", datadir, ns->suffix)
> > == -1)
> > + if (asprintf(&path, "%s/%s_indx.db", conf->datadir,
> > ns->suffix) == -1) return -1;
> > indx_db = btree_open(path, BT_NOSYNC, 0644);
> > free(path);
> > @@ -219,14 +219,14 @@ index_namespace(struct namespace *ns, const
> > char *data }
> >
> > int
> > -index_namespaces(const char *datadir)
> > +index_namespaces()
> > {
> > struct namespace *ns;
> >
> > TAILQ_FOREACH(ns, &conf->namespaces, next) {
> > if (SLIST_EMPTY(&ns->referrals))
> > continue;
> > - if (index_namespace(ns, datadir) != 0)
> > + if (index_namespace(ns) != 0)
> > return -1;
> > }
> >
> > @@ -247,7 +247,6 @@ main(int argc, char *argv[])
> > ssize_t n;
> > int ch;
> > enum action action = NONE;
> > - const char *datadir = DATADIR;
> > struct stat sb;
> > const char *sock = LDAPD_SOCKET;
> > char *conffile = CONFFILE;
> > @@ -262,9 +261,6 @@ main(int argc, char *argv[])
> > case 'f':
> > conffile = optarg;
> > break;
> > - case 'r':
> > - datadir = optarg;
> > - break;
> > case 's':
> > sock = optarg;
> > break;
> > @@ -282,11 +278,6 @@ main(int argc, char *argv[])
> > if (argc == 0)
> > usage();
> >
> > - if (stat(datadir, &sb) == -1)
> > - err(1, "%s", datadir);
> > - if (!S_ISDIR(sb.st_mode))
> > - errx(1, "%s is not a directory", datadir);
> > -
> > ldap_loginit(NULL, 1, verbose);
> >
> > if (strcmp(argv[0], "stats") == 0)
> > @@ -311,13 +302,22 @@ main(int argc, char *argv[])
> > if (parse_config(conffile) != 0)
> > exit(2);
> >
> > + if (conf->datadir == NULL) {
> > + if (asprintf(&conf->datadir, "%s",
> > LDAPD_DATADIR) == -1)
> > + errx(1, "malloc failed");
> > + }
> > + if (stat(conf->datadir, &sb) == -1)
> > + err(1, "%s", conf->datadir);
> > + if (!S_ISDIR(sb.st_mode))
> > + errx(1, "%s is not a directory",
> > conf->datadir); +
> > if (pledge("stdio rpath wpath cpath flock", NULL)
> > == -1) err(1, "pledge");
> >
> > if (action == COMPACT_DB)
> > - return compact_namespaces(datadir);
> > + return compact_namespaces();
> > else
> > - return index_namespaces(datadir);
> > + return index_namespaces();
> > }
> >
> > /* connect to ldapd control socket */
> > blob - db5a0be5ad93c11c0dd773d342ea131328dcaa82
> > blob + 8588bbe3dc5ffd81b91967cf809681b1998cbd69
> > --- usr.sbin/ldapd/ldapd.8
> > +++ usr.sbin/ldapd/ldapd.8
> > @@ -27,7 +27,6 @@
> > .Fl D Ar macro Ns = Ns Ar value
> > .Oc
> > .Op Fl f Ar file
> > -.Op Fl r Ar directory
> > .Op Fl s Ar file
> > .Sh DESCRIPTION
> > .Nm
> > @@ -61,11 +60,6 @@ as the configuration file, instead of the default
> > .It Fl n
> > Configtest mode.
> > Only check the configuration file for validity.
> > -.It Fl r Ar directory
> > -Store and read database files in
> > -.Ar directory ,
> > -instead of the default
> > -.Pa /var/db/ldap .
> > .It Fl s Ar file
> > Specify an alternative location for the socket file.
> > .It Fl v
> > blob - 797a36f89f33233c2d9655a307ec5e727b9fbaa1
> > blob + 69ae065ae99ba9ab326afd1fe0db5eeb820ea629
> > --- usr.sbin/ldapd/ldapd.c
> > +++ usr.sbin/ldapd/ldapd.c
> > @@ -55,7 +55,6 @@ static pid_t start_child(enum
> > ldapd_process, char *,
> > struct ldapd_stats stats;
> > pid_t ldape_pid;
> > -const char *datadir = DATADIR;
> >
> > void
> > usage(void)
> > @@ -63,7 +62,7 @@ usage(void)
> > extern char *__progname;
> >
> > fprintf(stderr, "usage: %s [-dnv] [-D macro=value] "
> > - "[-f file] [-r directory] [-s file]\n", __progname);
> > + "[-f file] [-s file]\n", __progname);
> > exit(1);
> > }
> >
> > @@ -151,9 +150,6 @@ main(int argc, char *argv[])
> > case 'n':
> > configtest = 1;
> > break;
> > - case 'r':
> > - datadir = optarg;
> > - break;
> > case 's':
> > csockpath = optarg;
> > break;
> > @@ -193,15 +189,20 @@ main(int argc, char *argv[])
> > exit(0);
> > }
> >
> > + if (conf->datadir == NULL) {
> > + if (asprintf(&conf->datadir, "%s", LDAPD_DATADIR)
> > == -1)
> > + errx(1, "malloc failed");
> > + }
> > +
> > log_init(debug, LOG_DAEMON);
> >
> > if (eflag)
> > ldape(debug, verbose, csockpath);
> >
> > - if (stat(datadir, &sb) == -1)
> > - err(1, "%s", datadir);
> > + if (stat(conf->datadir, &sb) == -1)
> > + err(1, "%s", conf->datadir);
> > if (!S_ISDIR(sb.st_mode))
> > - errx(1, "%s is not a directory", datadir);
> > + errx(1, "%s is not a directory", conf->datadir);
> >
> > if (!debug) {
> > if (daemon(1, 0) == -1)
> > @@ -213,7 +214,7 @@ main(int argc, char *argv[])
> > if (socketpair(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC |
> > SOCK_NONBLOCK, PF_UNSPEC, pipe_parent2ldap) != 0)
> > fatal("socketpair");
> > -
> > +
> > ldape_pid = start_child(PROC_LDAP_SERVER, saved_argv0,
> > pipe_parent2ldap[1], debug, verbose, csockpath,
> > conffile);
> > @@ -392,7 +393,7 @@ ldapd_open_request(struct imsgev *iev, struct
> > imsg *im /* make sure path is null-terminated */
> > oreq->path[PATH_MAX] = '\0';
> >
> > - if (strncmp(oreq->path, datadir, strlen(datadir)) != 0) {
> > + if (strncmp(oreq->path, conf->datadir,
> > strlen(conf->datadir)) != 0) { log_warnx("refusing to open file
> > %s", oreq->path); fatal("ldape sent invalid open request");
> > }
> > @@ -459,7 +460,7 @@ start_child(enum ldapd_process p, char *argv0,
> > int fd, argv[argc++] = "-f";
> > argv[argc++] = conffile;
> > }
> > -
> > +
> > argv[argc++] = NULL;
> >
> > execvp(argv0, argv);
> > blob - 3da0137e137e8b3bc64f6351579fcd6a9344b1f8
> > blob + 31c894c3e39ab70f3324415ecac5efc1301e0a54
> > --- usr.sbin/ldapd/ldapd.conf.5
> > +++ usr.sbin/ldapd/ldapd.conf.5
> > @@ -130,6 +130,11 @@ The URL has the following format:
> > ldap://ldap.example.com
> > ldaps://ldap.example.com:3890
> > .Ed
> > +.It datadir Ar directory
> > +Store and read database files in
> > +.Ar directory ,
> > +instead of the default
> > +.Pa /var/db/ldap .
> > .It rootdn Ar dn
> > Specify the distinguished name of the root user for all namespaces.
> > The root user is always allowed to read and write entries in all
> > blob - 3f995d184b442f094a95f089e3bb4a727eabf559
> > blob + 2736d366bf5f92e3579f915f4ceb2a3610d27d54
> > --- usr.sbin/ldapd/ldapd.h
> > +++ usr.sbin/ldapd/ldapd.h
> > @@ -41,7 +41,7 @@
> > #define CONFFILE "/etc/ldapd.conf"
> > #define LDAPD_USER "_ldapd"
> > #define LDAPD_SOCKET "/var/run/ldapd.sock"
> > -#define DATADIR "/var/db/ldap"
> > +#define LDAPD_DATADIR "/var/db/ldap"
> > #define LDAP_PORT 389
> > #define LDAPS_PORT 636
> > #define LDAPD_SESSION_TIMEOUT 30
> > @@ -252,6 +252,7 @@ struct ldapd_config
> > struct schema *schema;
> > char *rootdn;
> > char *rootpw;
> > + char *datadir;
> > };
> >
> > struct ldapd_stats
> > blob - 2259f57bde3c2918a8b200747c194e5768ae40f1
> > blob + 2e39abf6cc94ac6f301876df996a6229a032a7b7
> > --- usr.sbin/ldapd/namespace.c
> > +++ usr.sbin/ldapd/namespace.c
> > @@ -29,8 +29,6 @@
> > #include "ldapd.h"
> > #include "log.h"
> >
> > -extern const char *datadir;
> > -
> > /* Maximum number of requests to queue per namespace during
> > compaction.
> > * After this many requests, we return LDAP_BUSY.
> > */
> > @@ -118,7 +116,8 @@ namespace_open(struct namespace *ns)
> > if (ns->sync == 0)
> > db_flags |= BT_NOSYNC;
> >
> > - if (asprintf(&ns->data_path, "%s/%s_data.db", datadir,
> > ns->suffix) == -1)
> > + if (asprintf(&ns->data_path, "%s/%s_data.db",
> > conf->datadir,
> > + ns->suffix) == -1)
> > return -1;
> > log_info("opening namespace %s", ns->suffix);
> > ns->data_db = btree_open(ns->data_path, db_flags |
> > BT_REVERSEKEY, 0644); @@ -127,7 +126,8 @@ namespace_open(struct
> > namespace *ns)
> > btree_set_cache_size(ns->data_db, ns->cache_size);
> >
> > - if (asprintf(&ns->indx_path, "%s/%s_indx.db", datadir,
> > ns->suffix) == -1)
> > + if (asprintf(&ns->indx_path, "%s/%s_indx.db",
> > conf->datadir,
> > + ns->suffix) == -1)
> > return -1;
> > ns->indx_db = btree_open(ns->indx_path, db_flags, 0644);
> > if (ns->indx_db == NULL)
> > blob - bad9bc630403e5280e3b0db41d8ec1247db70352
> > blob + b72e4d89324b332498f8e5e0d1d8db822d4588f9
> > --- usr.sbin/ldapd/parse.y
> > +++ usr.sbin/ldapd/parse.y
> > @@ -117,7 +117,7 @@ static struct namespace *current_ns = NULL;
> > %}
> >
> > %token ERROR LISTEN ON TLS LDAPS PORT NAMESPACE ROOTDN
> > ROOTPW INDEX -%token SECURE RELAX STRICT SCHEMA USE
> > COMPRESSION LEVEL +%token SECURE RELAX STRICT SCHEMA USE
> > COMPRESSION LEVEL DATADIR %token INCLUDE CERTIFICATE FSYNC
> > CACHE_SIZE INDEX_CACHE_SIZE %token DENY ALLOW READ WRITE
> > BIND ACCESS TO ROOT REFERRAL %token ANY CHILDREN OF
> > ATTRIBUTE IN SUBTREE BY SELF @@ -225,6 +225,7 @@ conf_main :
> > LISTEN ON STRING port ssl certname {
> > normalize_dn(conf->rootdn); }
> > | ROOTPW STRING {
> > conf->rootpw = $2; }
> > + | DATADIR STRING { conf->datadir =
> > $2; } ;
> >
> > namespace : NAMESPACE STRING '{' '\n' {
> > @@ -441,6 +442,7 @@ lookup(char *s)
> > { "certificate", CERTIFICATE },
> > { "children", CHILDREN },
> > { "compression", COMPRESSION },
> > + { "datadir", DATADIR },
> > { "deny", DENY },
> > { "fsync", FSYNC },
> > { "in", IN },
> >
