Bug 886995 - open the cracklib dictionary directly to control how to handle errors
Summary: open the cracklib dictionary directly to control how to handle errors
Alias: None
Product: Fedora
Classification: Fedora
Component: libpwquality
Version: 18
Hardware: Unspecified
OS: Unspecified
Target Milestone: ---
Assignee: Tomas Mraz
QA Contact: Fedora Extras Quality Assurance
Depends On:
TreeView+ depends on / blocked
Reported: 2012-12-13 19:02 UTC by Nalin Dahyabhai
Modified: 2012-12-17 18:02 UTC (History)
1 user (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Last Closed: 2012-12-17 18:02:51 UTC
Type: Bug

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

System ID Private Priority Status Summary Last Updated
Red Hat Bugzilla 876716 0 unspecified CLOSED anaconda crashes after setting root password 2021-02-22 00:41:40 UTC

Internal Links: 876716

Description Nalin Dahyabhai 2012-12-13 19:02:22 UTC
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 19:03:17 UTC
Created attachment 663135 [details]
suggested libpwquality changes

Comment 2 Nalin Dahyabhai 2012-12-13 19:54:03 UTC
Line 676 of the patch should probably be "*auxerror = strerror(errno);" instead.

Comment 3 Tomas Mraz 2012-12-14 09:27:42 UTC
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 15:49:24 UTC
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 15:53:38 UTC
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 18:46:23 UTC
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 21:01:39 UTC
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 21:35:09 UTC
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 21:58:59 UTC
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 15:41:26 UTC
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 18:02:51 UTC
Thank you

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