Unity

Merge ~azzar1/unity:fix-1733557-xenial into unity:xenial

  1. Git
  2. lp:~azzar1/unity
  3. fix-1733557-xenial
  4. Merge into xenial
Proposed by Andrea Azzarone
Status: Merged
Approved by: Marco Trevisan (Treviño)
Approved revision: 6d1b7a7d8ff8d57bf61188cf5cc54b19be058969
Merged at revision: db57d37211ae86b97208015d5f9b0352519aa7e0
Proposed branch: ~azzar1/unity:fix-1733557-xenial
Merge into: unity:xenial
Diff against target: 159 lines (+57/-6)
4 files modified
lockscreen/LockScreenController.cpp (+2/-0)
lockscreen/UserAuthenticator.h (+1/-0)
lockscreen/UserAuthenticatorPam.cpp (+52/-6)
lockscreen/UserAuthenticatorPam.h (+2/-0)
Reviewer Review Type Date Requested Status
Marco Trevisan (Treviño) Approve
Review via email: mp+363311@code.launchpad.net

Commit message

lockscreen: cancel authentication if externally unlocked

The lockscreen can be externellay unlocked. This happens e.g. when
you unlock the session from LightDM or from the CLI. We need to
cancel the authentication in order to ensure that the next one does
not fail.

Fixes: https://bugs.launchpad.net/unity/+bug/1733557

To post a comment you must log in.
Revision history for this message
Marco Trevisan (Treviño) (3v1n0) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/lockscreen/LockScreenController.cpp b/lockscreen/LockScreenController.cpp
2index 5f5d54c..f8ba8c4 100644
3--- a/lockscreen/LockScreenController.cpp
4+++ b/lockscreen/LockScreenController.cpp
5@@ -531,6 +531,8 @@ void Controller::OnUnlockRequested()
6
7 HideBlankWindow();
8 HideShields();
9+
10+ user_authenticator_->AuthenticateCancel();
11 }
12
13 void Controller::HideShields()
14diff --git a/lockscreen/UserAuthenticator.h b/lockscreen/UserAuthenticator.h
15index e30ce2f..60e8684 100644
16--- a/lockscreen/UserAuthenticator.h
17+++ b/lockscreen/UserAuthenticator.h
18@@ -43,6 +43,7 @@ public:
19
20 // Authenticate the user in a background thread.
21 virtual bool AuthenticateStart(std::string const& username, AuthenticateEndCallback const&) = 0;
22+ virtual void AuthenticateCancel() = 0;
23
24 sigc::signal<void> start_failed;
25 sigc::signal<void, std::string, PromiseAuthCodePtr const&> echo_on_requested;
26diff --git a/lockscreen/UserAuthenticatorPam.cpp b/lockscreen/UserAuthenticatorPam.cpp
27index f4e1952..d1d6c27 100644
28--- a/lockscreen/UserAuthenticatorPam.cpp
29+++ b/lockscreen/UserAuthenticatorPam.cpp
30@@ -25,6 +25,8 @@
31 #include "unity-shared/UnitySettings.h"
32 #include "UnityCore/GLibWrapper.h"
33
34+#include <NuxCore/Logger.h>
35+
36 #include <cstring>
37 #include <security/pam_appl.h>
38 #include <vector>
39@@ -33,23 +35,48 @@ namespace unity
40 {
41 namespace lockscreen
42 {
43+namespace
44+{
45+DECLARE_LOGGER(logger, "unity.lockscreen");
46+}
47
48 bool UserAuthenticatorPam::AuthenticateStart(std::string const& username,
49 AuthenticateEndCallback const& authenticate_cb)
50 {
51 if (pam_handle_)
52+ {
53+ LOG_ERROR(logger) << "Unable to start authentication because another one has already been started";
54 return false;
55+ }
56
57 first_prompt_ = true;
58+ cancelled_ = false;
59 username_ = username;
60 authenticate_cb_ = authenticate_cb;
61
62 glib::Error error;
63- g_thread_try_new(nullptr, AuthenticationThreadFunc, this, &error);
64+ g_autoptr(GThread) thread = g_thread_try_new(nullptr, AuthenticationThreadFunc, this, &error);
65+
66+ if (!thread || error)
67+ {
68+ LOG_ERROR(logger) << "Unable to create a new thread for PAM authentication: " << error.Message();
69+ }
70
71 return !error;
72 }
73
74+void UserAuthenticatorPam::AuthenticateCancel()
75+{
76+ if (!pam_handle_)
77+ {
78+ LOG_DEBUG(logger) << "Unable to cancel authentication because none has been started";
79+ return;
80+ }
81+
82+ LOG_DEBUG(logger) << "Cancelling the authentication";
83+ cancelled_ = true;
84+}
85+
86 gpointer UserAuthenticatorPam::AuthenticationThreadFunc(gpointer data)
87 {
88 auto self = static_cast<UserAuthenticatorPam*>(data);
89@@ -78,7 +105,10 @@ gpointer UserAuthenticatorPam::AuthenticationThreadFunc(gpointer data)
90
91 pam_end(self->pam_handle_, self->status_);
92 self->pam_handle_ = nullptr;
93- self->source_manager_.AddTimeout(0, [self] { self->authenticate_cb_(self->status_ == PAM_SUCCESS); return false; });
94+
95+ if (!self->cancelled_)
96+ self->source_manager_.AddTimeout(0, [self] { self->authenticate_cb_(self->status_ == PAM_SUCCESS); return false; });
97+
98 return nullptr;
99 }
100
101@@ -88,8 +118,16 @@ bool UserAuthenticatorPam::InitPam()
102 conversation.conv = ConversationFunction;
103 conversation.appdata_ptr = static_cast<void*>(this);
104
105- return pam_start("unity", username_.c_str(),
106- &conversation, &pam_handle_) == PAM_SUCCESS;
107+ int ret = pam_start("unity", username_.c_str(),
108+ &conversation, &pam_handle_);
109+
110+ if (ret != PAM_SUCCESS)
111+ {
112+ LOG_ERROR(logger) << "Unable to start pam: " << pam_strerror(pam_handle_, ret);
113+ return false;
114+ }
115+
116+ return true;
117 }
118
119 int UserAuthenticatorPam::ConversationFunction(int num_msg,
120@@ -166,9 +204,17 @@ int UserAuthenticatorPam::ConversationFunction(int num_msg,
121 auto future = promise->get_future();
122 pam_response* resp_item = &tmp_response[i++];
123 resp_item->resp_retcode = 0;
124- resp_item->resp = strdup(future.get().c_str());
125+ resp_item->resp = nullptr;
126+
127+ std::future_status status;
128+ do
129+ {
130+ status = future.wait_for(std::chrono::seconds(1));
131+ if (status == std::future_status::ready)
132+ resp_item->resp = strdup(future.get().c_str());
133+ } while (status != std::future_status::ready && !user_auth->cancelled_);
134
135- if (!resp_item->resp)
136+ if (!resp_item->resp || user_auth->cancelled_)
137 {
138 raise_error = true;
139 break;
140diff --git a/lockscreen/UserAuthenticatorPam.h b/lockscreen/UserAuthenticatorPam.h
141index 82273cd..4043cd4 100644
142--- a/lockscreen/UserAuthenticatorPam.h
143+++ b/lockscreen/UserAuthenticatorPam.h
144@@ -39,6 +39,7 @@ class UserAuthenticatorPam : public UserAuthenticator
145 public:
146 UserAuthenticatorPam() = default;
147 bool AuthenticateStart(std::string const& username, AuthenticateEndCallback const&) override;
148+ void AuthenticateCancel() override;
149
150 private:
151 UserAuthenticatorPam(UserAuthenticatorPam const&) = delete;
152@@ -57,6 +58,7 @@ private:
153
154 int status_ = 0;
155 bool first_prompt_ = true;
156+ bool cancelled_ = false;
157 pam_handle* pam_handle_ = nullptr;
158 glib::SourceManager source_manager_;
159 };

Subscribers

People subscribed via source and target branches