Bug 380491

I noticed some odd behavior in screen locker indeed but I never figured out why that happens - thanks for the hint with submitting empty breaking it.

I think I've encountered the same issue.
Version: 5.10.0

I noticed that I am unable to unlock the lockscreen after the screen went off.

I can unlock the lockscreen with my password without any problem:
- If I lock the screen using ctrl+alt+L.
- If I put the computer in standby and wake it up again.

I cannot unlock the lockscreen (error: "Unlocking failed"):
- If I wait until the screen goes off and wake it up (sometimes using enter).

However, I can always use "loginctl unlock-session" to unlock from another tty.
Downgrading kscreenlocker to 5.9.5 fixes the issue.

This can also be reproduced by running kscreenlocker_greet --testing and then typing in an empty password as first password. What happens:

It initializes KCheckPass, sends the password (which is empty) to it, gets a ConvPutAuthFailed and then a ConvGetHidden request. Whenever it gets a request it sets m_ready to false and only sets it back to true when it receives a ConvPutReadyForAuthentication.

When we end up in ConvGetHidden it only sends back a 1 to kcheckpass if (!m_password.isEmpty()) and then it is never put back to ready and any subsequent request fails. If I remove the isEmpty check, it works, but I have no idea of the codebase and I also don't know what ConvGetHidden means and what "IsSecret" means, so I don't want to randomly change things here to make it somehow work... :(

I believe I figured out the issue, it's a simple protocol mismatch between sender and receiver.

The receiver (kcheckpass) reads a string and if it is !nullptr, reads an int:

        msg = GRecvStr ();
        if (msg && (GRecvInt() & IsPassword) && !*msg)

GRecvStr returns a nullptr if the received string has a length of 0 or malloc fails.

The sender (kscreenlocker_greet) sends a string and if it is not empty, send an int:

            GSendStr(m_password.toUtf8().constData());
            if (!m_password.isEmpty()) {
                // IsSecret
                GSendInt(1);
            }

Obviously that does not match, as an empty string still has a length of 1, so kcheckpass will not receive the int it expects.

I'll test that and make a patch that cleans it up a bit. Although I'm not quite sure what "IsSecret" (which should've been "IsPassword" instead, which even has a different value now!) was originally supposed to mean, I'll leave it as-is.

(In reply to Fabian Vogt from comment #4)
> I'll test that and make a patch that cleans it up a bit. Although I'm not
> quite sure what "IsSecret" (which should've been "IsPassword" instead, which
> even has a different value now!) was originally supposed to mean, I'll leave
> it as-is.

Please test and review: https://phabricator.kde.org/D6091

Patch tested, fixes the issue for KaOS, kscreenlocker 5.10.0, Qt 5.9.0.

Git commit 23fa33cedfa55cbac83bbdcc514b988d721552dc by Fabian Vogt.
Committed on 04/06/2017 at 18:57.
Pushed by fvogt into branch 'Plasma/5.10'.

Fixup protocol mismatch between greeter and kcheckpass

Summary:
The receiver (kcheckpass) reads a string and if it is !nullptr, reads an int:

	msg = GRecvStr ();
	if (msg && (GRecvInt() & IsPassword) && !*msg)

The sender (kscreenlocker_greet) sends a string and if it is not empty,
sends an int:

	GSendStr(m_password.toUtf8().constData());
	if (!m_password.isEmpty()) {
		// IsSecret
		GSendInt(1);
	}

This does not work out for empty strings, as those still have a length of 1,
resulting in kcheckpass waiting indefinitely for an int that does not get sent.
Testing for a nullptr on the sender side instead of the string length fixes this.

Also clean up the code duplication and IsSecret (1)/IsPassword (2) mismatch.

Test Plan:
Reproduced the bug without this patch, with this patch it does not
happen anymore. Authentication still works and fails as expected.

Reviewers: #plasma

Subscribers: plasma-devel

Tags: #plasma

Differential Revision: https://phabricator.kde.org/D6091

M  +7    -15   greeter/authenticator.cpp

https://commits.kde.org/kscreenlocker/23fa33cedfa55cbac83bbdcc514b988d721552dc

*** Bug 380509 has been marked as a duplicate of this bug. ***

(In reply to Fabian Vogt from comment #5)
> (In reply to Fabian Vogt from comment #4)
> > I'll test that and make a patch that cleans it up a bit. Although I'm not
> > quite sure what "IsSecret" (which should've been "IsPassword" instead, which
> > even has a different value now!) was originally supposed to mean, I'll leave
> > it as-is.
> 
> Please test and review: https://phabricator.kde.org/D6091

Patch fixes the issue on ArchLinux x86_64 as well.