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?
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 },
>
