Re: crypt_r()?
> On Feb 15, 2022, at 5:13 PM, Mouse wrote: > >> There really should be a function that takes a user name or ID and a clearte$ > > Maybe. But then you have a lot more failure modes and a lot more > possible attack surface. It would also mean that you can't check or > change passwords in single-user mode without starting the magic daemon; > that would be a substantial regression as far as user experience goes, > if nothing else. And what about checking the root password for > single-user boot with insecure console? You put the fallback logic into the function libc that can satisfy the request using the Old Way if the helper isn’t available. Obviously, to satisfy it the Old Way, the process would need to have root privileges, but this would be OK in the scenario you’re describing. > It _is_, however, very much in keeping with the "encapsulate > single-purpose code into a single place" attitude that has brought a > lot of benefits. I wonder if there isn't some better way I'm missing. It’s certainly a lot better than having a big complex program (that exposes itself to the network, potentially) require root privileges just to check passwords. -- thorpej
Re: crypt_r()?
On 2/15/2022 5:04 PM, Mouse wrote: (2) Hashing password, which takes the password and the settings and returns an allocated string with the resulting hash. [...] I really don't like making them depend on malloc, though I have a hard time articulating what bothers me about it. I can't say what bothers *you* about it :^) but what bothers me is that it makes it impossible to pre-allocate memory for the purpose. This might be a concern, for example, on a system with no swap. Combine that with a need for thread safety, say for a threaded login daemon on a system with no swap, and you get crypt_r(3) as described. Of course the clearest use-case for crypt_r(3) is a password cracker: each thread sets up its own local memory and blasts through calls to crypt_r(3) as fast as it can. I've run a cracker as a white-hat. But I can see not wanting to add the capability to base if that's the only convincing use case. Take care, Konrad Schroder perse...@.org
Re: crypt_r()?
> There really should be a function that takes a user name or ID and a clearte$ Maybe. But then you have a lot more failure modes and a lot more possible attack surface. It would also mean that you can't check or change passwords in single-user mode without starting the magic daemon; that would be a substantial regression as far as user experience goes, if nothing else. And what about checking the root password for single-user boot with insecure console? It _is_, however, very much in keeping with the "encapsulate single-purpose code into a single place" attitude that has brought a lot of benefits. I wonder if there isn't some better way I'm missing. /~\ The ASCII Mouse \ / Ribbon Campaign X Against HTMLmo...@rodents-montreal.org / \ Email! 7D C8 61 52 5D E7 2D 39 4E F1 31 3E E8 B3 27 4B
Re: crypt_r()?
> There are two sensible interface contracts here: > (1) Verification only, which takes the password and the expected hash > and returns a bool. [...] > (2) Hashing password, which takes the password and the settings and > returns an allocated string with the resulting hash. [...] Well, I disagree about the "allocated" part; I think there is a place for returning into a buffer specified by the caller. But, more what's leading me to write now: if you go with (1), you still need something like (2) in order to _set_ passwords. (2) - or something like (2) into a caller-specified string - can be used for either checking or setting. (1) can't. > Given that the goal of the crypt(3) interface is to be slow, > optimizing a memory allocation away saves nothing, except making a > more complicated interface. I disagree. It saves a bunch of error paths. Many (most? all?) of the password hashing algorithms run, or can be made to run, without needing any heap memory at all, if you use a caller-provided buffer for the returned string. I really don't like making them depend on malloc, though I have a hard time articulating what bothers me about it. /~\ The ASCII Mouse \ / Ribbon Campaign X Against HTMLmo...@rodents-montreal.org / \ Email! 7D C8 61 52 5D E7 2D 39 4E F1 31 3E E8 B3 27 4B
Re: crypt_r()?
> On Feb 15, 2022, at 3:30 PM, Joerg Sonnenberger wrote: > > Am Wed, Feb 16, 2022 at 12:04:16AM +0100 schrieb Niclas Rosenvik: >> 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? > > No. There are two sensible interface contracts here: > > (1) Verification only, which takes the password and the expected hash > and returns a bool. No separate settings necessary as the hash already > contains all the necessary parameters. > > (2) Hashing password, which takes the password and the settings and > returns an allocated string with the resulting hash. This is essentially > the same interface as crypt(3), but the caller is responsible for > free(3) the return value. Given that the goal of the crypt(3) interface > is to be slow, optimizing a memory allocation away saves nothing, except > making a more complicated interface. While we’re on the topic of “what we’d like the interfaces to do”… There really should be a function that takes a user name or ID and a cleartext password string, and IPCs to another (trusted system) process to do the verification, so that programs that want to verify passwords don’t need root privileges. Same for changing passwords. Then there’s no need for any of this crypt(3) tomfoolery. -- thorpej
Re: crypt_r()?
Am Wed, Feb 16, 2022 at 12:04:16AM +0100 schrieb Niclas Rosenvik: > 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? No. There are two sensible interface contracts here: (1) Verification only, which takes the password and the expected hash and returns a bool. No separate settings necessary as the hash already contains all the necessary parameters. (2) Hashing password, which takes the password and the settings and returns an allocated string with the resulting hash. This is essentially the same interface as crypt(3), but the caller is responsible for free(3) the return value. Given that the goal of the crypt(3) interface is to be slow, optimizing a memory allocation away saves nothing, except making a more complicated interface. Joerg
Re: crypt_r()?
On Sat, 12 Feb 2022 23:32:31 +0100 Joerg Sonnenberger 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 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
