--auth-user-pass does not null-terminate strings from console input via systemd-ask-password

[harzaf]

Steps to Reproduce

• Linux distro with systemd (testing on Arch Linux x64)
• OpenVPN 2.3.2 running as a client
• --auth-user-pass (to read in from console; no file specified)
• Enter a username of maximum length (USER_PASS_LEN in misc.h)
• Enter any password
• Observe in a debugger/printf that Openvpn has the username=username + password (because the username string is not null terminated)
• This causes a problem accessing at least one VPN provider which uses usernames of length 128 when the binary has USER_PASS_LEN as 128. (Eg ​Cryptostorm)

Error

In console.c:

 get_console_input_systemd (const char *prompt, const bool echo, char *input, const int capacity)
{
  ...

  CLEAR (*input);
  if (read (std_out, input, capacity) != 0)
    {
       chomp (input);
       ret = true;
    }
  ...
}

The read() fills the entire input buffer, which is then no longer null-terminated string.

Fix

The ideal fix should:

1. Ensure the input is always null-terminated
2. Warn the user if the input is too long instead of silently using wrong username/password

In console.c

get_console_input_systemd (const char *prompt, const bool echo, char *input, const int capacity)
{
  ssize_t bytes;

  ...

  CLEAR (*input); 
  
  bytes = read (std_out, input, capacity);
  
  if (bytes == capacity)
    {
      /* Warn the user instead of silently truncating input */
      msg (M_FATAL, "Input too long for buffer.", prompt);
    }
  else if (bytes != 0)
    {
       chomp (input);
       ret = true;
    }
...
}

[harzaf]

Patch for console.c

[Samuli Seppänen]

Assigning to dazo because systemd.

[David Sommerseth]

This should be fixed since v2.3.11 and master+2.4_alpha1

commit 17b22f9da909f0193dff30860b45673d9cd6c509
Author: Selva Nair <selva.nair@gmail.com>
Date:   Wed Apr 13 23:53:33 2016 -0400

    Ensure input read using systemd-ask-password is null terminated
    
    Also properly check the return value of read() and leave room
    for termination.
    Fixes junk data occasionally seen in strings read through systemd.
    
    Signed-off-by: Selva Nair <selva.nair@gmail.com>
    Acked-by: Gert Doering <gert@greenie.muc.de>
    Message-Id: <1460606013-4983-1-git-send-email-selva.nair@gmail.com>
    URL: http://article.gmane.org/gmane.network.openvpn.devel/11437
    Signed-off-by: Gert Doering <gert@greenie.muc.de>
    (cherry picked from commit d09fbf958f1c0b15372b3e87d784ae666b91a96b)

In addition the systemd part got revamped as well, in master/2.4_alpha

commit 3280d8c8f3583d03fed923837447ef45c8f83d52
Author: David Sommerseth <davids@openvpn.net>
Date:   Fri Aug 12 12:57:25 2016 +0200

    Re-implement the systemd support using the new query user API
    
    This provides exactly the same systemd functionality which existed
    before the query user infrastructure got implemented.
    
      [v5 - Ensure NULL termination fix in d09fbf958f1c is included ]
    
      [v4 - change disapproved &= syntax ]
    
      [v3 - Remove QUERY_USER_EXEC_ALTERNATIVE macro, simplify
            alternatives definition directly in console.h.  For
            now only depend on ENABLE_SYSTEMD]
    
      [v2 - Removed the QUERY_USER_FOREACH macro]
    
    Signed-off-by: David Sommerseth <davids@openvpn.net>
    Acked-by: Selva Nair <selva.nair@gmail.com>
    Message-Id: 1470999445-4288-1-git-send-email-davids@openvpn.net
    URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg12424.html

This commit explicitly ensures the former patch is fixed in the new implementation as well.

[tct]

cc