Skip to content

Commit 809f8ad

Browse files
fxduponttmarkwalder
fxdupont
authored and
tmarkwalder
committed
[#3133] Updated code and tests
1 parent 998d827 commit 809f8ad

File tree

4 files changed

+184
-26
lines changed

4 files changed

+184
-26
lines changed

‎src/bin/d2/tests/d2_simple_parser_unittest.cc

Lines changed: 104 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
#include <testutils/test_to_element.h>
1313

1414
#include <boost/lexical_cast.hpp>
15+
#include <fstream>
1516

1617
using namespace isc;
1718
using namespace isc::data;
@@ -80,18 +81,21 @@ void checkStringValue(const ConstElementPtr& element,
8081
/// @param name is the value to compare against key's name_.
8182
/// @param algorithm is the string value to compare against key's algorithm.
8283
/// @param secret is the value to compare against key's secret.
84+
/// @param secret_file is the file name where the secret can be found.
85+
/// @param digestbits is the minimum truncated length in bits.
8386
///
8487
/// @return returns true if there is a match across the board, otherwise it
8588
/// returns false.
8689
bool checkKey(TSIGKeyInfoPtr key, const std::string& name,
8790
const std::string& algorithm, const std::string& secret,
88-
uint32_t digestbits = 0) {
91+
std::string secret_file, uint32_t digestbits = 0) {
8992
// Return value, assume its a match.
9093
return (((key) &&
9194
(key->getName() == name) &&
92-
(key->getAlgorithm() == algorithm) &&
95+
(key->getAlgorithm() == algorithm) &&
9396
(key->getDigestbits() == digestbits) &&
94-
(key->getSecret() == secret) &&
97+
(key->getSecret() == secret) &&
98+
(key->getSecretFile() == secret_file) &&
9599
(key->getTSIGKey())));
96100
}
97101

@@ -232,7 +236,8 @@ class D2SimpleParserTest : public ::testing::Test {
232236
/// ensure any D2 object(s) that were created by a prior invocation are
233237
/// destroyed. This permits parsing to be conducted more than once
234238
/// in the same test.
235-
virtual void reset(){};
239+
virtual void reset() {
240+
}
236241

237242
/// @brief Adds default values to the given element tree
238243
///
@@ -303,7 +308,8 @@ class TSIGKeyInfoParserTest : public D2SimpleParserTest {
303308
public:
304309
/// @brief Constructor
305310
TSIGKeyInfoParserTest()
306-
: D2SimpleParserTest(D2ParserContext::PARSER_TSIG_KEY) {
311+
: D2SimpleParserTest(D2ParserContext::PARSER_TSIG_KEY),
312+
test_file_name_(TEST_DATA_BUILDDIR "/sf-test") {
307313
}
308314

309315
/// @brief Free up the keys created by parsing
@@ -313,6 +319,7 @@ class TSIGKeyInfoParserTest : public D2SimpleParserTest {
313319

314320
/// @brief Destructor
315321
virtual ~TSIGKeyInfoParserTest() {
322+
static_cast<void>(remove(test_file_name_.c_str()));
316323
reset();
317324
};
318325

@@ -340,8 +347,10 @@ class TSIGKeyInfoParserTest : public D2SimpleParserTest {
340347

341348
/// @brief Retains the TSIGKeyInfo created by a successful parsing
342349
TSIGKeyInfoPtr key_;
343-
};
344350

351+
/// @brief Secret file name.
352+
std::string test_file_name_;
353+
};
345354

346355
/// @brief Test fixture class for testing TSIGKeyInfo list parsing.
347356
class TSIGKeyInfoListParserTest : public D2SimpleParserTest {
@@ -599,6 +608,7 @@ class DdnsDomainListParserTest : public DdnsDomainParserTest {
599608
/// 1. Name cannot be blank.
600609
/// 2. Algorithm cannot be blank.
601610
/// 3. Secret cannot be blank.
611+
/// 4. Secret file cannot be invalid when specified.
602612
TEST_F(TSIGKeyInfoParserTest, invalidEntry) {
603613

604614
// Name cannot be blank.
@@ -643,8 +653,59 @@ TEST_F(TSIGKeyInfoParserTest, invalidEntry) {
643653
"}";
644654
PARSE_FAIL(config, "Cannot make D2TsigKey: non-zero bits left over"
645655
" bogus (<string>:1:1)");
646-
}
647656

657+
// Secret file must exist.
658+
config = "{"
659+
" \"name\": \"d2_key_one\" , "
660+
" \"algorithm\": \"HMAC-MD5\" , "
661+
" \"digest-bits\": 120 , "
662+
" \"secret-file\": \"/does/not/exist\" "
663+
"}";
664+
PARSE_FAIL(config, "tsig-key : Expected a file at path "
665+
"'/does/not/exist' (<string>:1:91)");
666+
667+
// Secret file must be a regular file.
668+
config = "{"
669+
" \"name\": \"d2_key_one\" , "
670+
" \"algorithm\": \"HMAC-MD5\" , "
671+
" \"digest-bits\": 120 , "
672+
" \"secret-file\": \"/\" "
673+
"}";
674+
PARSE_FAIL(config, "tsig-key : Expected '/' to be a regular file "
675+
"(<string>:1:91)");
676+
677+
// Secret file must not be empty.
678+
std::ofstream fs(test_file_name_.c_str(),
679+
std::ofstream::out | std::ofstream::trunc);
680+
ASSERT_TRUE(fs.is_open());
681+
fs.close();
682+
683+
config = "{"
684+
" \"name\": \"d2_key_one\" , "
685+
" \"algorithm\": \"HMAC-MD5\" , "
686+
" \"digest-bits\": 120 , "
687+
" \"secret-file\": \"";
688+
config += test_file_name_ + "\" }";
689+
std::string expected = "tsig-key : Expected '";
690+
expected += test_file_name_ + "' to not be empty (<string>:1:91)";
691+
PARSE_FAIL(config, expected);
692+
693+
// Secret file content must be valid.
694+
fs = std::ofstream(test_file_name_.c_str(),
695+
std::ofstream::out | std::ofstream::trunc);
696+
ASSERT_TRUE(fs.is_open());
697+
fs << "bogus";
698+
fs.close();
699+
700+
config = "{"
701+
" \"name\": \"d2_key_one\" , "
702+
" \"algorithm\": \"HMAC-MD5\" , "
703+
" \"digest-bits\": 120 , "
704+
" \"secret-file\": \"";
705+
config += test_file_name_ + "\" }";
706+
PARSE_FAIL(config, "Cannot make D2TsigKey: non-zero bits left over"
707+
" bogus (<string>:1:1)");
708+
}
648709

649710
/// @brief Verifies that TSIGKeyInfo parsing creates a proper TSIGKeyInfo
650711
/// when given a valid combination of entries.
@@ -662,7 +723,36 @@ TEST_F(TSIGKeyInfoParserTest, validEntry) {
662723

663724
// Verify the key contents.
664725
EXPECT_TRUE(checkKey(key_, "d2_key_one", "HMAC-MD5",
665-
"dGhpcyBrZXkgd2lsbCBtYXRjaA==", 120));
726+
"dGhpcyBrZXkgd2lsbCBtYXRjaA==", "", 120));
727+
728+
// Verify unparsing.
729+
runToElementTest<TSIGKeyInfo>(config, *key_);
730+
}
731+
732+
/// @brief Verifies that TSIGKeyInfo parsing creates a proper TSIGKeyInfo
733+
/// when given a valid secret file.
734+
TEST_F(TSIGKeyInfoParserTest, validSecretFile) {
735+
// Valid entries for TSIG key, all items are required.
736+
std::string config = "{"
737+
" \"name\": \"d2_key_one\" , "
738+
" \"algorithm\": \"HMAC-MD5\" , "
739+
" \"digest-bits\": 120 , "
740+
" \"secret-file\": \"";
741+
config += test_file_name_ + "\" }";
742+
// Create and fill the secret file.
743+
std::ofstream fs(test_file_name_.c_str(),
744+
std::ofstream::out | std::ofstream::trunc);
745+
ASSERT_TRUE(fs.is_open());
746+
fs << "dGhpcyBrZXkgd2lsbCBtYXRjaA==";
747+
fs.close();
748+
// Verify that it parses.
749+
PARSE_OK(config);
750+
ASSERT_TRUE(key_);
751+
752+
// Verify the key contents.
753+
EXPECT_TRUE(checkKey(key_, "d2_key_one", "HMAC-MD5",
754+
"dGhpcyBrZXkgd2lsbCBtYXRjaA==",
755+
test_file_name_, 120));
666756

667757
// Verify unparsing.
668758
runToElementTest<TSIGKeyInfo>(config, *key_);
@@ -770,7 +860,7 @@ TEST_F(TSIGKeyInfoListParserTest, validTSIGKeyList) {
770860

771861
// Verify the key contents.
772862
EXPECT_TRUE(checkKey(key, "key1", TSIGKeyInfo::HMAC_MD5_STR,
773-
ref_secret, 80));
863+
ref_secret, "", 80));
774864

775865
// Find the 2nd key and retrieve it.
776866
gotit = keys_->find("key2");
@@ -779,7 +869,7 @@ TEST_F(TSIGKeyInfoListParserTest, validTSIGKeyList) {
779869

780870
// Verify the key contents.
781871
EXPECT_TRUE(checkKey(key, "key2", TSIGKeyInfo::HMAC_SHA1_STR,
782-
ref_secret, 80));
872+
ref_secret, "", 80));
783873

784874
// Find the 3rd key and retrieve it.
785875
gotit = keys_->find("key3");
@@ -788,7 +878,7 @@ TEST_F(TSIGKeyInfoListParserTest, validTSIGKeyList) {
788878

789879
// Verify the key contents.
790880
EXPECT_TRUE(checkKey(key, "key3", TSIGKeyInfo::HMAC_SHA256_STR,
791-
ref_secret, 128));
881+
ref_secret, "", 128));
792882

793883
// Find the 4th key and retrieve it.
794884
gotit = keys_->find("key4");
@@ -797,7 +887,7 @@ TEST_F(TSIGKeyInfoListParserTest, validTSIGKeyList) {
797887

798888
// Verify the key contents.
799889
EXPECT_TRUE(checkKey(key, "key4", TSIGKeyInfo::HMAC_SHA224_STR,
800-
ref_secret, 112));
890+
ref_secret, "", 112));
801891

802892
// Find the 5th key and retrieve it.
803893
gotit = keys_->find("key5");
@@ -806,7 +896,7 @@ TEST_F(TSIGKeyInfoListParserTest, validTSIGKeyList) {
806896

807897
// Verify the key contents.
808898
EXPECT_TRUE(checkKey(key, "key5", TSIGKeyInfo::HMAC_SHA384_STR,
809-
ref_secret, 192));
899+
ref_secret, "", 192));
810900

811901
// Find the 6th key and retrieve it.
812902
gotit = keys_->find("key6");
@@ -815,7 +905,7 @@ TEST_F(TSIGKeyInfoListParserTest, validTSIGKeyList) {
815905

816906
// Verify the key contents.
817907
EXPECT_TRUE(checkKey(key, "key6", TSIGKeyInfo::HMAC_SHA512_STR,
818-
ref_secret, 256));
908+
ref_secret, "", 256));
819909
}
820910

821911
/// @brief Tests the enforcement of data validation when parsing DnsServerInfos.

‎src/bin/d2/tests/get_config_unittest.cc

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright (C) 2017-2021 Internet Systems Consortium, Inc. ("ISC")
1+
// Copyright (C) 2017-2024 Internet Systems Consortium, Inc. ("ISC")
22
//
33
// This Source Code Form is subject to the terms of the Mozilla Public
44
// License, v. 2.0. If a copy of the MPL was not distributed with this
@@ -51,6 +51,9 @@ const bool generate_action = true;
5151
const bool generate_action = false;
5252
#endif
5353

54+
/// @brief the test secret file name.
55+
std::string test_file_name(TEST_DATA_BUILDDIR "/sf-test");
56+
5457
/// @brief Read a file into a string
5558
std::string
5659
readFile(const std::string& file_path) {
@@ -107,6 +110,28 @@ void pathReplacer(ConstElementPtr d2_cfg) {
107110
first_lib->set("library", Element::create(lib_path));
108111
}
109112

113+
/// @brief Replace the test secret file name.
114+
void testFileNameReplacer(ConstElementPtr d2_cfg) {
115+
ConstElementPtr tsig_keys = d2_cfg->get("tsig-keys");
116+
if (!tsig_keys || (tsig_keys->size() != 4)) {
117+
return;
118+
}
119+
ElementPtr third_key = tsig_keys->getNonConst(2);
120+
if (third_key->contains("secret")) {
121+
return;
122+
}
123+
third_key->set("secret-file", Element::create(test_file_name));
124+
}
125+
126+
/// @brief Create and fill the secret file.
127+
void fillSecretFile() {
128+
std::ofstream fs(test_file_name.c_str(),
129+
std::ofstream::out | std::ofstream::trunc);
130+
ASSERT_TRUE(fs.is_open());
131+
fs << "Gwk53fvy3CmbupoI9TgigA==";
132+
fs.close();
133+
}
134+
110135
}
111136

112137
/// Test fixture class
@@ -119,9 +144,12 @@ class D2GetConfigTest : public ConfigParseTest {
119144
Daemon::setVerbose(false);
120145
// Create fresh context.
121146
resetConfiguration();
147+
// Fill test secret file.
148+
fillSecretFile();
122149
}
123150

124151
~D2GetConfigTest() {
152+
static_cast<void>(remove(test_file_name.c_str()));
125153
resetConfiguration();
126154
}
127155

@@ -168,6 +196,9 @@ class D2GetConfigTest : public ConfigParseTest {
168196
// update hooks-libraries
169197
pathReplacer(d2);
170198

199+
// update tsig-keys.
200+
testFileNameReplacer(d2);
201+
171202
// try DHCPDDNS configure
172203
ConstElementPtr status;
173204
try {
@@ -270,6 +301,7 @@ TEST_F(D2GetConfigTest, sample1) {
270301
ASSERT_NO_THROW(d2 = jsonj->get("DhcpDdns"));
271302
ASSERT_TRUE(d2);
272303
pathReplacer(d2);
304+
testFileNameReplacer(d2);
273305
// check that unparsed and expected values match
274306
EXPECT_TRUE(isEquivalent(unparsed, jsonj));
275307
// check on pretty prints too

‎src/lib/d2srv/d2_config.cc

Lines changed: 33 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6,20 +6,21 @@
66

77
#include <config.h>
88

9+
#include <asiolink/io_error.h>
910
#include <d2srv/d2_log.h>
1011
#include <d2srv/d2_cfg_mgr.h>
1112
#include <dhcpsrv/parsers/dhcp_parsers.h>
1213
#include <exceptions/exceptions.h>
13-
#include <asiolink/io_error.h>
14+
#include <util/filesystem.h>
1415

1516
#include <boost/scoped_ptr.hpp>
1617
#include <boost/algorithm/string/predicate.hpp>
1718

1819
#include <sstream>
1920
#include <string>
2021

21-
using namespace isc::process;
2222
using namespace isc::data;
23+
using namespace isc::process;
2324

2425
namespace isc {
2526
namespace d2 {
@@ -127,9 +128,10 @@ const char* TSIGKeyInfo::HMAC_SHA384_STR = "HMAC-SHA384";
127128
const char* TSIGKeyInfo::HMAC_SHA512_STR = "HMAC-SHA512";
128129

129130
TSIGKeyInfo::TSIGKeyInfo(const std::string& name, const std::string& algorithm,
130-
const std::string& secret, uint32_t digestbits)
131-
:name_(name), algorithm_(algorithm), secret_(secret),
132-
digestbits_(digestbits), tsig_key_() {
131+
const std::string& secret, std::string secret_file,
132+
uint32_t digestbits)
133+
: name_(name), algorithm_(algorithm), secret_(secret),
134+
secret_file_(secret_file), digestbits_(digestbits), tsig_key_() {
133135
remakeKey();
134136
}
135137

@@ -184,8 +186,12 @@ TSIGKeyInfo::toElement() const {
184186
result->set("name", Element::create(name_));
185187
// Set algorithm
186188
result->set("algorithm", Element::create(algorithm_));
187-
// Set secret
188-
result->set("secret", Element::create(secret_));
189+
// Set secret[-file]
190+
if (!secret_file_.empty()) {
191+
result->set("secret-file", Element::create(secret_file_));
192+
} else {
193+
result->set("secret", Element::create(secret_));
194+
}
189195
// Set digest-bits
190196
result->set("digest-bits",
191197
Element::create(static_cast<int64_t>(digestbits_)));
@@ -400,7 +406,24 @@ TSIGKeyInfoParser::parse(ConstElementPtr key_config) {
400406
std::string name = getString(key_config, "name");
401407
std::string algorithm = getString(key_config, "algorithm");
402408
uint32_t digestbits = getInteger(key_config, "digest-bits");
403-
std::string secret = getString(key_config, "secret");
409+
std::string secret_file;
410+
std::string secret;
411+
if (key_config->contains("secret-file")) {
412+
secret_file = getString(key_config, "secret-file");
413+
try {
414+
secret = util::file::getContent(secret_file);
415+
if (secret.empty()) {
416+
isc_throw(BadValue, "Expected '" << secret_file
417+
<< "' to not be empty");
418+
}
419+
} catch (const std::exception& ex) {
420+
isc_throw(D2CfgError, "tsig-key : " << ex.what()
421+
<< " (" << getPosition("secret-file", key_config)
422+
<< ")");
423+
}
424+
} else {
425+
secret = getString(key_config, "secret");
426+
}
404427
ConstElementPtr user_context = key_config->get("user-context");
405428

406429
// Algorithm must be valid.
@@ -434,7 +457,8 @@ TSIGKeyInfoParser::parse(ConstElementPtr key_config) {
434457
// with an invalid secret content.
435458
TSIGKeyInfoPtr key_info;
436459
try {
437-
key_info.reset(new TSIGKeyInfo(name, algorithm, secret, digestbits));
460+
key_info.reset(new TSIGKeyInfo(name, algorithm, secret,
461+
secret_file, digestbits));
438462
} catch (const std::exception& ex) {
439463
isc_throw(D2CfgError, ex.what() << " ("
440464
<< key_config->getPosition() << ")");

0 commit comments

Comments
 (0)