On 06/13/2013 05:37 PM, Jakub Hrozek wrote:
On Thu, Jun 13, 2013 at 04:40:43PM +0200, Ondrej Kos wrote:
On 06/12/2013 05:51 PM, Jakub Hrozek wrote:
On Fri, May 24, 2013 at 04:52:49PM +0200, Lukas Slebodnik wrote:
ehlo,
Commit c6872e79e8496fd075e20aec0343ade99cca725c caused that password migration
doesn't work using sssd.
If pre authentication failed then we should send message to backend,
so password migration could be detected.
https://fedorahosted.org/sssd/ticket/1873
Patch is attached.
LS
Hi,
sorry the review took such a long time. However, this is not the correct
approach. If you take a look at ipa_auth.c and how the migration is
performed in src/providers/ipa/ipa_auth.c:
263 if (state->pd->cmd == SSS_PAM_AUTHENTICATE &&
264 state->pd->pam_status == PAM_CRED_ERR) {
265
266 req = get_password_migration_flag_send(state, state->ev,
The migration is attempted if the krb5_auth request returns PAM_CRED_ERR
as the PAM error. Now the PAM error is converted from an error code the
Kerberos child process sends to the krb5_auth request in
krb5_auth_done().
When a user is migrated from LDAP then the authentication would fail
with KRB5_PREAUTH_FAILED in the krb5_child process and the krb5_child
would return ERR_AUTH_FAILED (see src/providers/krb5/krb5_child.c:1201).
ERR_AUTH_FAILED is then converted in krb5_auth_done() into PAM_AUTH_ERR.
So I see two options here:
1) change the condition in ipa_auth.c from detecting PAM_CRED_ERR to
detecting PAM_AUTH_ERR. I don't like this because any authentication
failure would then check the migration flag.
2) Add a new ERR_AUTH code, maybe ERR_PREAUTH_FAILED to krb5_child,
return that instead of ERR_AUTH_FAILED from the krb5_child and
convert ERR_PREAUTH_FAILED into PAM_CRED_ERR in krb5_auth_done.
BTW to prove this was indeed the root cause, I tested by implementing 1)
option above:
diff --git a/src/providers/ipa/ipa_auth.c b/src/providers/ipa/ipa_auth.c
index 651196a..03576eb 100644
--- a/src/providers/ipa/ipa_auth.c
+++ b/src/providers/ipa/ipa_auth.c
@@ -261,7 +261,7 @@ static void ipa_auth_handler_done(struct tevent_req *req)
}
if (state->pd->cmd == SSS_PAM_AUTHENTICATE &&
- state->pd->pam_status == PAM_CRED_ERR) {
+ state->pd->pam_status == PAM_AUTH_ERR) {
req = get_password_migration_flag_send(state, state->ev,
state->ipa_auth_ctx->sdap_id_ctx,
And the migration worked like a charm.
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Hi,
Attached is a patch witch fixes this issue correctly, following Jakub's
second provided option.
The patch works fine, thank you. Some comments about the error code
below:
--- a/src/providers/krb5/krb5_auth.c
+++ b/src/providers/krb5/krb5_auth.c
@@ -1032,8 +1032,8 @@ static void krb5_auth_done(struct tevent_req *subreq)
ret = EOK;
goto done;
- case ERR_AUTH_FAILED:
- state->pam_status = PAM_AUTH_ERR;
+ case ERR_PREAUTH_FAILED:
+ state->pam_status = PAM_CRED_ERR;
state->dp_err = DP_ERR_OK;
we want to keep both cases here, ERR_AUTH_FAILED is still returned if
authentication fails with wrong password.
ret = EOK;
goto done;
diff --git a/src/providers/krb5/krb5_child.c b/src/providers/krb5/krb5_child.c
index
8f746a8db561928349ffed8b7434db2a113a1f86..dc1f06dd1e86954ca1f4c90e97d9eb95022e07fd
100644
--- a/src/providers/krb5/krb5_child.c
+++ b/src/providers/krb5/krb5_child.c
@@ -1174,7 +1174,7 @@ static errno_t map_krb5_error(krb5_error_code kerr)
case KRB5KRB_AP_ERR_BAD_INTEGRITY:
BAD_INTEGRITY should still return AUTH_FAILED.
case KRB5_PREAUTH_FAILED:
case KRB5KDC_ERR_PREAUTH_FAILED:
- return ERR_AUTH_FAILED;
+ return ERR_PREAUTH_FAILED;
default:
return ERR_INTERNAL;
diff --git a/src/util/util_errors.c b/src/util/util_errors.c
index
b617f540691a245d1132469a1f019bcb0eb6e775..096ac532fc7b9c8deb025ef9e948bc04ed0e0959
100644
--- a/src/util/util_errors.c
+++ b/src/util/util_errors.c
@@ -35,6 +35,7 @@ struct err_string error_to_str[] = {
{ "Cached credentials are expired" }, /* ERR_CACHED_CREDS_EXPIRED */
{ "Authentication Denied" }, /* ERR_AUTH_DENIED */
{ "Authentication Failed" }, /* ERR_AUTH_FAILED */
I know I suggested ERR_PREAUTH_FAILED but now I'm thinking it might not
be the best code, because it's too Kerberos specific. So what about
using more generic ERR_CRED_INVALID ?
+ { "Preauthentication Failed"}, /* ERR_PREAUTH_FAILED */
{ "Password Change Denied" }, /* ERR_CHPASS_DENIED */
{ "Password Change Failed" }, /* ERR_CHPASS_FAILED */
{ "Network I/O Error" }, /* ERR_NETWORK_IO */
diff --git a/src/util/util_errors.h b/src/util/util_errors.h
index
a602a6ea92f72a51f5e21342940b2072bbe9296d..2143774c3a2113e5ad372aa97770220c4cf3f859
100644
--- a/src/util/util_errors.h
+++ b/src/util/util_errors.h
@@ -57,6 +57,7 @@ enum sssd_errors {
ERR_CACHED_CREDS_EXPIRED,
ERR_AUTH_DENIED,
ERR_AUTH_FAILED,
+ ERR_PREAUTH_FAILED,
The new error code would have to be updated here, too.
ERR_CHPASS_DENIED,
ERR_CHPASS_FAILED,
ERR_NETWORK_IO,
--
1.8.1.4
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Thanks for hints, new patch is attached.
Ondra
--
Ondrej Kos
Associate Software Engineer
Identity Management - SSSD
Red Hat Czech
From 76604fe178b8f06f8f361a311a62cb1b1b61477e Mon Sep 17 00:00:00 2001
From: Ondrej Kos <o...@redhat.com>
Date: Thu, 13 Jun 2013 15:28:23 +0200
Subject: [PATCH] KRB: Handle preauthentication error correctly
https://fedorahosted.org/sssd/ticket/1873
KRB preauthentication error was later mishandled like authentication error.
---
src/providers/krb5/krb5_auth.c | 6 ++++++
src/providers/krb5/krb5_child.c | 4 +++-
src/util/util_errors.c | 1 +
src/util/util_errors.h | 1 +
4 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/src/providers/krb5/krb5_auth.c b/src/providers/krb5/krb5_auth.c
index f65e5993d54a5a265e4217e7f23d9549915c6b32..f6acfb4891cf5e99878ccfa7994ffeddf5447e2c 100644
--- a/src/providers/krb5/krb5_auth.c
+++ b/src/providers/krb5/krb5_auth.c
@@ -1026,6 +1026,12 @@ static void krb5_auth_done(struct tevent_req *subreq)
ret = EOK;
goto done;
+ case ERR_CREDS_INVALID:
+ state->pam_status = PAM_CRED_ERR;
+ state->dp_err = DP_ERR_OK;
+ ret = EOK;
+ goto done;
+
case ERR_NO_CREDS:
state->pam_status = PAM_CRED_UNAVAIL;
state->dp_err = DP_ERR_OK;
diff --git a/src/providers/krb5/krb5_child.c b/src/providers/krb5/krb5_child.c
index 8f746a8db561928349ffed8b7434db2a113a1f86..74d730aaa2e84af111982a450dafd524d411f472 100644
--- a/src/providers/krb5/krb5_child.c
+++ b/src/providers/krb5/krb5_child.c
@@ -1172,9 +1172,11 @@ static errno_t map_krb5_error(krb5_error_code kerr)
return ERR_CREDS_EXPIRED;
case KRB5KRB_AP_ERR_BAD_INTEGRITY:
+ return ERR_AUTH_FAILED;
+
case KRB5_PREAUTH_FAILED:
case KRB5KDC_ERR_PREAUTH_FAILED:
- return ERR_AUTH_FAILED;
+ return ERR_CREDS_INVALID;
default:
return ERR_INTERNAL;
diff --git a/src/util/util_errors.c b/src/util/util_errors.c
index b617f540691a245d1132469a1f019bcb0eb6e775..22a3045a6f9656d9ab8fe66673301a508e444771 100644
--- a/src/util/util_errors.c
+++ b/src/util/util_errors.c
@@ -31,6 +31,7 @@ struct err_string error_to_str[] = {
{ "Invalid credential type" }, /* ERR_INVALID_CRED_TYPE */
{ "No credentials available" }, /* ERR_NO_CREDS */
{ "Credentials are expired" }, /* ERR_CREDS_EXPIRED */
+ { "Failure setting user credentials"}, /* ERR_CREDS_INVALID */
{ "No cached credentials available" }, /* ERR_NO_CACHED_CREDS */
{ "Cached credentials are expired" }, /* ERR_CACHED_CREDS_EXPIRED */
{ "Authentication Denied" }, /* ERR_AUTH_DENIED */
diff --git a/src/util/util_errors.h b/src/util/util_errors.h
index a602a6ea92f72a51f5e21342940b2072bbe9296d..65d37aedb544bb303d7540fc59e1a802aee11898 100644
--- a/src/util/util_errors.h
+++ b/src/util/util_errors.h
@@ -53,6 +53,7 @@ enum sssd_errors {
ERR_INVALID_CRED_TYPE,
ERR_NO_CREDS,
ERR_CREDS_EXPIRED,
+ ERR_CREDS_INVALID,
ERR_NO_CACHED_CREDS,
ERR_CACHED_CREDS_EXPIRED,
ERR_AUTH_DENIED,
--
1.8.1.4
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
