Created attachment 663134 [details]
sample test application
Description of problem:
I'm told that when the cracklib dictionary is not present (due to it being called while in a chroot environment that doesn't yet have the dictionary files available), anaconda, by way of libpwquality, is hitting an exit() in cracklib's FascistCheck() function.
If you open the dictionary directly using PWOpen(), you can decide how to handle an error there, and then use the FascistLook() function to perform the check using the dictionary handle.
While the FascistLook() symbol has been exported by the shared library for quite a while, it wasn't declared in any public headers. The cracklib upstream maintainer was kind enough to fix that for cracklib 2.8.21.
Version-Release number of selected component (if applicable):
Steps to Reproduce:
1. Build the attached chkchroot.c, linked against libcrack.
2. ( echo samplepassword1 ; echo samplepassword2 ) | sudo ./chkchroot
This will use FascistCheck().
3. ( echo samplepassword1 ; echo samplepassword2 ) | sudo ./chkchroot -
This will use PWOpen()/FascistLook()/PWClose().
Command 2 will just print an error, as the test program exits.
Command 3 will print "accepted" / "rejected" status for both passwords.
Created attachment 663135 [details]
suggested libpwquality changes
Line 676 of the patch should probably be "*auxerror = strerror(errno);" instead.
I am not too fond of copying code that is in the cracklib. Note that there are other users of cracklib (namely pam_cracklib) that also would not appreciate exit() in the middle of pam stack processing. This exit() call should be really removed from FascistCheck() anyway.
So in my opinion fixing cracklib directly (returning "missing dictionary" error is really preferable to workarounding it in libpwquality.
I don't intend to push for a change in library behavior here -- it's not as if the exit-on-error is new there.
It basically did not matter because nobody ever called cracklib in situation where the dictionary was not available. But Nalin, do you really think that calling exit() in the middle of library call is a reasonable thing to do when returning error instead is fully sensible and safe.
In general, I don't.
But at this point that's been the library's behavior for longer than some of the things which use it have even existed. I'm not a fan of breaking expectations about that which can have developed -- and for good or bad, that's a major change to how that function does things. The entry points which I'm suggesting you use have been accessible (though one wasn't declared) literally for years, which means that using them also avoids creating a version dependency on your end.
If we were talking about RHEL update release I could understand that.
But on Fedora? Nalin, this is really silly. Can you give an example of such expectation? Who could expect having an application 'crash' (I know that it is not a real crash but exit.) when the dictionary is not available instead of a message 'fatal error - dictionary missing' or similar.
BTW, here is the list of packages that require cracklib - can you name an example where aborting would make more sense than the message above?
repoquery --whatrequires 'libcrack.so.2()(64bit)'
repoquery --whatrequires 'cracklib-python'
Also I didn't before know that the exit() call is there. But now I know it is there and if you really refuse to remove it - it means that many of the applications above should be fixed and the code copied in them. At least pam_cracklib definitely should be as calling exit() in middle of a pam stack is nonsense.
I can't claim to know what every caller, in Fedora or not, does or expects.
You gave me the impression that a quick answer was preferable for the F18 schedule, so a workaround that you could use that didn't require blocking on changes to any other package seemed ideal.
But alright, I'll see what upstream thinks.
Actually neither fix is really going to work for the F18 anaconda problem. Either the problem needs to be fixed in anaconda directly or as a workaround libpwquality would have to make the cracklib check optional and anaconda would have to explicitly switch it off.
Upstream change the behavior to be more in line with that specified in comment 3. It seems closed->won't-fix is appropriate here.