New issue
Advanced search Search tips

Issue 886911 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Oct 1
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug

Blocking:
issue 884511



Sign in to add a comment

shill fails to escape openvpn usernames properly

Project Member Reported by mnissler@chromium.org, Sep 19

Issue description

Spin-off from issue 884511:

It was found that the shill OpenVPN management server does not escape the username when sent to the OpenVPN process [4], which allows for the injection of newline characters onto the wire.

[4] https://chromium.googlesource.com/chromiumos/platform2/+/master/shill/vpn/openvpn_management_server.cc#470


We should either escape properly or catch strings with funny characters and throw an error. While at it, check whether other parameters are similarly affected.
 
Cc: briannorris@chromium.org
Cc: benchan@chromium.org
Labels: -Pri-3 Pri-1
Owner: benchan@chromium.org
Status: Started (was: Available)
Project Member

Comment 4 by bugdroid1@chromium.org, Sep 29

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/a30b0379c5c241f55199ef60dbe5575bf0f9e23e

commit a30b0379c5c241f55199ef60dbe5575bf0f9e23e
Author: Ben Chan <benchan@chromium.org>
Date: Sat Sep 29 11:27:32 2018

shill: openvpn: escape username and tag sent to OpenVPN

According to the OpenVPN documentation
(openvpn/doc/management-notes.txt), the username and password passed to
OpenVPN through the OpenVPN Management interface can be in quotes where
special characters like double quote and backslash must be escaped with
with a leading backslash. This CL fixes shill to properly escape the
username as well as the tag associated with the username/password
command passed to OpenVPN. Practically speaking, the tag associated with
the username/password command is extracted from a prior notification
from OpenVPN, which shouldn't contain special characters that need
escaping. But to be extra paranoid, the tag is escaped as well.

BUG= chromium:886911 
TEST=Run unit tests.
TEST=Run network_VPNConnect.openvpn* tests.

Change-Id: I7c9cd715c110cf7cba16316c9d3a2f78fc348952
Reviewed-on: https://chromium-review.googlesource.com/1250131
Commit-Ready: Ben Chan <benchan@chromium.org>
Tested-by: Ben Chan <benchan@chromium.org>
Reviewed-by: Jorge Lucangeli Obes <jorgelo@chromium.org>
Reviewed-by: Brian Norris <briannorris@chromium.org>

[modify] https://crrev.com/a30b0379c5c241f55199ef60dbe5575bf0f9e23e/shill/vpn/openvpn_management_server.cc
[modify] https://crrev.com/a30b0379c5c241f55199ef60dbe5575bf0f9e23e/shill/vpn/openvpn_management_server.h
[modify] https://crrev.com/a30b0379c5c241f55199ef60dbe5575bf0f9e23e/shill/vpn/openvpn_management_server_test.cc

Status: Fixed (was: Started)
Project Member

Comment 6 by sheriffbot@chromium.org, Oct 2

Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify
Project Member

Comment 7 by sheriffbot@chromium.org, Jan 8

Labels: -Restrict-View-SecurityNotify allpublic
This bug has been closed for more than 14 weeks. Removing security view restrictions.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Sign in to add a comment