Opened 10 years ago

Closed 8 years ago

Last modified 5 years ago

#368 closed Bug / Defect (fixed)

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

Reported by: harzaf Owned by: David Sommerseth
Priority: major Milestone: release 2.3.11
Component: Generic / unclassified Version: OpenVPN 2.3.2 (Community Ed)
Severity: Not set (select this one, unless your'e a OpenVPN developer) Keywords:


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)


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.


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;

Attachments (1)

console.c.patch (1000 bytes) - added by harzaf 10 years ago.
Patch for console.c

Download all attachments as: .zip

Change History (5)

Changed 10 years ago by harzaf

Attachment: console.c.patch added

Patch for console.c

comment:1 Changed 10 years ago by Samuli Seppänen

Owner: set to David Sommerseth
Status: newassigned

Assigning to dazo because systemd.

comment:2 Changed 9 years ago by Samuli Seppänen

Milestone: release 2.5

comment:3 Changed 8 years ago by David Sommerseth

Milestone: release 2.5release 2.3.11
Resolution: fixed
Status: assignedclosed

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

commit 17b22f9da909f0193dff30860b45673d9cd6c509
Author: Selva Nair <>
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 <>
    Acked-by: Gert Doering <>
    Message-Id: <>
    Signed-off-by: Gert Doering <>
    (cherry picked from commit d09fbf958f1c0b15372b3e87d784ae666b91a96b)

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

commit 3280d8c8f3583d03fed923837447ef45c8f83d52
Author: David Sommerseth <>
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 <>
    Acked-by: Selva Nair <>

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

comment:4 Changed 5 years ago by tct


Note: See TracTickets for help on using tickets.