pam_pkcs11.c: Reduce verbosity when authenticating

[andsens]

Respect the quiet config flag in more places.
The "Welcome ..." message has been integrated into the "PIN: ..." prompt.

I am a complete novice at C, so kindly take extra care in verifying this, especially the PIN prompt.
I don't know if the change introduces an overflow issue because the token label can have an arbitrary length.

[andsens]

ping Any chance of this getting reviewed/merged?

[wolneykien]

Hi. In PAM there are two types of messages (informational, error) and two types of user conversation queries (entering cleartext data and entering secret data). All of them are needed as the front-end applications could display different kinds of messages in different windows, etc. So it is OK to have a "Welcome" informational message.

On the other hand, there is a problem with the use of quiet option in code: somewhere it blocks syslog output, somewhere it blocks PAM informational (and sometimes error!) messages, somewhere both. Probably, it would be better to have two independent configuration options: one for syslog and another for PAM informational messages. Is there a real need to block PAM error messages? No, I don't think so…

[andsens]

The primary motivation for this PR is simply that running sudo something.sh ends up clobbering the output. Users know that [sudo] password for user: in the output when running a script is not actually the script, but them running sudo. But once you add up to 3 other informational messages (with no [sudo] prefix), it becomes an unnecessary mental load to filter that out when parsing output.

I understand that the messages could be useful informational output in other contexts (like a GDM password prompt), but even then I wouldn't want it to output anything. One principle of a unix tool is to be quiet when everything went well and to be noisy when things go awry.

[wolneykien]

As a first step, it would be okay to make quiet configuration option to suppress PAM informational (but not error!) messages along with syslog. Can you update the code that way?

[andsens]

I'm sorry but I fail to see which one of the changes affects an error message. Maybe you could add a note to the line(s) that is the issue?

[wolneykien]

Sorry, I've changed my mind. Now I think, that the effect of quiet option should be left unchanged: let it suppress error messages (both in syslog and in PAM). However, your goal is to suppress informational messages and it's better to have a separate option for that, say noinfo?

A nice thing is that all such messages are displayed via pam_prompt() proc. So, in order to suppess PAM_TEXT_INFO messages it would be enough to filter them in that single place. And in order to know whether they should be suppressed, we need a configuration handle. So, the proc should be modified in a way something like this:

static int pam_prompt(struct configuration_st *configuration, pam_handle_t *pamh, int style, char **response, char *fmt, ...)
{
...
    if (configuration->noinfo && style == PAM_TEXT_INFO)
        return;
}

I.e., it should return with no output if noinfo is set and the style of message is PAM_TEXT_INFO. Of course, you then need to pass configuration to all invocations of pam_prompt() and to add noinfo itself to the configuration structure and its parser.

[andsens]

I can see the advantage of doing it like that (alternatively you could maybe mask PAM_TEXT_INFO based on the config). However, that is quite outside the scope of both the PR and my capabilities.
Would it make sense to merge this as a stop-gap measure until the new pam_prompt() is implemented?

[andsens]

*ping