commit bb985105b95110ae756be1b570a5e9084694785a
author Paul Stewart <pstew@chromium.org> Thu Jun 27 16:51:11 2013 -0700
committer ChromeBot <chrome-bot@google.com> Fri Jun 28 10:27:34 2013 -0700
tree 5e48ac51d44d4c564692ce03c1fbe2d1df48179f
parent 08757aa8221a55004d3ab0a6405c8e0113554fa7

shill: OpenVPNDriver: Fix user authentication default setting

By default, OpenVPN is started with user authentication enabled
if no certificate credentials are given.  It is explicitly enabled
if either user authentication is explicitly enabled or if a username
is provided.  However until this change, if a pkcs11 certificate ID
was provided, OpenVPN would by default also use user authentication
even if no user credentials were provided.  This change disables
user authentication by default in the pkcs11 certificate ID case,
similar to certificate authentication.

BUG=chromium:249363
TEST=Unit test.  I ran into this problem while writing the TPM
flavor of the OpenVPN network_VPNConnect autotest, and verified
functionality using this test (as of https://gerrit.chromium.org/gerrit/60296)

Change-Id: Ia59ff7717d57d2fa0d350db1d78f50171cf05a26
Reviewed-on: https://gerrit.chromium.org/gerrit/60294
Reviewed-by: Paul Stewart <pstew@chromium.org>
Tested-by: Paul Stewart <pstew@chromium.org>
Commit-Queue: Paul Stewart <pstew@chromium.org>

openvpn_driver.cc

diff --git a/openvpn_driver.cc b/openvpn_driver.cc
index db668df..a8706fd 100644
--- a/openvpn_driver.cc
+++ b/openvpn_driver.cc
@@ -724,7 +724,8 @@
 }
 
 void OpenVPNDriver::InitClientAuthOptions(vector<string> *options) {
-  bool has_cert = AppendValueOption(kOpenVPNCertProperty, "--cert", options);
+  bool has_cert = AppendValueOption(kOpenVPNCertProperty, "--cert", options) ||
+      !args()->LookupString(flimflam::kOpenVPNClientCertIdProperty, "").empty();
   bool has_key = AppendValueOption(kOpenVPNKeyProperty, "--key", options);
   // If the AuthUserPass property is set, or the User property is non-empty, or
   // there's neither a key, nor a cert available, specify user-password client

openvpn_driver_unittest.cc

diff --git a/openvpn_driver_unittest.cc b/openvpn_driver_unittest.cc
index b8788b5..4a356e9 100644
--- a/openvpn_driver_unittest.cc
+++ b/openvpn_driver_unittest.cc
@@ -602,7 +602,7 @@
   ExpectInFlags(options, "--pkcs11-id", kID);
   ExpectInFlags(options, "--ca", OpenVPNDriver::kDefaultCACertificates);
   ExpectInFlags(options, "--syslog");
-  ExpectInFlags(options, "--auth-user-pass");
+  ExpectNotInFlags(options, "--auth-user-pass");
 }
 
 TEST_F(OpenVPNDriverTest, InitOptionsHostWithPort) {
@@ -737,6 +737,45 @@
   driver_->InitClientAuthOptions(&options);
   ExpectInFlags(options, "--auth-user-pass");
   ExpectInFlags(options, "--key", kTestValue);
+
+  // Empty PKCS11 certificate id, no user/password/cert.
+  options.clear();
+  RemoveStringArg(kOpenVPNKeyProperty);
+  RemoveStringArg(kOpenVPNCertProperty);
+  RemoveStringArg(flimflam::kOpenVPNUserProperty);
+  SetArg(flimflam::kOpenVPNClientCertIdProperty, "");
+  driver_->InitClientAuthOptions(&options);
+  ExpectInFlags(options, "--auth-user-pass");
+  ExpectNotInFlags(options, "--key");
+  ExpectNotInFlags(options, "--cert");
+  ExpectNotInFlags(options, "--pkcs11-id");
+
+  // Non-empty PKCS11 certificate id, no user/password/cert.
+  options.clear();
+  SetArg(flimflam::kOpenVPNClientCertIdProperty, kTestValue);
+  driver_->InitClientAuthOptions(&options);
+  ExpectNotInFlags(options, "--auth-user-pass");
+  ExpectNotInFlags(options, "--key");
+  ExpectNotInFlags(options, "--cert");
+  // The "--pkcs11-id" option is added in InitPKCS11Options(), not here.
+  ExpectNotInFlags(options, "--pkcs11-id");
+
+  // PKCS11 certificate id available, AuthUserPass set.
+  options.clear();
+  SetArg(flimflam::kOpenVPNAuthUserPassProperty, kTestValue);
+  driver_->InitClientAuthOptions(&options);
+  ExpectInFlags(options, "--auth-user-pass");
+  ExpectNotInFlags(options, "--key");
+  ExpectNotInFlags(options, "--cert");
+
+  // PKCS11 certificate id available, User set.
+  options.clear();
+  RemoveStringArg(flimflam::kOpenVPNAuthUserPassProperty);
+  SetArg(flimflam::kOpenVPNUserProperty, "user");
+  driver_->InitClientAuthOptions(&options);
+  ExpectInFlags(options, "--auth-user-pass");
+  ExpectNotInFlags(options, "--key");
+  ExpectNotInFlags(options, "--cert");
 }
 
 TEST_F(OpenVPNDriverTest, InitPKCS11Options) {