Bug 7124 - no error to user or in log file for pamtester errors
Summary: no error to user or in log file for pamtester errors
Status: CLOSED FIXED
Alias: None
Product: ThinLinc
Classification: Unclassified
Component: Web Access (show other bugs)
Version: 1.3.1
Hardware: PC Unknown
: P2 Normal
Target Milestone: 4.10.0
Assignee: Samuel Mannehed
URL:
Keywords: hean01_tester, prosaic
Depends on:
Blocks:
 
Reported: 2018-03-08 15:04 CET by Pierre Ossman
Modified: 2018-06-12 10:18 CEST (History)
2 users (show)

See Also:
Acceptance Criteria:


Attachments
pam_tester module to inject messages and error codes for testing (7.48 KB, text/x-csrc)
2018-06-04 14:27 CEST, Henrik Andersson
Details

Description Pierre Ossman cendio 2018-03-08 15:04:26 CET
We managed to get a system here in to a state where it was not possible to log in using web access. There were no errors given to the user, and no errors in the server log. The user just got the login page again.

With some debug lines added we found that pamtester sends back the following lines:

> pamtester: performing operation - acct_mgmt
> pamtester: Permission denied

But all lines starting with "pamtester: " are ignored, so neither of these lines result in an error to the user or the log.

We don't know yet why the system is throwing these errors.
Comment 1 Pierre Ossman cendio 2018-03-08 15:16:54 CET
It is sssd that is denying login in for the thinlinc service:

From sssd's logs (after enabling debug output):
> (Thu Mar  8 15:13:51 2018) [sssd[be[lab.lkpg.cendio.se]]] [ad_gpo_access_send] (0x0400): service thinlinc maps to Denied

From auth.log:
> Mar  8 15:13:51 ubuntu1604 pamtester: pam_sss(thinlinc:account): Access denied for user ossman: 6 (Permission denied)
Comment 2 Pierre Ossman cendio 2018-03-08 15:56:45 CET
See bug 7125 for why sssd was denying things.
Comment 6 Samuel Mannehed cendio 2018-05-18 13:39:00 CEST
After a closer look, we need a clean up here. We had a look at how sshd behaves and we will try to mimic that behavior.

The previous commit also broke some cases.
Comment 12 Samuel Mannehed cendio 2018-05-18 14:22:26 CEST
Fixed now.

These are the cases we could think of:

A: auth failed, message from PAM

B: auth failed, no message from PAM

C: account failed, message from PAM

D: account failed, no message from PAM

The expected behavior is to get generic Auth or Access failed log messages in all cases. The user should see the PAM message if we got one, or a generic message otherwise.

Unfortunately we couldn't find any PAM module that allowed us to test B, so we will have to settle for a code review for this case.
Comment 13 Henrik Andersson cendio 2018-06-04 14:27:09 CEST
Created attachment 870 [details]
pam_tester module to inject messages and error codes for testing

Created a PAM module to inject errors of different kinds. One can also make it silent so there is no pam_conv() of error messages available to the application.
Comment 14 Henrik Andersson cendio 2018-06-04 14:28:42 CEST
(In reply to comment #12)

> 
> A: auth failed, message from PAM
> 

A generic message is viewed, expected view of PAM message

> B: auth failed, no message from PAM
> 

A generic message is viewed

> C: account failed, message from PAM
> 

Message from PAM module is displayed

> D: account failed, no message from PAM
> 

A generic messages is viewed
Comment 15 Pierre Ossman cendio 2018-06-04 15:46:54 CEST
(In reply to comment #14)
> (In reply to comment #12)
> 
> > 
> > A: auth failed, message from PAM
> > 
> 
> A generic message is viewed, expected view of PAM message
> 

This can apparently be broken down a bit:

1. Error message, no prompt, auth failed
2. Error message, then prompt, auth failed
3. Error message, then prompt, auth succeeded
4. Prompt, then error message, auth failed
5. Prompt, then error message, auth succeeded

2., 3. and 5. will be handled on bug 5086 as we need that fixed to handle messages that are part of an ongoing conversation.

4. works fine right now.

So that leaves 1., which is indeed broken.
Comment 17 Pierre Ossman cendio 2018-06-05 16:07:25 CEST
Should be fixed now.

Tester should also check authentication without any password (using pam_permit.so) as that was affected by this change.
Comment 18 Henrik Andersson cendio 2018-06-12 10:18:57 CEST
Verified using build 5807

- Error message with no prompt
- Pam permit with and without error message

Works as expected

Note You need to log in before you can comment on or make changes to this bug.