Race condition in pam_motd

[dana]

This is kind of esoteric, but it seems there is a race condition in pam_motd: If two SSH sessions are started at almost the same time, the MOTD will appear truncated in the earlier session.

I have replicated this on both Xenial and (after accounting for bug # 1368864) Trusty. Here's an illustration of what happens with the default MOTD stuff that comes with Trusty server:

# Single SSH session: MOTD looks fine

workstation:~ % ssh server
Welcome to Ubuntu 14.04.3 LTS (GNU/Linux 3.13.0-112-generic x86_64)

  System information as of Thu Apr 27 21:26:21 CDT 2017

  System load: 0.03 Processes: 114
  Usage of /: 45.5% of 13.40GB Users logged in: 0
  Memory usage: 8% IP address for eth0: 1.2.3.4
  Swap usage: 0% IP address for eth1: 5.6.7.8

71 packages can be updated.
0 updates are security updates.

New release '16.04.2 LTS' available.
Run 'do-release-upgrade' to upgrade to it.

*** System restart required ***
Last login: Thu Apr 27 21:25:19 2017 from workstation
server:~ %

# Multiple SSH sessions: MOTD is truncated

workstation:~ % ( sleep 0.25; ssh server /bin/true ) > /dev/null 2>&1 & ssh server
[1] 93182
Welcome to Ubuntu 14.04.3 LTS (GNU/Linux 3.13.0-112-generic x86_64)

  System information as of Thu Apr 27 21:26:45 CDT 2017

Last login: Thu Apr 27 21:26:21 2017 from workstation
[1] + done ( sleep 0.25; ssh server /bin/true; ) > /dev/null 2>&1
server:~ %

You might have to play with the `sleep` time to get it to happen.

Obviously this is a contrived scenario. The actual reason i keep running into the problem is that i use OpenSSH's LocalCommand directive to rsync (via SSH) my dotfiles to the servers i manage. As soon as i connect to the server, but before the remote shell starts, OpenSSH fires the rsync command, which begins a new SSH session, which triggers the race condition. I suppose the same thing would happen 'in the wild' if two different users connected at exactly the same time, but that's unlikely too. It happens with the LocalCommand thing probably over half of the time though.

An attempt is made in pam_motd.c to make the file-update process atomic, by dumping the contents to /run/motd.dynamic.new before renaming it to /run/motd.dynamic -- but obviously that doesn't help here. I was able to fix the problem by simply appending the current PID to the temp-file name, like so:

if (do_update && (stat("/etc/update-motd.d", &st) == 0)
    && S_ISDIR(st.st_mode))
{
    mode_t old_mask = umask(0022);

    char tmp[64];
    char cmd[256];

    sprintf(tmp, "/run/motd.dynamic.new.%d", getpid());

    sprintf(cmd, "/usr/bin/env -i PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin run-parts --lsbsysinit /etc/update-motd.d > %s", tmp);

    if (!system(cmd))
        rename(tmp, "/run/motd.dynamic");

    umask(old_mask);
}

This is probably broken somehow because i'm horrible at C -- plus i couldn't get the update-motd patch in the libpam-modules package source to apply cleanly -- otherwise i'd supply my own patch. But i think you see what i mean.

cheers