shill: L2TPIpsecDriver: Use PEM arrays for CACertPEM
Switch the "CACertPEM" option to use a string array of PEM.
strongSwan is equally capable of accepting PEM and DER
certificates in its CA certificate store. Since it's not easy
to export multiple certificates in a single DER file, use
a PEM file instead of DER for the output file in the CaCertPEM
case. Since this is the last user of CreateDERFromString(),
remove this method from CertificateFile. Since we no longer
generate DER, there is no longer a need for CertificateFile
to hold a GLib instance.
CQ-DEPEND=CL:60542
BUG=chromium:249363
TEST=Unit tests.
Change-Id: I9acde1f165006458ea0e1cd34e5037fad4784feb
Reviewed-on: https://gerrit.chromium.org/gerrit/60557
Reviewed-by: Darin Petkov <petkov@chromium.org>
Commit-Queue: Paul Stewart <pstew@chromium.org>
Tested-by: Paul Stewart <pstew@chromium.org>
diff --git a/certificate_file.cc b/certificate_file.cc
index 4083a6a..a7dacce 100644
--- a/certificate_file.cc
+++ b/certificate_file.cc
@@ -14,7 +14,6 @@
#include <base/string_util.h>
#include <base/stringprintf.h>
-#include "shill/glib.h"
#include "shill/logging.h"
using base::FilePath;
@@ -30,9 +29,8 @@
const char CertificateFile::kPEMHeader[] = "-----BEGIN CERTIFICATE-----";
const char CertificateFile::kPEMFooter[] = "-----END CERTIFICATE-----";
-CertificateFile::CertificateFile(GLib *glib)
- : root_directory_(FilePath(kDefaultRootDirectory)),
- glib_(glib) {
+CertificateFile::CertificateFile()
+ : root_directory_(FilePath(kDefaultRootDirectory)) {
SLOG(Crypto, 2) << __func__;
}
@@ -57,17 +55,6 @@
return WriteFile(JoinString(pem_output, ""));
}
-FilePath CertificateFile::CreateDERFromString(const string &pem_contents) {
- string hex_data = ExtractHexData(pem_contents);
- string der_contents;
- if (!glib_->B64Decode(hex_data, &der_contents)) {
- LOG(ERROR) << "Could not decode hex data from input PEM";
- return FilePath();
- }
-
- return WriteFile(der_contents);
-}
-
// static
string CertificateFile::ExtractHexData(const std::string &pem_data) {
bool found_header = false;
diff --git a/certificate_file.h b/certificate_file.h
index c3b87a1..0a9f510 100644
--- a/certificate_file.h
+++ b/certificate_file.h
@@ -12,15 +12,13 @@
namespace shill {
-class GLib;
-
// Creates a scoped temporary file containing the DER or PEM
// equivalent of an input PEM-format certificate. When this object
// is destroyed (or a different file is created from the same object)
// the previous temporary file is destroyed.
class CertificateFile {
public:
- explicit CertificateFile(GLib *glib);
+ CertificateFile();
virtual ~CertificateFile();
// Write out a PEM file from an input vector of strings in PEM format.
@@ -28,10 +26,6 @@
virtual base::FilePath CreatePEMFromStrings(
const std::vector<std::string> &pem_contents);
- // Write out a DER file from an input string in PEM format.
- // Returns an empty path on failure.
- virtual base::FilePath CreateDERFromString(const std::string &pem_contents);
-
// Setters.
void set_root_directory(const base::FilePath &root_directory) {
root_directory_ = root_directory;
@@ -64,9 +58,6 @@
// File path for the created temporary file.
base::FilePath output_file_;
- // GLib instance.
- GLib *glib_;
-
DISALLOW_COPY_AND_ASSIGN(CertificateFile);
};
diff --git a/certificate_file_unittest.cc b/certificate_file_unittest.cc
index 5c957a0..bc63d63 100644
--- a/certificate_file_unittest.cc
+++ b/certificate_file_unittest.cc
@@ -11,10 +11,9 @@
#include <base/file_util.h>
#include <base/files/scoped_temp_dir.h>
#include <base/stringprintf.h>
+#include <gmock/gmock.h>
#include <gtest/gtest.h>
-#include "shill/mock_glib.h"
-
using base::FilePath;
using base::StringPrintf;
using std::string;
@@ -28,7 +27,7 @@
class CertificateFileTest : public testing::Test {
public:
- CertificateFileTest() : certificate_file_(&glib_) {}
+ CertificateFileTest() {}
virtual void SetUp() {
CHECK(temp_dir_.CreateUniqueTempDir());
@@ -51,7 +50,6 @@
const char *GetPEMFooter() { return CertificateFile::kPEMFooter; }
CertificateFile certificate_file_;
- MockGLib glib_;
base::ScopedTempDir temp_dir_;
base::FilePath certificate_directory_;
};
@@ -102,25 +100,6 @@
EXPECT_TRUE(file_util::PathExists(outfile1));
}
-TEST_F(CertificateFileTest, CreateDERFromString) {
- // Create a DER file from the inner HEX data.
- const string kPEMString = kPEMData;
- const string fake_data("this is a fake");
-
- EXPECT_CALL(glib_, B64Decode(StrEq(kPEMString), _))
- .WillOnce(Return(false))
- .WillOnce(DoAll(SetArgumentPointee<1>(fake_data),
- Return(true)));
- EXPECT_TRUE(certificate_file_.CreateDERFromString(kPEMData).empty());
-
- FilePath outfile = certificate_file_.CreateDERFromString(kPEMData);
- EXPECT_FALSE(outfile.empty());
- EXPECT_TRUE(file_util::PathExists(outfile));
- string file_string;
- EXPECT_TRUE(file_util::ReadFileToString(outfile, &file_string));
- EXPECT_EQ(fake_data, file_string);
-}
-
TEST_F(CertificateFileTest, ExtractHexData) {
EXPECT_EQ("", ExtractHexData(""));
EXPECT_EQ("foo\n", ExtractHexData("foo"));
@@ -142,7 +121,7 @@
TEST_F(CertificateFileTest, Destruction) {
FilePath outfile;
{
- CertificateFile certificate_file(&glib_);
+ CertificateFile certificate_file;
certificate_file.set_root_directory(temp_dir_.path());
outfile = certificate_file.CreatePEMFromStrings(vector<string>{ kPEMData });
EXPECT_TRUE(file_util::PathExists(outfile));
diff --git a/ethernet.cc b/ethernet.cc
index d37af1e..38786d2 100644
--- a/ethernet.cc
+++ b/ethernet.cc
@@ -61,7 +61,6 @@
is_eap_detected_(false),
eap_listener_(new EapListener(dispatcher, interface_index)),
nss_(NSS::GetInstance()),
- certificate_file_(manager->glib()),
proxy_factory_(ProxyFactory::GetInstance()),
weak_ptr_factory_(this) {
PropertyStore *store = this->mutable_store();
diff --git a/l2tp_ipsec_driver.cc b/l2tp_ipsec_driver.cc
index ea96ce0..f8a6c77 100644
--- a/l2tp_ipsec_driver.cc
+++ b/l2tp_ipsec_driver.cc
@@ -76,7 +76,7 @@
{ flimflam::kL2tpIpsecUserProperty, 0 },
{ flimflam::kProviderHostProperty, 0 },
{ flimflam::kProviderTypeProperty, 0 },
- { kL2tpIpsecCaCertPemProperty, 0 },
+ { kL2tpIpsecCaCertPemProperty, Property::kArray },
{ kL2tpIpsecTunnelGroupProperty, 0 },
{ kL2TPIPSecIPSecTimeoutProperty, 0 },
{ kL2TPIPSecLeftProtoPortProperty, 0 },
@@ -101,7 +101,7 @@
device_info_(device_info),
glib_(glib),
nss_(NSS::GetInstance()),
- certificate_file_(new CertificateFile(glib)),
+ certificate_file_(new CertificateFile()),
weak_ptr_factory_(this) {}
L2TPIPSecDriver::~L2TPIPSecDriver() {
@@ -292,13 +292,16 @@
}
bool L2TPIPSecDriver::InitPEMOptions(vector<string> *options) {
- string ca_cert = args()->LookupString(kL2tpIpsecCaCertPemProperty, "");
- if (ca_cert.empty()) {
+ vector<string> ca_certs;
+ if (args()->ContainsStrings(kL2tpIpsecCaCertPemProperty)) {
+ ca_certs = args()->GetStrings(kL2tpIpsecCaCertPemProperty);
+ }
+ if (ca_certs.empty()) {
return false;
}
- FilePath certfile = certificate_file_->CreateDERFromString(ca_cert);
+ FilePath certfile = certificate_file_->CreatePEMFromStrings(ca_certs);
if (certfile.empty()) {
- LOG(ERROR) << "Unable to extract certificate from PEM string.";
+ LOG(ERROR) << "Unable to extract certificates from PEM string.";
return false;
}
options->push_back("--server_ca_file");
diff --git a/l2tp_ipsec_driver_unittest.cc b/l2tp_ipsec_driver_unittest.cc
index fb3f61b..7de16b0 100644
--- a/l2tp_ipsec_driver_unittest.cc
+++ b/l2tp_ipsec_driver_unittest.cc
@@ -79,6 +79,10 @@
driver_->args()->SetString(arg, value);
}
+ void SetArgArray(const string &arg, const vector<string> &value) {
+ driver_->args()->SetStrings(arg, value);
+ }
+
KeyValueStore *GetArgs() {
return driver_->args();
}
@@ -300,12 +304,12 @@
}
TEST_F(L2TPIPSecDriverTest, InitPEMOptions) {
- static const char kCaCertPEM[] = "Insert PEM encoded data here";
+ const vector<string> kCaCertPEM{ "Insert PEM encoded data here" };
static const char kPEMCertfile[] = "/tmp/der-file-from-pem-cert";
FilePath empty_cert;
FilePath pem_cert(kPEMCertfile);
- SetArg(kL2tpIpsecCaCertPemProperty, kCaCertPEM);
- EXPECT_CALL(*certificate_file_, CreateDERFromString(kCaCertPEM))
+ SetArgArray(kL2tpIpsecCaCertPemProperty, kCaCertPEM);
+ EXPECT_CALL(*certificate_file_, CreatePEMFromStrings(kCaCertPEM))
.WillOnce(Return(empty_cert))
.WillOnce(Return(pem_cert));
diff --git a/mock_certificate_file.cc b/mock_certificate_file.cc
index 9cfcf7e..b110adb 100644
--- a/mock_certificate_file.cc
+++ b/mock_certificate_file.cc
@@ -6,7 +6,7 @@
namespace shill {
-MockCertificateFile::MockCertificateFile() : CertificateFile(NULL) {}
+MockCertificateFile::MockCertificateFile() : CertificateFile() {}
MockCertificateFile::~MockCertificateFile() {}
diff --git a/mock_certificate_file.h b/mock_certificate_file.h
index e1f1a39..fe12e85 100644
--- a/mock_certificate_file.h
+++ b/mock_certificate_file.h
@@ -20,8 +20,6 @@
base::FilePath(const std::string &pem_contents));
MOCK_METHOD1(CreatePEMFromStrings,
base::FilePath(const std::vector<std::string> &pem_contents));
- MOCK_METHOD1(CreateDERFromString,
- base::FilePath(const std::string &pem_contents));
private:
DISALLOW_COPY_AND_ASSIGN(MockCertificateFile);
diff --git a/openvpn_driver.cc b/openvpn_driver.cc
index a8706fd..1b45bbc 100644
--- a/openvpn_driver.cc
+++ b/openvpn_driver.cc
@@ -137,7 +137,7 @@
glib_(glib),
management_server_(new OpenVPNManagementServer(this, glib)),
nss_(NSS::GetInstance()),
- certificate_file_(new CertificateFile(glib)),
+ certificate_file_(new CertificateFile()),
process_killer_(ProcessKiller::GetInstance()),
lsb_release_file_(kLSBReleaseFile),
pid_(0),
diff --git a/wifi_service.cc b/wifi_service.cc
index 22e69c8..ae124a3 100644
--- a/wifi_service.cc
+++ b/wifi_service.cc
@@ -74,7 +74,7 @@
ssid_(ssid),
ieee80211w_required_(false),
nss_(NSS::GetInstance()),
- certificate_file_(new CertificateFile(manager->glib())),
+ certificate_file_(new CertificateFile()),
provider_(provider) {
PropertyStore *store = this->mutable_store();
store->RegisterConstString(flimflam::kModeProperty, &mode_);