Hi,
this patch is the SSSD part of the Authentication Indicator related changes in FreeIPA. The basic part is that now it is possible to authenticate at will with either password or 2FA as described on https://fedorahosted.org/sssd/wiki/DesignDocs/PromptingForMultipleAuthentica....
To test you need a FreeIPA server build from the FreeIPA master branch and optionally Nathaniel's '[PATCH 0093] Enable service authentication indicator management' which is currently under review for the kvno related test. Additionally you need MIT Kerberos packages which contain the fix for https://bugzilla.redhat.com//show_bug.cgi?id=1340304.
Since this patch changes how libkrb5 gets the password for password authentication with newer version of MIT Kerberos it would be nice if someone can run some regression tests with the AD or plain KRB5 auth_provider.
bye, Sumit
On (30/05/16 17:07), Sumit Bose wrote:
Hi,
this patch is the SSSD part of the Authentication Indicator related changes in FreeIPA. The basic part is that now it is possible to authenticate at will with either password or 2FA as described on https://fedorahosted.org/sssd/wiki/DesignDocs/PromptingForMultipleAuthentica....
To test you need a FreeIPA server build from the FreeIPA master branch and optionally Nathaniel's '[PATCH 0093] Enable service authentication indicator management' which is currently under review for the kvno related test. Additionally you need MIT Kerberos packages which contain the fix for https://bugzilla.redhat.com//show_bug.cgi?id=1340304.
Since this patch changes how libkrb5 gets the password for password authentication with newer version of MIT Kerberos it would be nice if someone can run some regression tests with the AD or plain KRB5 auth_provider.
I can see failures in password change for AD provider. I haven't tried KRB5 or IPA yet.
From 31f0e621fa63764f9f89f8d6e01f94c49675216f Mon Sep 17 00:00:00 2001 From: Sumit Bose sbose@redhat.com Date: Thu, 26 May 2016 13:20:59 +0200 Subject: [PATCH] PAM/KRB5: optional otp and password prompting
Depending on the available Kerberos pre-authentication methods pam_sss will prompt the user for a password, 2 authentication factors or both.
Resolves https://fedorahosted.org/sssd/ticket/2988
src/providers/krb5/krb5_child.c | 76 +++++++++++++++++++++++++++++++++++++++-- src/sss_client/pam_message.h | 2 ++ src/sss_client/pam_sss.c | 15 ++++++-- src/sss_client/sss_cli.h | 5 +++ 4 files changed, 93 insertions(+), 5 deletions(-)
//snip
static krb5_error_code sss_krb5_prompter(krb5_context context, void *data, const char *name, const char *banner, int num_prompts, krb5_prompt prompts[]) { int ret;
size_t c; struct krb5_req *kr = talloc_get_type(data, struct krb5_req);
DEBUG(SSSDBG_TRACE_ALL,
"sss_krb5_prompter name [%s] banner [%s] num_prompts [%d] EINVAL.\n",name, banner, num_prompts);if (num_prompts != 0) { DEBUG(SSSDBG_CRIT_FAILURE, "Cannot handle password prompts.\n");
if (DEBUG_IS_SET(SSSDBG_TRACE_ALL)) {for (c = 0; c < num_prompts; c++) {DEBUG(SSSDBG_TRACE_ALL, "Prompt [%u][%s].\n", c,
^^^ type of "c" is size_t an therefore there should be "%zu". otherwise there is a warning on 64 bit platforms CC src/providers/krb5/krb5_child-krb5_child.o ../sssd/src/providers/krb5/krb5_child.c: In function ‘sss_krb5_prompter’: ../sssd/src/providers/krb5/krb5_child.c:665:17: error: format ‘%u’ expects argument of type ‘unsigned int’, but argument 6 has type ‘size_t {aka long unsigned int}’ [-Werror=format=] DEBUG(SSSDBG_TRACE_ALL, "Prompt [%u][%s].\n", c, ^~~~~ cc1: all warnings being treated as errors
LS
On Fri, Jun 10, 2016 at 02:28:16PM +0200, Lukas Slebodnik wrote:
On (30/05/16 17:07), Sumit Bose wrote:
Hi,
this patch is the SSSD part of the Authentication Indicator related changes in FreeIPA. The basic part is that now it is possible to authenticate at will with either password or 2FA as described on https://fedorahosted.org/sssd/wiki/DesignDocs/PromptingForMultipleAuthentica....
To test you need a FreeIPA server build from the FreeIPA master branch and optionally Nathaniel's '[PATCH 0093] Enable service authentication indicator management' which is currently under review for the kvno related test. Additionally you need MIT Kerberos packages which contain the fix for https://bugzilla.redhat.com//show_bug.cgi?id=1340304.
Since this patch changes how libkrb5 gets the password for password authentication with newer version of MIT Kerberos it would be nice if someone can run some regression tests with the AD or plain KRB5 auth_provider.
I can see failures in password change for AD provider. I haven't tried KRB5 or IPA yet.
Thank you for testing and reviewing. I guess it was a general issue, the new patch should fix it.
...
DEBUG(SSSDBG_TRACE_ALL, "Prompt [%u][%s].\n", c,^^^ type of "c" is size_t an therefore there should be "%zu". otherwise there is a warning on 64 bit platformsCC src/providers/krb5/krb5_child-krb5_child.o ../sssd/src/providers/krb5/krb5_child.c: In function ‘sss_krb5_prompter’: ../sssd/src/providers/krb5/krb5_child.c:665:17: error: format ‘%u’ expects argument of type ‘unsigned int’, but argument 6 has type ‘size_t {aka long unsigned int}’ [-Werror=format=] DEBUG(SSSDBG_TRACE_ALL, "Prompt [%u][%s].\n", c, ^~~~~ cc1: all warnings being treated as errors
Fixed as well. New version attached.
bye, Sumit
and now with patch ...
On Mon, Jun 13, 2016 at 02:00:37PM +0200, Sumit Bose wrote:
On Fri, Jun 10, 2016 at 02:28:16PM +0200, Lukas Slebodnik wrote:
On (30/05/16 17:07), Sumit Bose wrote:
Hi,
this patch is the SSSD part of the Authentication Indicator related changes in FreeIPA. The basic part is that now it is possible to authenticate at will with either password or 2FA as described on https://fedorahosted.org/sssd/wiki/DesignDocs/PromptingForMultipleAuthentica....
To test you need a FreeIPA server build from the FreeIPA master branch and optionally Nathaniel's '[PATCH 0093] Enable service authentication indicator management' which is currently under review for the kvno related test. Additionally you need MIT Kerberos packages which contain the fix for https://bugzilla.redhat.com//show_bug.cgi?id=1340304.
Since this patch changes how libkrb5 gets the password for password authentication with newer version of MIT Kerberos it would be nice if someone can run some regression tests with the AD or plain KRB5 auth_provider.
I can see failures in password change for AD provider. I haven't tried KRB5 or IPA yet.
Thank you for testing and reviewing. I guess it was a general issue, the new patch should fix it.
...
DEBUG(SSSDBG_TRACE_ALL, "Prompt [%u][%s].\n", c,^^^ type of "c" is size_t an therefore there should be "%zu". otherwise there is a warning on 64 bit platformsCC src/providers/krb5/krb5_child-krb5_child.o ../sssd/src/providers/krb5/krb5_child.c: In function ‘sss_krb5_prompter’: ../sssd/src/providers/krb5/krb5_child.c:665:17: error: format ‘%u’ expects argument of type ‘unsigned int’, but argument 6 has type ‘size_t {aka long unsigned int}’ [-Werror=format=] DEBUG(SSSDBG_TRACE_ALL, "Prompt [%u][%s].\n", c, ^~~~~ cc1: all warnings being treated as errors
Fixed as well. New version attached.
bye, Sumit
On (13/06/16 14:01), Sumit Bose wrote:
and now with patch ...
On Mon, Jun 13, 2016 at 02:00:37PM +0200, Sumit Bose wrote:
On Fri, Jun 10, 2016 at 02:28:16PM +0200, Lukas Slebodnik wrote:
On (30/05/16 17:07), Sumit Bose wrote:
Hi,
this patch is the SSSD part of the Authentication Indicator related changes in FreeIPA. The basic part is that now it is possible to authenticate at will with either password or 2FA as described on https://fedorahosted.org/sssd/wiki/DesignDocs/PromptingForMultipleAuthentica....
To test you need a FreeIPA server build from the FreeIPA master branch and optionally Nathaniel's '[PATCH 0093] Enable service authentication indicator management' which is currently under review for the kvno related test. Additionally you need MIT Kerberos packages which contain the fix for https://bugzilla.redhat.com//show_bug.cgi?id=1340304.
Since this patch changes how libkrb5 gets the password for password authentication with newer version of MIT Kerberos it would be nice if someone can run some regression tests with the AD or plain KRB5 auth_provider.
I can see failures in password change for AD provider. I haven't tried KRB5 or IPA yet.
Thank you for testing and reviewing. I guess it was a general issue, the new patch should fix it.
...
DEBUG(SSSDBG_TRACE_ALL, "Prompt [%u][%s].\n", c,^^^ type of "c" is size_t an therefore there should be "%zu". otherwise there is a warning on 64 bit platformsCC src/providers/krb5/krb5_child-krb5_child.o ../sssd/src/providers/krb5/krb5_child.c: In function ‘sss_krb5_prompter’: ../sssd/src/providers/krb5/krb5_child.c:665:17: error: format ‘%u’ expects argument of type ‘unsigned int’, but argument 6 has type ‘size_t {aka long unsigned int}’ [-Werror=format=] DEBUG(SSSDBG_TRACE_ALL, "Prompt [%u][%s].\n", c, ^~~~~ cc1: all warnings being treated as errors
Fixed as well. New version attached.
bye, Sumit
From 28b5cd4eb73cee9401eefd7d0232c5dc69e69f8c Mon Sep 17 00:00:00 2001 From: Sumit Bose sbose@redhat.com Date: Thu, 26 May 2016 13:20:59 +0200 Subject: [PATCH] PAM/KRB5: optional otp and password prompting
Depending on the available Kerberos pre-authentication methods pam_sss will prompt the user for a password, 2 authentication factors or both.
Resolves https://fedorahosted.org/sssd/ticket/2988
Bug with password change is fixed There aren't any newly introduced isues reported by static analyzers. All AD tests passed.
I didn't try OTP becuase I do not have such setup and I need to work on other thing. If someone could help I will appreciate it.
LS
Patch WFM and looks clean. My only question is on the overall prompting.
Currently we have: * 1FA: - "Password" * 2FA (required): - "First Factor" - "Second Factor" * 2FA (optional): - "First Factor or Password" - "Second Factor, press return for Password authentication"
This all seems a little verbose to me. It also seems like it reveals technical details that don't matter and/or might confuse the user.
Is there a reason not to do the following?
* 1FA: - "Password" * 2FA (required): - "Password" - "Token Code" * 2FA (optional): - "Password" - "Token Code (optional)"
This seems to me simpler, shorter and more descriptive. Alternatively:
* 1FA: - "Password" * 2FA (required): - "Password" - "Second Factor" * 2FA (optional): - "Password" - "Second Factor (optional)"
Whatever we decide to do, this patch has my ACK. So feel free to just change the string and merge it.
On Mon, 2016-06-13 at 14:01 +0200, Sumit Bose wrote:
and now with patch ...
On Mon, Jun 13, 2016 at 02:00:37PM +0200, Sumit Bose wrote:
On Fri, Jun 10, 2016 at 02:28:16PM +0200, Lukas Slebodnik wrote:
On (30/05/16 17:07), Sumit Bose wrote:
Hi,
this patch is the SSSD part of the Authentication Indicator related changes in FreeIPA. The basic part is that now it is possible to authenticate at will with either password or 2FA as described on https://fedorahosted.org/sssd/wiki/DesignDocs/PromptingForMulti pleAuthenticationTypes.
To test you need a FreeIPA server build from the FreeIPA master branch and optionally Nathaniel's '[PATCH 0093] Enable service authentication indicator management' which is currently under review for the kvno related test. Additionally you need MIT Kerberos packages which contain the fix for https://bugzilla.redhat.com//show_bug.cgi?id=134030 4.
Since this patch changes how libkrb5 gets the password for password authentication with newer version of MIT Kerberos it would be nice if someone can run some regression tests with the AD or plain KRB5 auth_provider.
I can see failures in password change for AD provider. I haven't tried KRB5 or IPA yet.
Thank you for testing and reviewing. I guess it was a general issue, the new patch should fix it.
...
+ DEBUG(SSSDBG_TRACE_ALL, "Prompt [%u][%s].\n", c,
^^^ type of "c" is size_t an therefore there should be "%zu". otherwise there is a warning on 64 bit platforms CC src/providers/krb5/krb5_child-krb5_child.o ../sssd/src/providers/krb5/krb5_child.c: In function ‘sss_krb5_prompter’: ../sssd/src/providers/krb5/krb5_child.c:665:17: error: format ‘%u’ expects argument of type ‘unsigned int’, but argument 6 has type ‘size_t {aka long unsigned int}’ [-Werror=format=] DEBUG(SSSDBG_TRACE_ALL, "Prompt [%u][%s].\n", c, ^~~~~ cc1: all warnings being treated as errors
Fixed as well. New version attached.
bye, Sumit
sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahos ted.org
On Wed, Jul 06, 2016 at 09:03:45AM -0400, Nathaniel McCallum wrote:
Patch WFM and looks clean. My only question is on the overall
Thank you very much for testing.
prompting.
Currently we have: * 1FA: - "Password" * 2FA (required): - "First Factor" - "Second Factor" * 2FA (optional): - "First Factor or Password" - "Second Factor, press return for Password authentication"
This all seems a little verbose to me. It also seems like it reveals technical details that don't matter and/or might confuse the user.
Is there a reason not to do the following?
* 1FA: - "Password" * 2FA (required): - "Password" - "Token Code" * 2FA (optional): - "Password" - "Token Code (optional)"
This seems to me simpler, shorter and more descriptive. Alternatively:
* 1FA: - "Password" * 2FA (required): - "Password" - "Second Factor" * 2FA (optional): - "Password" - "Second Factor (optional)"
Whatever we decide to do, this patch has my ACK. So feel free to just change the string and merge it.
When the 2FA prompting was added in the previous release I think I asked for alternative suggestions as well. My main reasons, especially with the 'First Factor' prompt were first to make the user aware that something new is happen. New in the sense that the user should be prepared to enter the second factor in a new prompt and not put the combined password in the 'Password' prompt as he was used to before.
And second I wasn't sure if users are aware that the first factor is typically their long term password. Iirc the first factor with RSA tokens are typically referred to as PIN and asking for a password might confuse the users.
Maybe we should reach out to UX designers to see what they recommend?
bye, Sumit
On Mon, 2016-06-13 at 14:01 +0200, Sumit Bose wrote:
and now with patch ...
On Mon, Jun 13, 2016 at 02:00:37PM +0200, Sumit Bose wrote:
On Fri, Jun 10, 2016 at 02:28:16PM +0200, Lukas Slebodnik wrote:
On (30/05/16 17:07), Sumit Bose wrote:
Hi,
this patch is the SSSD part of the Authentication Indicator related changes in FreeIPA. The basic part is that now it is possible to authenticate at will with either password or 2FA as described on https://fedorahosted.org/sssd/wiki/DesignDocs/PromptingForMulti pleAuthenticationTypes.
To test you need a FreeIPA server build from the FreeIPA master branch and optionally Nathaniel's '[PATCH 0093] Enable service authentication indicator management' which is currently under review for the kvno related test. Additionally you need MIT Kerberos packages which contain the fix for https://bugzilla.redhat.com//show_bug.cgi?id=134030 4.
Since this patch changes how libkrb5 gets the password for password authentication with newer version of MIT Kerberos it would be nice if someone can run some regression tests with the AD or plain KRB5 auth_provider.
I can see failures in password change for AD provider. I haven't tried KRB5 or IPA yet.
Thank you for testing and reviewing. I guess it was a general issue, the new patch should fix it.
...
+ DEBUG(SSSDBG_TRACE_ALL, "Prompt [%u][%s].\n", c,
^^^ type of "c" is size_t an therefore there should be "%zu". otherwise there is a warning on 64 bit platforms CC src/providers/krb5/krb5_child-krb5_child.o ../sssd/src/providers/krb5/krb5_child.c: In function ‘sss_krb5_prompter’: ../sssd/src/providers/krb5/krb5_child.c:665:17: error: format ‘%u’ expects argument of type ‘unsigned int’, but argument 6 has type ‘size_t {aka long unsigned int}’ [-Werror=format=] DEBUG(SSSDBG_TRACE_ALL, "Prompt [%u][%s].\n", c, ^~~~~ cc1: all warnings being treated as errors
Fixed as well. New version attached.
bye, Sumit
sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahos ted.org
sssd-devel@lists.fedorahosted.org
-
Lukas Slebodnik -
Nathaniel McCallum -
Sumit Bose