Bug 231093

Created attachment 149315 [details]
cvs diff dblayer.c

File: ldap/servers/slapd/back-ldbm/dblayer.c

Problem description: 
The server instance home directory in the dblayer private structure
(li->li_dblayer_private) is empty.  Either of these fields is supposed to be
set:
  dblayer_home_directory = 0x0
  dblayer_dbhome_directory = 0x8125ba0 ""

Fix descrption: if the fields are not set, issues an error message and returns.

In this code:
https://bugzilla.redhat.com/bugzilla/attachment.cgi?id=149315&action=diff#dblayer.c_sec3

Do you need to initialize inst->inst_parent_dir_name, or is it already
initialized to NULL?

In this code:
https://bugzilla.redhat.com/bugzilla/attachment.cgi?id=149315&action=diff#dblayer.c_sec5

Do you need to initialize priv->dblayer_home_directory to NULL?

In this code:
https://bugzilla.redhat.com/bugzilla/attachment.cgi?id=149315&action=diff#dblayer.c_sec6

Do you need to initialize envdir to NULL?

In this code:
https://bugzilla.redhat.com/bugzilla/attachment.cgi?id=149315&action=diff#dblayer.c_sec12

Do you need to initialize dirhandle to NULL?

Thanks for the comments, Rich.

(In reply to comment #2)
> In this code:
>
https://bugzilla.redhat.com/bugzilla/attachment.cgi?id=149315&action=diff#dblayer.c_sec3
>
> Do you need to initialize inst->inst_parent_dir_name, or is it already
> initialized to NULL?

The structure ldbm_instance (inst) is allocated in ldbm_instance_create, where
the field is initialized to NULL.

> In this code:
>
https://bugzilla.redhat.com/bugzilla/attachment.cgi?id=149315&action=diff#dblayer.c_sec5
>
> Do you need to initialize priv->dblayer_home_directory to NULL?

The structure "dblayer_private" is allocated in dblayer_init, where the field is
initialized to NULL.  And the attribute value of nsslapd-directory is set in
dblayer_start.

> In this code:
>
https://bugzilla.redhat.com/bugzilla/attachment.cgi?id=149315&action=diff#dblayer.c_sec6
>
> Do you need to initialize envdir to NULL?

I think it is.
   2269 int dblayer_release_aux_id2entry(backend *be, DB *pDB, DB_ENV *pEnv)
   2270 {
   2271     ldbm_instance *inst;
   2272     char *envdir = NULL;
    [...]
   2284     inst_dirp = dblayer_get_full_inst_dir(inst->inst_li, inst,
   2285                                           inst_dir, MAXPATHLEN);
   2286     if (inst_dirp && *inst_dirp)
   2287     {
   2288         envdir = slapi_ch_smprintf("%s/dbenv", inst_dirp);
   2289     }

> In this code:
>
https://bugzilla.redhat.com/bugzilla/attachment.cgi?id=149315&action=diff#dblayer.c_sec12
>
> Do you need to initialize dirhandle to NULL?

I think it is.
   4111 static int _dblayer_delete_instance_dir(ldbm_instance *inst, int startdb
       )
   4112 {
   4113     PRDir *dirhandle = NULL;
    [...]
   4146     inst_dirp = dblayer_get_full_inst_dir(li, inst, inst_dir, MAXPATHLEN
       );
   4147     if (inst_dirp && *inst_dirp) {
   4148         dirhandle = PR_OpenDir(inst_dirp);
   4149     }

Checked in with 232050: Change format of DBVERSION and guardian files
Checking in dblayer.c;
/cvs/dirsec/ldapserver/ldap/servers/slapd/back-ldbm/dblayer.c,v  <--  dblayer.c
new revision: 1.18; previous revision: 1.17
done

Steps to reproduce:
1. stop the server
2. edit dse.ldif to remove nsslapd-directory and nsslapd-db-home-directory.
3. run db2bak

Note: the server's owner should be non root (e.g., nobody)

If the command line fails with errors as follows instead of crashes, it's okay.
[...]
[...] - WARNING---no write permission to file /usr/sbin//vgexport
[...] - libdb: /usr/sbin/__db.001: Permission denied
[...] - Opening database environment (/usr/sbin/) failed. err=13: Permission denied
[...] - db2archive: Failed to init database

Tried the above steps. when I run db2bak, I get these messages.

[root@colossus slapd-colossus]# ./db2bak
Back up directory: /var/lib/dirsrv/slapd-colossus/bak/colossus-2007_11_29_14_03_58
[29/Nov/2007:14:03:58 -0800] - WARNING---no write permission to file
/usr/sbin//amreport
[29/Nov/2007:14:03:58 -0800] - WARNING---no write permission to file
/usr/sbin//tcpslice
[29/Nov/2007:14:03:59 -0800] - WARNING---no write permission to file
/usr/sbin//semodule
[29/Nov/2007:14:03:59 -0800] - WARNING---no write permission to file
/usr/sbin//named-checkconf
[29/Nov/2007:14:03:59 -0800] - WARNING---no write permission to file
/usr/sbin//genhomedircon
[29/Nov/2007:14:03:59 -0800] - WARNING---no write permission to file /usr/sbin//ripd
[29/Nov/2007:14:03:59 -0800] - WARNING---no write permission to file
/usr/sbin//lockdev
[29/Nov/2007:14:03:59 -0800] - WARNING---no write permission to file
/usr/sbin//postqueue
[29/Nov/2007:14:03:59 -0800] - WARNING---no write permission to file
/usr/sbin//foomatic-ppdload
[29/Nov/2007:14:03:59 -0800] - WARNING---no write permission to file
/usr/sbin//setup-ds-admin.pl

...
...

[29/Nov/2007:14:04:00 -0800] - libdb: /usr/sbin/__db.001: Permission denied
[29/Nov/2007:14:04:00 -0800] - Opening database environment (/usr/sbin/) failed.
err=13: Permission denied
[29/Nov/2007:14:04:00 -0800] - db2archive: Failed to init database

######################################################################
db2bak is attempting to write to each file in /usr/sbin/. that seems
wrong to me. 

if the variable nsslapd-directory is not set in dse.ldif, db2bak 
should print out a message saying, this variable is not set (OR)
dont have permission to write to value defined in nsslapd-directory
and quit.

Created attachment 276401 [details]
cvs diffs

Files:
 ldbm_config.h
 ldbm_instance_config.c
 ldbm_config.c
 dblayer.c
 start.c

Description:
Set the strong requirement: nsslapd-directory must have some value.
to guarantee it:
1) checking errors from ldbm_config_directory_set. If the check fails, don't
start the server.
2) if nsslapd-directory does not exist or the value is empty in dse.ldif,
issuing an error message and returning the error code.
3) since it was difficult to distinguish the nsslapd-directory empty value from
the initial default value, introduced CONFIG_FLAG_SKIP_DEFAULT_SETTING flag to
tell the backend config code to skip setting the default value.

Created attachment 276431 [details]
test cases and results

If there is a problem with the entry, does it print the DN of the entry?

Good idea!  I've added the backend config entry to the error message:
[..] - ERROR: db directory is not set; check nsslapd-directory in the db config:
cn=config,cn=ldbm database,cn=plugins,cn=config

Created attachment 276451 [details]
revised ldbm_config.c

This diff includes the suggestion from Rich and PL_strcmp fix.

Created attachment 276461 [details]
cvs commit message

Thank you to Rich for the reviews and comments.

Checked in into CVS HEAD.