git.iko.soy/unity8
on xenial help sign in
Rejigger stored timestamp to be start of lock-out
commita7220cfc0602eaa616d984f922a81232fc468950 authorMichael Terry <michael.terry@canonical.com> dateWed 19 Aug 2015, 15:41 (11 years ago) parenta21dd1b
5 files changed+66−44
compare with xenial
unifiedsplit
data/com.canonical.Unity8.gschema.xml+3−3 qml/Greeter/Greeter.qml+35−13 tests/mocks/GSettings.1.0/fake_gsettings.cpp+12−12 tests/mocks/GSettings.1.0/fake_gsettings.h+8−8 tests/qmltests/Greeter/tst_Greeter.qml+8−8
data/com.canonical.Unity8.gschema.xml+3−3
@@ -14,10 +14,10 @@
1414 </schema>
1515
1616 <schema path="/com/canonical/unity8/greeter/" id="com.canonical.Unity8.Greeter" gettext-domain="unity8">
17− <key type="t" name="locked-out-until">
17+ <key type="t" name="locked-out-time">
1818 <default>0</default>
19− <summary>When the greeter will no longer be locked.</summary>
20− <description>If too many unsuccessful login attempts have been made in a row, the greeter locks you out for a while. This unix timestamp indicates when you will be able to log back in.</description>
19+ <summary>When the greeter was last locked.</summary>
20+ <description>If too many unsuccessful login attempts have been made in a row, the greeter locks you out for a while. This unix timestamp indicates when you were locked out, so unity8 knows when to let you back in.</description>
2121 </key>
2222 </schema>
2323</schemalist>
qml/Greeter/Greeter.qml+35−13
@@ -196,13 +196,25 @@ Showable {
196196 // for a few minutes. When we wake up, we want to quickly be correct.
197197 interval: 500
198198
199− property var delayTarget;
199+ property Date delayTarget;
200200 property int delayMinutes;
201201
202− function forceDelay(delay /* in minutes */) {
203− var when = new Date().getTime() + delay * 60000;
204− greeterSettings.lockedOutUntil = when;
205− // code below that watches GSettings will pick up the change
202+ function forceDelay() {
203+ // Store the beginning time for a lockout in GSettings, so that
204+ // we still lock the user out if they reboot. And we store
205+ // starting time rather than end-time or how-long because:
206+ // - If storing end-time and on boot we have a problem with NTP,
207+ // we might get locked out for a lot longer than we thought.
208+ // - If storing how-long, and user turns their phone off for an
209+ // hour rather than wait, they wouldn't expect to still be locked
210+ // out.
211+ // - A malicious actor could manipulate either of the above
212+ // settings to keep the user out longer. But by storing
213+ // start-time, we never make the user wait longer than the full
214+ // lock out time.
215+ // The code below that watches GSettings will pick up the change
216+ // for us.
217+ greeterSettings.lockedOutTime = new Date().getTime();
206218 }
207219
208220 onTriggered: {
@@ -212,17 +224,27 @@ Showable {
212224 start(); // go again
213225 } else {
214226 delayMinutes = 0;
215−
216− // clean up, to avoid accidentally locking user out if phone
217− // gets time wrong for whatever reason in the future.
218− greeterSettings.lockedOutUntil = 0;
219227 }
220228 }
221229
222230 function checkForForcedDelay() {
223− delayTarget = new Date();
224− delayTarget.setTime(greeterSettings.lockedOutUntil);
225− triggered();
231+ var now = new Date();
232+ delayTarget.setTime(greeterSettings.lockedOutTime + failedLoginsDelayMinutes * 60000);
233+
234+ // If tooEarly is true, something went very wrong. Bug or NTP
235+ // misconfiguration maybe?
236+ var tooEarly = now.getTime() < greeterSettings.lockedOutTime;
237+ var tooLate = now > delayTarget;
238+
239+ // Compare stored time to system time. If a malicious actor is
240+ // able to manipulate time to avoid our lockout, they already have
241+ // enough access to cause damage. So we choose to trust this check.
242+ if (tooEarly || tooLate) {
243+ stop();
244+ delayMinutes = 0;
245+ } else {
246+ triggered();
247+ }
226248 }
227249
228250 Component.onCompleted: checkForForcedDelay()
@@ -398,7 +420,7 @@ Showable {
398420 if (failedLoginsDelayAttempts > 0
399421 && AccountsService.failedLogins > 0
400422 && AccountsService.failedLogins % failedLoginsDelayAttempts == 0) {
401− forcedDelayTimer.forceDelay(failedLoginsDelayMinutes);
423+ forcedDelayTimer.forceDelay();
402424 }
403425
404426 loader.item.notifyAuthenticationFailed();
tests/mocks/GSettings.1.0/fake_gsettings.cpp+12−12
@@ -62,16 +62,16 @@ void GSettingsControllerQml::setUsageMode(const QString &usageMode)
6262 }
6363}
6464
65−quint64 GSettingsControllerQml::lockedOutUntil() const
65+quint64 GSettingsControllerQml::lockedOutTime() const
6666{
67− return m_lockedOutUntil;
67+ return m_lockedOutTime;
6868}
6969
70−void GSettingsControllerQml::setLockedOutUntil(quint64 timestamp)
70+void GSettingsControllerQml::setLockedOutTime(quint64 timestamp)
7171{
72− if (m_lockedOutUntil != timestamp) {
73− m_lockedOutUntil = timestamp;
74− Q_EMIT lockedOutUntilChanged(m_lockedOutUntil);
72+ if (m_lockedOutTime != timestamp) {
73+ m_lockedOutTime = timestamp;
74+ Q_EMIT lockedOutTimeChanged(m_lockedOutTime);
7575 }
7676}
7777
@@ -112,8 +112,8 @@ GSettingsQml::GSettingsQml(QObject *parent)
112112 this, SIGNAL(pictureUriChanged(const QString &)));
113113 connect(GSettingsControllerQml::instance(), SIGNAL(usageModeChanged(const QString &)),
114114 this, SIGNAL(usageModeChanged(const QString &)));
115− connect(GSettingsControllerQml::instance(), SIGNAL(lockedOutUntilChanged(quint64)),
116− this, SIGNAL(lockedOutUntilChanged(quint64)));
115+ connect(GSettingsControllerQml::instance(), SIGNAL(lockedOutTimeChanged(quint64)),
116+ this, SIGNAL(lockedOutTimeChanged(quint64)));
117117}
118118
119119GSettingsSchemaQml * GSettingsQml::schema() const {
@@ -152,18 +152,18 @@ void GSettingsQml::setUsageMode(const QString &usageMode)
152152 }
153153}
154154
155−quint64 GSettingsQml::lockedOutUntil() const
155+quint64 GSettingsQml::lockedOutTime() const
156156{
157157 if (m_schema->id() == "com.canonical.Unity8.Greeter") {
158− return GSettingsControllerQml::instance()->lockedOutUntil();
158+ return GSettingsControllerQml::instance()->lockedOutTime();
159159 } else {
160160 return 0;
161161 }
162162}
163163
164−void GSettingsQml::setLockedOutUntil(quint64 timestamp)
164+void GSettingsQml::setLockedOutTime(quint64 timestamp)
165165{
166166 if (m_schema->id() == "com.canonical.Unity8.Greeter") {
167− GSettingsControllerQml::instance()->setLockedOutUntil(timestamp);
167+ GSettingsControllerQml::instance()->setLockedOutTime(timestamp);
168168 }
169169}
tests/mocks/GSettings.1.0/fake_gsettings.h+8−8
@@ -47,7 +47,7 @@ class GSettingsQml: public QObject
4747 Q_PROPERTY(GSettingsSchemaQml* schema READ schema NOTIFY schemaChanged)
4848 Q_PROPERTY(QString pictureUri READ pictureUri WRITE setPictureUri NOTIFY pictureUriChanged)
4949 Q_PROPERTY(QString usageMode READ usageMode WRITE setUsageMode NOTIFY usageModeChanged)
50− Q_PROPERTY(quint64 lockedOutUntil READ lockedOutUntil WRITE setLockedOutUntil NOTIFY lockedOutUntilChanged)
50+ Q_PROPERTY(quint64 lockedOutTime READ lockedOutTime WRITE setLockedOutTime NOTIFY lockedOutTimeChanged)
5151
5252public:
5353 GSettingsQml(QObject *parent = nullptr);
@@ -55,17 +55,17 @@ public:
5555 GSettingsSchemaQml * schema() const;
5656 QString pictureUri() const;
5757 QString usageMode() const;
58− quint64 lockedOutUntil() const;
58+ quint64 lockedOutTime() const;
5959
6060 void setPictureUri(const QString &str);
6161 void setUsageMode(const QString &usageMode);
62− void setLockedOutUntil(quint64 timestamp);
62+ void setLockedOutTime(quint64 timestamp);
6363
6464Q_SIGNALS:
6565 void schemaChanged();
6666 void pictureUriChanged(const QString&);
6767 void usageModeChanged(const QString&);
68− void lockedOutUntilChanged(quint64);
68+ void lockedOutTimeChanged(quint64);
6969
7070private:
7171 GSettingsSchemaQml* m_schema;
@@ -87,20 +87,20 @@ public:
8787 QString usageMode() const;
8888 Q_INVOKABLE void setUsageMode(const QString &usageMode);
8989
90− quint64 lockedOutUntil() const;
91− Q_INVOKABLE void setLockedOutUntil(quint64 timestamp);
90+ quint64 lockedOutTime() const;
91+ Q_INVOKABLE void setLockedOutTime(quint64 timestamp);
9292
9393Q_SIGNALS:
9494 void pictureUriChanged(const QString&);
9595 void usageModeChanged(const QString&);
96− void lockedOutUntilChanged(quint64 timestamp);
96+ void lockedOutTimeChanged(quint64 timestamp);
9797
9898private:
9999 GSettingsControllerQml();
100100
101101 QString m_pictureUri;
102102 QString m_usageMode;
103− quint64 m_lockedOutUntil;
103+ quint64 m_lockedOutTime;
104104
105105 static GSettingsControllerQml* s_controllerInstance;
106106 QList<GSettingsQml *> m_registeredGSettings;
tests/qmltests/Greeter/tst_Greeter.qml+8−8
@@ -154,7 +154,7 @@ Item {
154154 verifySelected(LightDM.Users.data(0, LightDM.UserRoles.NameRole));
155155 greeter.failedLoginsDelayAttempts = 7;
156156 greeter.failedLoginsDelayMinutes = 5;
157− greeterSettings.lockedOutUntil = 0;
157+ greeterSettings.lockedOutTime = 0;
158158 }
159159
160160 function cleanup() {
@@ -443,22 +443,23 @@ Item {
443443 greeter.failedLoginsDelayMinutes = 1;
444444 selectUser("has-password");
445445 tryCompare(viewShowPromptSpy, "count", 1);
446+ compare(greeterSettings.lockedOutTime, 0);
446447 view.responded("wr0ng p4ssw0rd");
447448
448− var timestamp = new Date().getTime() + 60000;
449− verify(Math.abs(greeterSettings.lockedOutUntil - timestamp) < 3);
449+ var timestamp = new Date().getTime();
450+ verify(Math.abs(greeterSettings.lockedOutTime - timestamp) < 3);
450451 }
451452
452453 function test_forcedDelayFromGSettings() {
454+ greeter.failedLoginsDelayMinutes = 1;
453455 compare(view.delayMinutes, 0);
454− var timestamp = new Date().getTime() + 30; // in 30 seconds
455− greeterSettings.lockedOutUntil = timestamp;
456+ greeterSettings.lockedOutTime = new Date().getTime();
456457 compare(view.delayMinutes, 1);
457458 }
458459
459460 function test_forcedDelayOnConstruction() {
460− var timestamp = new Date().getTime() + 30; // in 30 seconds
461− greeterSettings.lockedOutUntil = timestamp;
461+ greeter.failedLoginsDelayMinutes = 1;
462+ greeterSettings.lockedOutTime = new Date().getTime();
462463 cleanup();
463464 view = findChild(greeter, "testView");
464465 compare(view.delayMinutes, 1);
@@ -475,7 +476,6 @@ Item {
475476 view.responded("wr0ng p4ssw0rd");
476477 compare(view.delayMinutes, 1);
477478 tryCompare(view, "delayMinutes", 0);
478− compare(greeterSettings.lockedOutUntil, 0);
479479 }
480480
481481 function test_dbusRequestAuthenticationUser() {