[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: GDBM/DB password file support



Elliot Lee wrote:
> A few comments:
> 
> - The config file format probably should be a bit more flexible.

Probably.

> 	I.e.:
> 
> 	CONFIGFILE: dblist
> 		| CONFIGFILE dblist
> 	dblist: 'databasename' ':' whitespace sourcelist
> 	sourcelist: 'sourcename'
> 		| sourcelist whitespace 'sourcename'
> 	whitespace: '[ \t\n\r]+'

sourcename is currently a list of databases that are merged (with '+').
For example,

	user:	nis+shadow
		unix+shadow

would indicate that the default setup is for NIS users whose passwords may
optionally be located in the /etc/shadow file, but if users are not located
in nis then look for them in the local /etc/passwd + shadow files.

> That way people could do syntax like /etc/nsswitch.conf if they wanted, or
> put one thing per line if they wanted. Also it says

The above is equivalent to

	user: nis+shadow unix+shadow

and

	user: 	nis
		+shadow
	     	unix+

		shadow

[But it doesn't look as nice]

> "The nameN items being one of the following: nis, unix, radius and shadow."
> That should probably just be changed to "The nameN items being the data
> source for the database in question. Examples are nis, unix, radius, and
> shadow".

Fine. This is a beter set of words...

> - A DNS module will be needed eventually, for host map lookups...? (Or are
> we only doing user-database things here?) Also a hesiod module (hesiod
> basically uses DNS to store the passwd & group maps). It might make sense
> not to restrict this API to just user & group lookups.

Initially, the library was intended to only support user and group info.
[Actually, the group stuff has not been implemented yet. So it is still not
inconvenient to add this level of generality. It is something to look into]

> - The memory allocation on the user's side of things looks kind of messy.
> Any way to hide that? The sample code didn't seem to need it, though, so
> that may not be an issue (yet).

The application using the library should not need to call malloc at all. The
library dynamically allocates everything and the call to pwdb_end() deletes
and free()'s it all. Actually, because everything the app sees is a const
pointer, this aspect of the library should be very clean.

> - Things from different sources are seeming to be a bit complicated. I.e.
> the stipulations that you can't ask for this info from this module, and
> you can't use a UID to get info from raduis, for example. Why not just

specific info can be requested from individual databases with pwdb_request

> have a password structure that holds all the elements of information one
> could possibly have on a user or group (or an associative array type of
> thing). The application could fill in whatever information it has on the
> user (i.e. if it has a UID, pass UID in, if it has username and GECOS pass
> that in). Then libpwdb would go through the modules one by one getting
> them to fill in whatever information they have on the user, and out comes
> a nicely populated user/group information structure. This would have

This is what happens as the library probes each of the '+' separated
datbases.

> problems if two different user databases had conflicting entries for a
> user, but this is likely to happen only for authentication tokens, which
> PAM modules will take care of anyways.

The later databases overwrite the entries of the earlier ones. To overcome
this problem in the case of "passwd" entries, we are placing "x" entries in
the databases that do not contain the password (actually, "x" is the "passwd
is in the shadow file" entry and other characters will be used for other
databases.)

> libpwdb modules could also each return a value that says whether they
> found the user in their database or not. If none of the modules find a
> user, then libpwdb returns back to the program with PWDB_NOT_FOUND

Currently, if the first database in a "+" list does not contain the user,
the corresponding list is ignored and the next list is searched instead.
Only if none of the user: lists contain an entry for that user is
PWDB_NOT_FOUND returned.

> <My too-long $0.02 ends> Comments/flames welcome.

I guess I just doubled its length.. Sorry.

I like the idea of increasing the types of database available.. This will
require reorganizing the generic functions a little but its a good thing to
do it sooner rather than later..

Thanks!

Andrew



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index] []