shill: vpn: Accept OpenVPN's OTP and Pkcs11.ID properties.
Also, use bit flags to control how properties behave -- i.e., whether they
persist or not, crypted, broadcasted, etc.
BUG=chromium-os:29424,chromium-os:29565
TEST=unit tests, tested on device making sure OTP is sent to openvpn
Change-Id: I9edc3b54f199b1594b7e676f72f0adfe8781c844
Reviewed-on: https://gerrit.chromium.org/gerrit/20366
Commit-Ready: Darin Petkov <petkov@chromium.org>
Reviewed-by: Darin Petkov <petkov@chromium.org>
Tested-by: Darin Petkov <petkov@chromium.org>
diff --git a/openvpn_driver.cc b/openvpn_driver.cc
index 0cf2dcd..8224afa 100644
--- a/openvpn_driver.cc
+++ b/openvpn_driver.cc
@@ -64,47 +64,51 @@
const char OpenVPNDriver::kOpenVPNScript[] = SCRIPTDIR "/openvpn-script";
// static
const OpenVPNDriver::Property OpenVPNDriver::kProperties[] = {
- { flimflam::kOpenVPNAuthNoCacheProperty, false },
- { flimflam::kOpenVPNAuthProperty, false },
- { flimflam::kOpenVPNAuthRetryProperty, false },
- { flimflam::kOpenVPNAuthUserPassProperty, false },
- { flimflam::kOpenVPNCaCertNSSProperty, false },
- { flimflam::kOpenVPNCaCertProperty, false },
- { flimflam::kOpenVPNCipherProperty, false },
- { flimflam::kOpenVPNCompLZOProperty, false },
- { flimflam::kOpenVPNCompNoAdaptProperty, false },
- { flimflam::kOpenVPNKeyDirectionProperty, false },
- { flimflam::kOpenVPNNsCertTypeProperty, false },
- { flimflam::kOpenVPNPasswordProperty, true },
- { flimflam::kOpenVPNPinProperty, false },
- { flimflam::kOpenVPNPortProperty, false },
- { flimflam::kOpenVPNProtoProperty, false },
- { flimflam::kOpenVPNProviderProperty, false },
- { flimflam::kOpenVPNPushPeerInfoProperty, false },
- { flimflam::kOpenVPNRemoteCertEKUProperty, false },
- { flimflam::kOpenVPNRemoteCertKUProperty, false },
- { flimflam::kOpenVPNRemoteCertTLSProperty, false },
- { flimflam::kOpenVPNRenegSecProperty, false },
- { flimflam::kOpenVPNServerPollTimeoutProperty, false },
- { flimflam::kOpenVPNShaperProperty, false },
- { flimflam::kOpenVPNStaticChallengeProperty, false },
- { flimflam::kOpenVPNTLSAuthContentsProperty, false },
- { flimflam::kOpenVPNTLSRemoteProperty, false },
- { flimflam::kOpenVPNUserProperty, false },
- { flimflam::kProviderHostProperty, false },
- { flimflam::kProviderNameProperty, false },
- { flimflam::kProviderTypeProperty, false },
- { kOpenVPNCertProperty, false },
- { kOpenVPNKeyProperty, false },
- { kOpenVPNPingExitProperty, false },
- { kOpenVPNPingProperty, false },
- { kOpenVPNPingRestartProperty, false },
- { kOpenVPNTLSAuthProperty, false },
- { kOpenVPNVerbProperty, false },
- { kVPNMTUProperty, false },
+ { flimflam::kOpenVPNAuthNoCacheProperty, 0 },
+ { flimflam::kOpenVPNAuthProperty, 0 },
+ { flimflam::kOpenVPNAuthRetryProperty, 0 },
+ { flimflam::kOpenVPNAuthUserPassProperty, 0 },
+ { flimflam::kOpenVPNCaCertNSSProperty, 0 },
+ { flimflam::kOpenVPNCaCertProperty, 0 },
+ { flimflam::kOpenVPNCipherProperty, 0 },
+ { flimflam::kOpenVPNClientCertIdProperty,
+ OpenVPNDriver::Property::kEphemeral | OpenVPNDriver::Property::kCrypted },
+ { flimflam::kOpenVPNCompLZOProperty, 0 },
+ { flimflam::kOpenVPNCompNoAdaptProperty, 0 },
+ { flimflam::kOpenVPNKeyDirectionProperty, 0 },
+ { flimflam::kOpenVPNNsCertTypeProperty, 0 },
+ { flimflam::kOpenVPNOTPProperty,
+ OpenVPNDriver::Property::kEphemeral | OpenVPNDriver::Property::kCrypted },
+ { flimflam::kOpenVPNPasswordProperty, OpenVPNDriver::Property::kCrypted },
+ { flimflam::kOpenVPNPinProperty, 0 },
+ { flimflam::kOpenVPNPortProperty, 0 },
+ { flimflam::kOpenVPNProtoProperty, 0 },
+ { flimflam::kOpenVPNProviderProperty, 0 },
+ { flimflam::kOpenVPNPushPeerInfoProperty, 0 },
+ { flimflam::kOpenVPNRemoteCertEKUProperty, 0 },
+ { flimflam::kOpenVPNRemoteCertKUProperty, 0 },
+ { flimflam::kOpenVPNRemoteCertTLSProperty, 0 },
+ { flimflam::kOpenVPNRenegSecProperty, 0 },
+ { flimflam::kOpenVPNServerPollTimeoutProperty, 0 },
+ { flimflam::kOpenVPNShaperProperty, 0 },
+ { flimflam::kOpenVPNStaticChallengeProperty, 0 },
+ { flimflam::kOpenVPNTLSAuthContentsProperty, 0 },
+ { flimflam::kOpenVPNTLSRemoteProperty, 0 },
+ { flimflam::kOpenVPNUserProperty, 0 },
+ { flimflam::kProviderHostProperty, 0 },
+ { flimflam::kProviderNameProperty, 0 },
+ { flimflam::kProviderTypeProperty, 0 },
+ { kOpenVPNCertProperty, 0 },
+ { kOpenVPNKeyProperty, 0 },
+ { kOpenVPNPingExitProperty, 0 },
+ { kOpenVPNPingProperty, 0 },
+ { kOpenVPNPingRestartProperty, 0 },
+ { kOpenVPNTLSAuthProperty, 0 },
+ { kOpenVPNVerbProperty, 0 },
+ { kVPNMTUProperty, 0 },
// Provided only for compatibility. crosbug.com/29286
- { flimflam::kOpenVPNMgmtEnableProperty, false },
+ { flimflam::kOpenVPNMgmtEnableProperty, 0 },
};
OpenVPNDriver::OpenVPNDriver(ControlInterface *control,
@@ -622,9 +626,12 @@
bool OpenVPNDriver::Load(StoreInterface *storage, const string &storage_id) {
for (size_t i = 0; i < arraysize(kProperties); i++) {
+ if ((kProperties[i].flags & Property::kEphemeral)) {
+ continue;
+ }
const string property = kProperties[i].property;
string value;
- bool loaded = kProperties[i].crypted ?
+ bool loaded = (kProperties[i].flags & Property::kCrypted) ?
storage->GetCryptedString(storage_id, property, &value) :
storage->GetString(storage_id, property, &value);
if (loaded) {
@@ -638,11 +645,14 @@
bool OpenVPNDriver::Save(StoreInterface *storage, const string &storage_id) {
for (size_t i = 0; i < arraysize(kProperties); i++) {
+ if ((kProperties[i].flags & Property::kEphemeral)) {
+ continue;
+ }
const string property = kProperties[i].property;
const string value = args_.LookupString(property, "");
if (value.empty()) {
storage->DeleteKey(storage_id, property);
- } else if (kProperties[i].crypted) {
+ } else if ((kProperties[i].flags & Property::kCrypted)) {
storage->SetCryptedString(storage_id, property, value);
} else {
storage->SetString(storage_id, property, value);
@@ -714,7 +724,7 @@
for (size_t i = 0; i < arraysize(kProperties); i++) {
// Never return any encrypted properties.
- if (kProperties[i].crypted) {
+ if ((kProperties[i].flags & Property::kCrypted)) {
continue;
}
diff --git a/openvpn_driver.h b/openvpn_driver.h
index f6cf7ce..1395337 100644
--- a/openvpn_driver.h
+++ b/openvpn_driver.h
@@ -113,8 +113,13 @@
FRIEND_TEST(OpenVPNDriverTest, VerifyPaths);
struct Property {
+ enum Flags {
+ kEphemeral = 1 << 0,
+ kCrypted = 1 << 1,
+ };
+
const char *property;
- bool crypted;
+ int flags;
};
static const char kOpenVPNPath[];
diff --git a/openvpn_driver_unittest.cc b/openvpn_driver_unittest.cc
index bacf277..080dbf2 100644
--- a/openvpn_driver_unittest.cc
+++ b/openvpn_driver_unittest.cc
@@ -8,6 +8,7 @@
#include <base/file_path.h>
#include <base/file_util.h>
+#include <base/string_number_conversions.h>
#include <base/string_util.h>
#include <chromeos/dbus/service_constants.h>
#include <gtest/gtest.h>
@@ -32,6 +33,7 @@
#include "shill/vpn.h"
#include "shill/vpn_service.h"
+using base::UintToString;
using std::map;
using std::string;
using std::vector;
@@ -715,6 +717,20 @@
flimflam::kOpenVPNPasswordProperty,
password))
.WillOnce(Return(true));
+ EXPECT_CALL(storage,
+ SetCryptedString(
+ kStorageID, flimflam::kOpenVPNOTPProperty, _)).Times(0);
+ EXPECT_CALL(storage,
+ SetString(
+ kStorageID, flimflam::kOpenVPNOTPProperty, _)).Times(0);
+ EXPECT_CALL(storage,
+ SetCryptedString(
+ kStorageID, flimflam::kOpenVPNClientCertIdProperty, _))
+ .Times(0);
+ EXPECT_CALL(storage,
+ SetString(
+ kStorageID, flimflam::kOpenVPNClientCertIdProperty, _))
+ .Times(0);
EXPECT_CALL(storage, DeleteKey(kStorageID, _)).Times(AnyNumber());
EXPECT_CALL(storage, DeleteKey(kStorageID, flimflam::kOpenVPNAuthProperty))
.Times(1);
@@ -811,31 +827,33 @@
EXPECT_EQ(Error::kNotFound, error.type());
}
- // This one should be write-only.
- {
+ // These ones should be write-only.
+ static const char * const kWriteOnly[] = {
+ flimflam::kOpenVPNClientCertIdProperty,
+ flimflam::kOpenVPNPasswordProperty,
+ flimflam::kOpenVPNOTPProperty
+ };
+
+ for (size_t i = 0; i < arraysize(kWriteOnly); i++) {
Error error;
string string_property;
EXPECT_FALSE(
- FindStringPropertyInStore(store, flimflam::kOpenVPNPasswordProperty,
- &string_property,
- &error));
+ FindStringPropertyInStore(
+ store, kWriteOnly[i], &string_property, &error)) << kWriteOnly[i];
// We get NotFound here instead of PermissionDenied here due to the
// implementation of ReadablePropertyConstIterator: it shields us from
// store members for which Value() would have returned an error.
- EXPECT_EQ(Error::kNotFound, error.type());
+ EXPECT_EQ(Error::kNotFound, error.type()) << kWriteOnly[i];
}
- // Write a property to the driver args using the PropertyStore interface.
- {
+ // Write properties to the driver args using the PropertyStore interface.
+ for (size_t i = 0; i < arraysize(kWriteOnly); i++) {
+ string value = "some-value-" + UintToString(i);
Error error;
- EXPECT_TRUE(store.SetStringProperty(flimflam::kOpenVPNPasswordProperty,
- kPassword1,
- &error));
+ EXPECT_TRUE(store.SetStringProperty(kWriteOnly[i], value, &error))
+ << kWriteOnly[i];
+ EXPECT_EQ(value, GetArgs()->GetString(kWriteOnly[i])) << kWriteOnly[i];
}
-
- KeyValueStore *driver_args = GetArgs();
- EXPECT_EQ(kPassword1,
- driver_args->GetString(flimflam::kOpenVPNPasswordProperty));
}
} // namespace shill