Bug 886995 - open the cracklib dictionary directly to control how to handle errors
open the cracklib dictionary directly to control how to handle errors
Product: Fedora
Classification: Fedora
Component: libpwquality (Show other bugs)
Unspecified Unspecified
unspecified Severity unspecified
: ---
: ---
Assigned To: Tomas Mraz
Fedora Extras Quality Assurance
Depends On:
  Show dependency treegraph
Reported: 2012-12-13 14:02 EST by Nalin Dahyabhai
Modified: 2012-12-17 13:02 EST (History)
1 user (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Last Closed: 2012-12-17 13:02:51 EST
Type: Bug
Regression: ---
Mount Type: ---
Documentation: ---
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---

Attachments (Terms of Use)
sample test application (1.21 KB, text/plain)
2012-12-13 14:02 EST, Nalin Dahyabhai
no flags Details
suggested libpwquality changes (2.45 KB, patch)
2012-12-13 14:03 EST, Nalin Dahyabhai
no flags Details | Diff

  None (edit)
Description Nalin Dahyabhai 2012-12-13 14:02:22 EST
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):

How reproducible:

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().
Actual results:

Command 2 will just print an error, as the test program exits.

Expected results:

Command 3 will print "accepted" / "rejected" status for both passwords.
Comment 1 Nalin Dahyabhai 2012-12-13 14:03:17 EST
Created attachment 663135 [details]
suggested libpwquality changes
Comment 2 Nalin Dahyabhai 2012-12-13 14:54:03 EST
Line 676 of the patch should probably be "*auxerror = strerror(errno);" instead.
Comment 3 Tomas Mraz 2012-12-14 04:27:42 EST
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.
Comment 4 Nalin Dahyabhai 2012-12-14 10:49:24 EST
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.
Comment 5 Tomas Mraz 2012-12-14 10:53:38 EST
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.
Comment 6 Nalin Dahyabhai 2012-12-14 13:46:23 EST
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.
Comment 7 Tomas Mraz 2012-12-14 16:01:39 EST
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.
Comment 8 Nalin Dahyabhai 2012-12-14 16:35:09 EST
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.
Comment 9 Tomas Mraz 2012-12-14 16:58:59 EST
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.
Comment 10 Nalin Dahyabhai 2012-12-17 10:41:26 EST
Upstream change the behavior to be more in line with that specified in comment 3.  It seems closed->won't-fix is appropriate here.
Comment 11 Tomas Mraz 2012-12-17 13:02:51 EST
Thank you

Note You need to log in before you can comment on or make changes to this bug.