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.
+ switch (kerr) {
+ case KRB5_KDC_UNREACH:
+ pam_status = PAM_AUTHINFO_UNAVAIL;
+ break;
+ case KRB5KDC_ERR_KEY_EXP:
+ pam_status = PAM_NEW_AUTHTOK_REQD;
+ break;
+ case KRB5KRB_AP_ERR_BAD_INTEGRITY:
+ pam_status = PAM_AUTH_ERR;
+ break;
+ case KRB5KDC_ERR_PREAUTH_FAILED:
+ pam_status = PAM_CRED_ERR;
+ break;
+ default:
+ pam_status = PAM_SYSTEM_ERR;
+ break;
+ }
+
+ return pam_status;
+}