Re: crypt_r()?

[Niclas Rosenvik <nros%netbsd.org@localhost>]

On Sat, 12 Feb 2022 23:32:31 +0100
Joerg Sonnenberger <joerg%bec.de@localhost> wrote:

> Am Sat, Feb 12, 2022 at 05:25:11PM +0100 schrieb Niclas Rosenvik:
> > On Mon, 7 Feb 2022 16:12:17 +0100
> > Thomas Klausner <wiz%NetBSD.org@localhost> wrote:
> > 
> > > Hi!
> > > 
> > > I've been asked by the filezilla software developer if NetBSD
> > > will add crypt_r() as a thread-safe crypt() replacement.
> > > 
> > > Is anyone interested in working on this?
> > >  Thomas
> > 
> > Here is a cvs diff that implements crypt_r, as mouse pointed out
> > it is really trivial since __crypt is already essentially crypt_r.
> 
> Please don't commit a new interface when my initial question has not
> been answered. What is this interface supposed to solve? This is
> essential as a password verification interface very naturally can
> internalize all storage necessary for the different backends where as
> a contract that involves returning a "decrypted" string can not. For
> the same reason, the interface contract here is completely wrong.
> There is no need to expose the internals like this. The only
> non-thread-safe space used by crypt(3) that should ever be exposed is
> the space necessary for the return storage.
> 
> Joerg

Nothing is being committed before review and discussion and maybe not
committed all since there seems to be resistance. The first thing
crypt_r solves is that the function and its interface is used by
programs in pkgsrc and they need patching or will be missing
functionality. The static storage that you mention is considered a bug
according to NetBSD:s crypt man page. crypt_r solves this bug. You
claim that the interface is completely wrong. When you write
"expose the internals", do you mean the initialized variable in struct
crypt_data? It is unused in this implementation and is there for
compatibility with glibc. Programs written for glibc are to set
initialized to 0 before calling crypt_r the first time and by including
it in crypt_data compilation of these programs will not break because
they set initialized. __buf is the space needed for return storage or
do you mean that the interface should be 
crypt_r(const char *key, const char setting, char * storage, size_t
*storage_len)
where storage can be set to NULL to return the needed storage size in
storage_len?
I see a point with your claim about the static buffer, if a new
algorithm is introduced that needs more than 512 bytes returned from
crypt and crypt_r then the length of __buf will have to be updated for
it to work correctly so the change won't be internal to the crypt
library.

Niclas