Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

More descriptive error messages #291

Closed
LecrisUT opened this issue Mar 28, 2023 · 8 comments
Closed

More descriptive error messages #291

LecrisUT opened this issue Mar 28, 2023 · 8 comments

Comments

@LecrisUT
Copy link

Is your feature request related to a problem?

On a Fedora KDE install, using the following pam configuration

auth        requisite                                    pam_u2f.so cue

We get non-descriptive error messages like invalid login when the yubikey is not present, instead of a message like: Required yubikey not present.

Describe the solution that you'd like

Print out more descriptive errors:

  • Required yubikey not present
  • Invalid yubikey found
  • etc.
@LDVG
Copy link
Contributor

LDVG commented Mar 29, 2023

Hi,

Thank you for your request! This has some similarity to the proposed solution to #209. The suggested implementation can be found under the experimental cue branch . If you want to give that a go, we'd be happy for any feedback.

Note that some clients may display the messages later than expected, not at all, or immediately overwrite it with something else. While we can't do much about the clients themselves in this project, we'd nonetheless like to hear about your experience, preferably with as many clients as possible!

@LecrisUT
Copy link
Author

After a lot of trial and error getting a development environment for this, in the end the proposed solution this not work. Still only invalid login is shown. Maybe I didn't compile it correctly.

@LDVG
Copy link
Contributor

LDVG commented Mar 29, 2023

  • After compiling, have you pointed the service configuration to your build of pam_u2f.so?
  • What application/client are you attempting to authenticate with?

@LecrisUT
Copy link
Author

I have copied it in place from build/.lib/pam_u2f.so to /usr/lib64/security/pam_u2f.so.

I have tried plain linux authenthication: ctrl+alt+F4 and login after changing authselect

@LDVG
Copy link
Contributor

LDVG commented Mar 29, 2023

For sanity checking: Was the build out of the cue branch? Is the cue flag still present in the service configuration? What happens if you test through e.g. sudo?

@LecrisUT
Copy link
Author

Here's the template of authselect:

auth        requisite                                    pam_u2f.so cue {if not "without-pam-u2f-nouserok":nouserok} {include if "with-pam-u2f-2fa"}

Get rid of the curly statements and yes, cue flag is present. I am not asked to insert the pin though. I did not try the cue branch, but I replaced a similar line there with:

        } else {
          converse(pamh, PAM_TEXT_INFO, fido_strerr(r));
        }

Specificly:

        if (r == FIDO_OK) {
          if (opts.pin == FIDO_OPT_TRUE || opts.uv == FIDO_OPT_TRUE) {
            r = fido_assert_set_uv(assert, FIDO_OPT_TRUE);
            if (r != FIDO_OK) {
              debug_dbg(cfg, "Failed to set UV");
              goto out;
            }
          }
          r = fido_assert_verify(assert, 0, pk.type, pk.ptr);
          if (r == FIDO_OK) {
            retval = 1;
            goto out;
          }
        } else {
          converse(pamh, PAM_TEXT_INFO, fido_strerr(r));
        }

I did not test sudo or other applications, if I find the time to thoroughly debug it, maybe I'll test some more.

@LDVG
Copy link
Contributor

LDVG commented Mar 29, 2023

OK. Your modification will only print out an error if an authenticator is found but an error occurs when trying to authenticate using it (e.g. user presence is not collected, PIN does not verify, etc.). To also print out an error message when no authenticators are found, you need additional changes.

@LecrisUT
Copy link
Author

Ok, with the cue branch. It does print out suitable error messages. Can mark that PR to close this issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants