On Thu, Sep 20, 2012 at 10:04:02AM +0200, Pavel Březina wrote:
On 09/17/2012 08:29 PM, Jakub Hrozek wrote:
>On Fri, Sep 14, 2012 at 05:34:17PM +0200, Joschi Brauchle wrote:
>>>Hallo Jakub,
>>>
>>>I have tested the latest patch successfully.
>>>
>>>Please note that in the 2nd hunk,
>>>1) the line with "KRB5_DEBUG(1, kerr);" should also be removed to
>>>avoid duplicate debug messages and
>>>2) you could directly call "kerr_handle_error" instead of
>>>"kerr_to_status" (as kerr != 0).
>>>
>>>See current version:
>>>------------
>>>@@ -777,9 +812,7 @@ static errno_t changepw_child(int fd, struct
>>>krb5_req *kr)
>>> kr->options);
>>> if (kerr != 0) {
>>> KRB5_DEBUG(1, kerr);
>>>- if (kerr == KRB5_KDC_UNREACH) {
>>>- pam_status = PAM_AUTHINFO_UNAVAIL;
>>>- }
>>>+ pam_status = kerr_to_status(kerr);
>>> goto sendresponse;
>>> }
>>>------------
>>>
>>>But those are just minor cosmetic problems and do not affect the
>>>functionality of the patch.
>>>
>>>Best regards,
>>>Joschi Brauchle
>>>
>Thank you for the careful review, Joschi.
>
>An updated patch that fixes the two items pointed out above is attached.
Nack (nitpick).
>0001-KRB5-Return-PAM_AUTH_ERR-on-incorrect-password.patch
>
>+static int kerr_handle_error(krb5_error_code kerr)
>+{
>+ int pam_status;
>+
>+ KRB5_DEBUG(1, kerr);
You should use SSSDBG macro here.
Thanks, a new patch is attached.