Merge lp:~khurshid-alam/unity-greeter/systemd-240-fix into lp:unity-greeter

[Khurshid Alam]

According to systemd-dev,

"mlockall() is generally a bad idea and certainly has no place in a graphical program.
A program like this uses lots of memory and it is crucial that this memory can be paged
out to relieve memory pressure."

With systemd version 239 the ulimit for RLIMIT_MEMLOCK was set to 16 MiB
and therefore the mlockall call would fail. This is lucky becasue the subsequent mmap would not fail.

With systemd version 240 the RLIMIT_MEMLOCK is now set to 64 MiB
and now the mlockall no longer fails. However, it not possible to mmap in all
the memory and because that would still exceed the MEMLOCK limit.
"
See https://bugzilla.redhat.com/show_bug.cgi?id=1662857 &
https://github.com/CanonicalLtd/lightdm/issues/55

RLIMIT_MEMLOCK = 64 MiB means, unity-greeter will most likely fail with 64 bit and
will always fail on 32 bit systems.

[Seth Arnold (seth-arnold)]

Removing mlockall() makes sense. It'd be best to replace it with an explicit mlock for the memory that contains the password, and any other memory that may reflect an interaction with the password, as well as use memset_s() or similar standards-level memory clearing functions once the password is no longer needed.

Encrypted swap would also be worthwhile.

But this fix doesn't need to wait for these other mitigations to be put in place.

Thanks

review: Approve

[Robert Ancell (robert-ancell)]

I'm happy if security is happy.

review: Approve

[Robert Ancell (robert-ancell)]

@seth-arnold - does removing this call from the daemon (https://github.com/CanonicalLtd/lightdm/issues/55) also make sense?

[Alberts Muktupāvels (muktupavels)]

Should not it be better to simply remove this then? Not add big comment and comment out code...

[Seth Arnold (seth-arnold)]

Robert, how much memory does the daemon process take?

I also agree it'd be better to just delete one line than add a dozen lines of comments. I figured whoever did the work would probably reach the same conclusion independently.

Thanks

[Robert Ancell (robert-ancell)]

In 16.04 LTS lightdm is using about 350MB.

[Sebastien Bacher (seb128)]

setting as merged but I uploaded a version deleting the lines rather than commenting

review: Approve

=== modified file 'src/unity-greeter.vala'
--- src/unity-greeter.vala	2016-11-08 23:15:47 +0000
+++ src/unity-greeter.vala	2019-02-11 17:17:50 +0000
@@ -540,8 +540,30 @@
 
     public static int main (string[] args)
     {
-        /* Protect memory from being paged to disk, as we deal with passwords */
-        Posix.mlockall (Posix.MCL_CURRENT | Posix.MCL_FUTURE);
+        /* Protect memory from being paged to disk, as we deal with passwords 
+
+           According to systemd-dev,
+
+           "mlockall() is generally a bad idea and certainly has no place in a graphical program. 
+           A program like this uses lots of memory and it is crucial that this memory can be paged 
+           out to relieve memory pressure."
+
+           With systemd version 239 the ulimit for RLIMIT_MEMLOCK was set to 16 MiB 
+           and therefore the mlockall call would fail. This is lucky becasue the subsequent mmap would not fail.
+
+           With systemd version 240 the RLIMIT_MEMLOCK is now set to 64 MiB 
+           and now the mlockall no longer fails. However, it not possible to mmap in all 
+           the memory and because that would still exceed the MEMLOCK limit.
+           "
+           See https://bugzilla.redhat.com/show_bug.cgi?id=1662857 &
+           https://github.com/CanonicalLtd/lightdm/issues/55    
+
+           RLIMIT_MEMLOCK = 64 MiB means, unity-greeter will most likely fail with 64 bit and
+           will always fail on 32 bit systems. 
+
+           Hence we better disable it. */
+
+        /*Posix.mlockall (Posix.MCL_CURRENT | Posix.MCL_FUTURE);*/
 
         /* Disable the stupid global menubar */
         Environment.unset_variable ("UBUNTU_MENUPROXY");