Bug 92886

Summary: PolkitAgentSession incorrectly handles multiline PAM_TEXT_INFO output
Product: PolicyKit Reporter: Dariusz Gadomski <dariusz.gadomski>
Component: daemonAssignee: David Zeuthen (not reading bugmail) <zeuthen>
Status: RESOLVED FIXED QA Contact: David Zeuthen (not reading bugmail) <zeuthen>
Severity: normal    
Priority: medium    
Version: unspecified   
Hardware: Other   
OS: All   
Whiteboard:
i915 platform: i915 features:
Attachments: Tested patch fixing the issue
Refactoring to use send_to_helper
Refactoring to use send_to_helper

Description Dariusz Gadomski 2015-11-10 09:42:00 UTC
There is an error observed when Ubuntu is configured to perform authentication via pam_vas (Vintela Authentication Services by Dell) in a disconnected mode (using cached authentication).

Steps to reproduce:
1. Configure pam_vas client authenticating to a remote server.
2. Perform authentication to cache the credentials.
3. Disconnect from the network where the server is reachable (to force using cached information).
4. Perform an action requiring polkit authentication.

Expected result:
Authentication succeeds accompanied by the following message "You have logged in using cached account information. Some network services will be unavailable".

Actual result:
Authentication fails accompanied by the following message "You have logged in using cached account information. Some network services will be unavailable".

Probable cause:
The PolkitAgentSession part of polkit is designed to interpret only 1-line output, while interaction with pam_vas in the above scenario triggers helper to produce the following 2-line output:
PAM_TEXT_INFO You have logged in using cached account information. Some network services
will be unavailable.

The 'will be unavailable.' part is interpreted as an unknown message and causes failed authorization.
Comment 1 Dariusz Gadomski 2015-11-10 09:55:13 UTC
Created attachment 119534 [details] [review]
Tested patch fixing the issue
Comment 2 Miloslav Trmac 2015-11-10 17:47:20 UTC
Thanks for the patch.

Yes, this is the correct thing to do (polkitagentsession.c:io_watch_have_data is always calling g_strcompress()).

Having the same newline handling and escaping code in three places seems too ugly and unnecessary, though.  Would you be willing to update the patch so that polkitagenthelper-pam.c uses the send_to_helper function throughout (modified to do the escaping as necessary), or at least to test such an updated patch?
Comment 3 Dariusz Gadomski 2015-11-11 13:00:21 UTC
Miroslav, thanks for taking a look.

Sure, I can improve the patch. I'll attach the new version as soon as it's done.
Comment 4 Dariusz Gadomski 2015-11-13 15:37:00 UTC
Created attachment 119643 [details]
Refactoring to use send_to_helper

I have refactored the code to make use of send_to_helper instead of duplicating the code.

I've chosen to apply it on top of the previous change to keep 2 different actions (escaping PAM_TEXT_INFO and refactoring) on the same step.

I have done sanity testing to make sure everything works as expected.
Comment 5 Dariusz Gadomski 2015-11-13 15:57:38 UTC
Typo, what I've meant in the above comment is that I wanted to avoid doing 2 different actions in the same commit.
Comment 6 Dariusz Gadomski 2015-11-16 10:42:52 UTC
Created attachment 119702 [details] [review]
Refactoring to use send_to_helper
Comment 7 Miloslav Trmac 2015-11-18 22:31:24 UTC
Perfect.  Committed, thank you!

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.