BIND 10 master, updated. 432a1a86cea043993348f31d14628548e267e3b8 Changelog
BIND 10 source code commits
bind10-changes at lists.isc.org
Thu May 26 18:16:30 UTC 2011
The branch, master has been updated
via 432a1a86cea043993348f31d14628548e267e3b8 (commit)
via fe5e7003544e4e8f18efa7b466a65f336d8c8e4d (commit)
via 5178a891fb3713609ade921d010bed453498f355 (commit)
via e9975a7383a299789fd18b184880a00dfb0e15ed (commit)
via c8cf64492fbb3329d10b15ff660c9f262e46ae1f (commit)
via 1ccec996459f9cea1bc32b6d736692fd087aae0e (commit)
via 4f06531cf0b40476bbfd7d6a4312bacb1d958ffe (commit)
via e3a81c18ff3f55727c1308bb894a8eb03e8f48b1 (commit)
via d6b120403a24c9c233fa70ff8fca4a5da39f209e (commit)
via e156ec53472f22bbb890ebc76a1acdd9a9eda70c (commit)
via d694e5ded809ef2842eb5101e4d03e6e657e4671 (commit)
via 5d33e8c43d18b6aef9461086c8fbd5ee493b55df (commit)
via b2c418d1d6aed9e2eb599941218e1f8c0d13a445 (commit)
via 48fab55d59089444ac9dbd81ea3c6978f55f9474 (commit)
via efb25130e7c98335f72741cb62c9b3cf4ab076b3 (commit)
via e37672ef8e814456d53772220d885c91941db7bd (commit)
via ec6446047b5007290ca4d6d31e67239c424f33bb (commit)
via 990a0cff9891fce08b3e163720dfa08fffdede5f (commit)
via 54f86d7796c55289518befaefdcdd7d84ebefa88 (commit)
via f39c8aea4a0f9dc4b910c003b336e124149ab88b (commit)
via 5dcd9589aff3bc2a91ad2dbef98cd9012ed9b637 (commit)
via 857abf9ee880aecc9039b993e73683f1a6019cf6 (commit)
via 5db2d64ddcd9f4eec0db6bbb6a160c95ac81cc6e (commit)
via 37ded0b31a0a0dcadb93da7bd85248b90d9af57f (commit)
via 67d9442a23eb4fab4dff3f4d1ab142948e21e880 (commit)
via 5f74b5f0820d7bf4aa50d0341c9316e6a915738e (commit)
via 030c77386b5018d30282cda2fb6aabe620a0f15b (commit)
via a206dbd5432240440b6bbc63ff3165ad579d5efb (commit)
via 6342eda010ae512cb972e70f3824ca0de638c293 (commit)
via ff008c094a577e61f619cc39df2eab858dee0fe5 (commit)
via a8307030f7af9fc88e3e66b6eefcc89f6b6e15c5 (commit)
via 07a778c5592002042269785ec41bf4d93fc7db91 (commit)
via 9cc0c06cac86d3460ee1f5b5e2c8669d9709663e (commit)
via b8da54961c78a690c6bf02618d4c28fb9d320177 (commit)
via af788f1fef2cbdb63b05f14106f907dff87fd6bf (commit)
from 67727b623a4889ee0e38bd0ad9aaba710e722e4f (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
- Log -----------------------------------------------------------------
commit 432a1a86cea043993348f31d14628548e267e3b8
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Thu May 26 20:16:07 2011 +0200
Changelog
commit fe5e7003544e4e8f18efa7b466a65f336d8c8e4d
Merge: 67727b623a4889ee0e38bd0ad9aaba710e722e4f 5178a891fb3713609ade921d010bed453498f355
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Thu May 26 20:03:01 2011 +0200
Merge branch 'work/sign2'
commit 5178a891fb3713609ade921d010bed453498f355
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Thu May 26 11:16:36 2011 +0200
[trac931] Comment
commit e9975a7383a299789fd18b184880a00dfb0e15ed
Author: JINMEI Tatuya <jinmei at isc.org>
Date: Wed May 25 14:32:16 2011 -0700
[trac931] added a test case with the default keyring config.
also added a note in updateKeyring() about why we need to catch the case
of list==NULL explicitly.
commit c8cf64492fbb3329d10b15ff660c9f262e46ae1f
Author: JINMEI Tatuya <jinmei at isc.org>
Date: Wed May 25 13:58:32 2011 -0700
[trac931] corrected a (non related) typo I happened to notice.
commit 1ccec996459f9cea1bc32b6d736692fd087aae0e
Author: JINMEI Tatuya <jinmei at isc.org>
Date: Wed May 25 13:52:42 2011 -0700
[trac931] corrected the document added in the previous commit a bit.
commit 4f06531cf0b40476bbfd7d6a4312bacb1d958ffe
Author: JINMEI Tatuya <jinmei at isc.org>
Date: Wed May 25 13:43:01 2011 -0700
[trac931] added a test to reproduce the problem we had with addRemoteConfig.
Also added a note to the addRemoteConfig() document that when it's called
for a module the module cc session must not have been "started".
commit e3a81c18ff3f55727c1308bb894a8eb03e8f48b1
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Wed May 25 13:43:05 2011 +0200
[trac931] Tweaks of addRemoteConfig
Small refactoring, one more test
commit d6b120403a24c9c233fa70ff8fca4a5da39f209e
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Wed May 25 13:35:24 2011 +0200
[trac931] Remove stray experiment
commit e156ec53472f22bbb890ebc76a1acdd9a9eda70c
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Wed May 25 13:33:32 2011 +0200
[trac931] Different test for the double read
commit d694e5ded809ef2842eb5101e4d03e6e657e4671
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Wed May 25 12:42:06 2011 +0200
Revert "[trac931] Use uint32_t for fixed-sized lengths"
This reverts commit 6342eda010ae512cb972e70f3824ca0de638c293.
commit 5d33e8c43d18b6aef9461086c8fbd5ee493b55df
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Wed May 25 12:40:04 2011 +0200
Revert the double read bug test
Revert "[trac931_2] make sure the workaround -Wno-unused-parameter follows -Wextra"
Revert "[trac931] Disable warning on one needed file"
Revert "[trac931] Missing includes"
Revert "[trac931] Test reproducing the double read bug"
This reverts commit ec6446047b5007290ca4d6d31e67239c424f33bb.
This reverts commit 990a0cff9891fce08b3e163720dfa08fffdede5f.
This reverts commit 54f86d7796c55289518befaefdcdd7d84ebefa88.
This reverts commit 37ded0b31a0a0dcadb93da7bd85248b90d9af57f.
commit b2c418d1d6aed9e2eb599941218e1f8c0d13a445
Author: JINMEI Tatuya <jinmei at isc.org>
Date: Tue May 24 15:23:20 2011 -0700
[trac931] constify as much as possible
commit 48fab55d59089444ac9dbd81ea3c6978f55f9474
Author: JINMEI Tatuya <jinmei at isc.org>
Date: Tue May 24 13:56:11 2011 -0700
[trac931] clanup: removed redundant blank lines
commit efb25130e7c98335f72741cb62c9b3cf4ab076b3
Author: JINMEI Tatuya <jinmei at isc.org>
Date: Tue May 24 13:34:40 2011 -0700
[trac931] comment cleanup
commit e37672ef8e814456d53772220d885c91941db7bd
Author: JINMEI Tatuya <jinmei at isc.org>
Date: Tue May 24 13:07:50 2011 -0700
[trac931] corrected a confusing comment in addRemoteConfig().
added some comments about how the remote config value is retrieved
in a test case.
commit ec6446047b5007290ca4d6d31e67239c424f33bb
Author: JINMEI Tatuya <jinmei at isc.org>
Date: Tue May 24 00:15:41 2011 -0700
[trac931_2] make sure the workaround -Wno-unused-parameter follows -Wextra
commit 990a0cff9891fce08b3e163720dfa08fffdede5f
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Mon May 23 11:34:51 2011 +0200
[trac931] Disable warning on one needed file
commit 54f86d7796c55289518befaefdcdd7d84ebefa88
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Mon May 23 11:21:54 2011 +0200
[trac931] Missing includes
commit f39c8aea4a0f9dc4b910c003b336e124149ab88b
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Fri May 20 14:04:11 2011 +0200
[trac931] Decouple the server from keyring
It now uses a pointer that is set to the global variable instead of the
global variable directly. This makes tests easier as they don't need to
set it back and is more flexible.
commit 5dcd9589aff3bc2a91ad2dbef98cd9012ed9b637
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Fri May 20 13:37:42 2011 +0200
[trac931] Hopefully increase readability
* Split part into a function
* Use functions to construct commands instead of directly manipulating JSON
* Add some comments
commit 857abf9ee880aecc9039b993e73683f1a6019cf6
Merge: 5db2d64ddcd9f4eec0db6bbb6a160c95ac81cc6e 74567fe1e92c8f10da3a639d235cdbc4c4d5cc9d
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Fri May 20 13:18:32 2011 +0200
Merge branch 'master' into work/sign2
commit 5db2d64ddcd9f4eec0db6bbb6a160c95ac81cc6e
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Fri May 20 13:14:47 2011 +0200
[trac931] Another tests for signatures
commit 37ded0b31a0a0dcadb93da7bd85248b90d9af57f
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Fri May 20 12:22:39 2011 +0200
[trac931] Test reproducing the double read bug
commit 67d9442a23eb4fab4dff3f4d1ab142948e21e880
Author: JINMEI Tatuya <jinmei at isc.org>
Date: Thu May 19 18:15:41 2011 +0200
[trac931] Fix double reads
It was possible to schedule two reads of message length at once. That
actually read two 4-bytes chunks, one from the beginning of the real
message.
commit 5f74b5f0820d7bf4aa50d0341c9316e6a915738e
Author: JINMEI Tatuya <jinmei at isc.org>
Date: Thu May 19 17:55:07 2011 +0200
[trac931] Fix sending spec from cfgmgr
commit 030c77386b5018d30282cda2fb6aabe620a0f15b
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Thu May 19 09:11:36 2011 +0200
[trac931] Address some review comments
commit a206dbd5432240440b6bbc63ff3165ad579d5efb
Author: JINMEI Tatuya <jinmei at isc.org>
Date: Wed May 18 14:51:47 2011 -0700
[trac931] trivial suggested changes:
- typo in comment
- test NULL explicitly
- constify
commit 6342eda010ae512cb972e70f3824ca0de638c293
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Wed May 18 23:05:44 2011 +0200
[trac931] Use uint32_t for fixed-sized lengths
The length sent over wire are 32bit unsigned integers, but it could read
it as size_t which might be 64bit. It's strange this didn't cause any
trouble, but this is cleaner anyway.
commit ff008c094a577e61f619cc39df2eab858dee0fe5
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Wed May 18 22:59:55 2011 +0200
[trac931] Send the correct name of module
commit a8307030f7af9fc88e3e66b6eefcc89f6b6e15c5
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Tue May 17 20:30:50 2011 +0200
[trac931] Spelling
commit 07a778c5592002042269785ec41bf4d93fc7db91
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Tue May 17 20:10:56 2011 +0200
[trac931] Initialize keyring at startup
commit 9cc0c06cac86d3460ee1f5b5e2c8669d9709663e
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Tue May 17 20:07:58 2011 +0200
[trac931] Sign even when error
commit b8da54961c78a690c6bf02618d4c28fb9d320177
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Tue May 17 19:57:11 2011 +0200
[trac931] Most of the signing
There's a need to construct 0-length signature on error.
commit af788f1fef2cbdb63b05f14106f907dff87fd6bf
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Mon May 16 19:01:37 2011 +0200
[trac931] TSIG signing tests for auth
-----------------------------------------------------------------------
Summary of changes:
ChangeLog | 7 ++
src/bin/auth/auth_srv.cc | 100 ++++++++++++++----
src/bin/auth/auth_srv.h | 11 ++
src/bin/auth/main.cc | 16 +++-
src/bin/auth/tests/auth_srv_unittest.cc | 138 ++++++++++++++++++++++++
src/lib/config/ccsession.cc | 83 ++++++++++-----
src/lib/config/ccsession.h | 29 +++++-
src/lib/config/tests/ccsession_unittests.cc | 63 +++++++++++-
src/lib/config/tests/fake_session.cc | 22 ++++-
src/lib/config/tests/fake_session.h | 9 ++
src/lib/python/bind10_config.py.in | 3 +-
src/lib/python/isc/config/cfgmgr.py | 7 +-
src/lib/python/isc/config/tests/cfgmgr_test.py | 5 +
src/lib/server_common/keyring.cc | 7 +-
src/lib/server_common/keyring.h | 16 ++-
src/lib/server_common/tests/keyring_test.cc | 25 ++++-
src/lib/testutils/srv_test.cc | 8 +-
src/lib/testutils/srv_test.h | 3 +-
18 files changed, 480 insertions(+), 72 deletions(-)
-----------------------------------------------------------------------
diff --git a/ChangeLog b/ChangeLog
index 4e953be..10db072 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,10 @@
+245. [func] vorner
+ Authoritative server can now sign the answers using TSIG
+ (configured in tsig_keys/keys, list of strings like
+ "name:<base64-secret>:sha1-hmac"). It doesn't use them for
+ ACL yet, only verifies them and signs if the request is signed.
+ (Trac875, git fe5e7003544e4e8f18efa7b466a65f336d8c8e4d)
+
244. [func] stephen
In unit tests, allow the choice of whether unhandled exceptions are
caught in the unit test program (and details printed) or allowed to
diff --git a/src/bin/auth/auth_srv.cc b/src/bin/auth/auth_srv.cc
index a863ef3..9e01155 100644
--- a/src/bin/auth/auth_srv.cc
+++ b/src/bin/auth/auth_srv.cc
@@ -20,6 +20,7 @@
#include <cassert>
#include <iostream>
#include <vector>
+#include <memory>
#include <boost/bind.hpp>
@@ -43,6 +44,7 @@
#include <dns/rrset.h>
#include <dns/rrttl.h>
#include <dns/message.h>
+#include <dns/tsig.h>
#include <datasrc/query.h>
#include <datasrc/data_source.h>
@@ -73,6 +75,7 @@ using namespace isc::xfr;
using namespace isc::asiolink;
using namespace isc::asiodns;
using namespace isc::server_common::portconfig;
+using boost::shared_ptr;
class AuthSrvImpl {
private:
@@ -85,11 +88,14 @@ public:
isc::data::ConstElementPtr setDbFile(isc::data::ConstElementPtr config);
bool processNormalQuery(const IOMessage& io_message, MessagePtr message,
- OutputBufferPtr buffer);
+ OutputBufferPtr buffer,
+ auto_ptr<TSIGContext> tsig_context);
bool processAxfrQuery(const IOMessage& io_message, MessagePtr message,
- OutputBufferPtr buffer);
+ OutputBufferPtr buffer,
+ auto_ptr<TSIGContext> tsig_context);
bool processNotify(const IOMessage& io_message, MessagePtr message,
- OutputBufferPtr buffer);
+ OutputBufferPtr buffer,
+ auto_ptr<TSIGContext> tsig_context);
IOService io_service_;
@@ -116,6 +122,9 @@ public:
/// Addresses we listen on
AddressList listen_addresses_;
+
+ /// The TSIG keyring
+ const shared_ptr<TSIGKeyRing>* keyring_;
private:
std::string db_file_;
@@ -139,6 +148,7 @@ AuthSrvImpl::AuthSrvImpl(const bool use_cache,
memory_datasrc_class_(RRClass::IN()),
statistics_timer_(io_service_),
counters_(verbose_mode_),
+ keyring_(NULL),
xfrout_connected_(false),
xfrout_client_(xfrout_client)
{
@@ -241,7 +251,9 @@ public:
void
makeErrorMessage(MessagePtr message, OutputBufferPtr buffer,
- const Rcode& rcode, const bool verbose_mode)
+ const Rcode& rcode, const bool verbose_mode,
+ std::auto_ptr<TSIGContext> tsig_context =
+ std::auto_ptr<TSIGContext>())
{
// extract the parameters that should be kept.
// XXX: with the current implementation, it's not easy to set EDNS0
@@ -272,7 +284,11 @@ makeErrorMessage(MessagePtr message, OutputBufferPtr buffer,
message->setRcode(rcode);
MessageRenderer renderer(*buffer);
- message->toWire(renderer);
+ if (tsig_context.get() != NULL) {
+ message->toWire(renderer, *tsig_context);
+ } else {
+ message->toWire(renderer);
+ }
if (verbose_mode) {
cerr << "[b10-auth] sending an error response (" <<
@@ -446,29 +462,51 @@ AuthSrv::processMessage(const IOMessage& io_message, MessagePtr message,
}
// Perform further protocol-level validation.
+ // TSIG first
+ // If this is set to something, we know we need to answer with TSIG as well
+ std::auto_ptr<TSIGContext> tsig_context;
+ const TSIGRecord* tsig_record(message->getTSIGRecord());
+ TSIGError tsig_error(TSIGError::NOERROR());
+
+ // Do we do TSIG?
+ // The keyring can be null if we're in test
+ if (impl_->keyring_ != NULL && tsig_record != NULL) {
+ tsig_context.reset(new TSIGContext(tsig_record->getName(),
+ tsig_record->getRdata().
+ getAlgorithm(),
+ **impl_->keyring_));
+ tsig_error = tsig_context->verify(tsig_record, io_message.getData(),
+ io_message.getDataSize());
+ }
bool sendAnswer = true;
- if (message->getOpcode() == Opcode::NOTIFY()) {
- sendAnswer = impl_->processNotify(io_message, message, buffer);
+ if (tsig_error != TSIGError::NOERROR()) {
+ makeErrorMessage(message, buffer, tsig_error.toRcode(),
+ impl_->verbose_mode_, tsig_context);
+ } else if (message->getOpcode() == Opcode::NOTIFY()) {
+ sendAnswer = impl_->processNotify(io_message, message, buffer,
+ tsig_context);
} else if (message->getOpcode() != Opcode::QUERY()) {
if (impl_->verbose_mode_) {
cerr << "[b10-auth] unsupported opcode" << endl;
}
makeErrorMessage(message, buffer, Rcode::NOTIMP(),
- impl_->verbose_mode_);
+ impl_->verbose_mode_, tsig_context);
} else if (message->getRRCount(Message::SECTION_QUESTION) != 1) {
makeErrorMessage(message, buffer, Rcode::FORMERR(),
- impl_->verbose_mode_);
+ impl_->verbose_mode_, tsig_context);
} else {
ConstQuestionPtr question = *message->beginQuestion();
const RRType &qtype = question->getType();
if (qtype == RRType::AXFR()) {
- sendAnswer = impl_->processAxfrQuery(io_message, message, buffer);
+ sendAnswer = impl_->processAxfrQuery(io_message, message, buffer,
+ tsig_context);
} else if (qtype == RRType::IXFR()) {
makeErrorMessage(message, buffer, Rcode::NOTIMP(),
- impl_->verbose_mode_);
+ impl_->verbose_mode_, tsig_context);
} else {
- sendAnswer = impl_->processNormalQuery(io_message, message, buffer);
+ sendAnswer = impl_->processNormalQuery(io_message, message, buffer,
+ tsig_context);
}
}
@@ -477,7 +515,8 @@ AuthSrv::processMessage(const IOMessage& io_message, MessagePtr message,
bool
AuthSrvImpl::processNormalQuery(const IOMessage& io_message, MessagePtr message,
- OutputBufferPtr buffer)
+ OutputBufferPtr buffer,
+ auto_ptr<TSIGContext> tsig_context)
{
ConstEDNSPtr remote_edns = message->getEDNS();
const bool dnssec_ok = remote_edns && remote_edns->getDNSSECAwareness();
@@ -523,7 +562,11 @@ AuthSrvImpl::processNormalQuery(const IOMessage& io_message, MessagePtr message,
const bool udp_buffer =
(io_message.getSocket().getProtocol() == IPPROTO_UDP);
renderer.setLengthLimit(udp_buffer ? remote_bufsize : 65535);
- message->toWire(renderer);
+ if (tsig_context.get() != NULL) {
+ message->toWire(renderer, *tsig_context);
+ } else {
+ message->toWire(renderer);
+ }
if (verbose_mode_) {
cerr << "[b10-auth] sending a response ("
@@ -536,7 +579,8 @@ AuthSrvImpl::processNormalQuery(const IOMessage& io_message, MessagePtr message,
bool
AuthSrvImpl::processAxfrQuery(const IOMessage& io_message, MessagePtr message,
- OutputBufferPtr buffer)
+ OutputBufferPtr buffer,
+ auto_ptr<TSIGContext> tsig_context)
{
// Increment query counter.
incCounter(io_message.getSocket().getProtocol());
@@ -545,7 +589,8 @@ AuthSrvImpl::processAxfrQuery(const IOMessage& io_message, MessagePtr message,
if (verbose_mode_) {
cerr << "[b10-auth] AXFR query over UDP isn't allowed" << endl;
}
- makeErrorMessage(message, buffer, Rcode::FORMERR(), verbose_mode_);
+ makeErrorMessage(message, buffer, Rcode::FORMERR(), verbose_mode_,
+ tsig_context);
return (true);
}
@@ -572,7 +617,8 @@ AuthSrvImpl::processAxfrQuery(const IOMessage& io_message, MessagePtr message,
cerr << "[b10-auth] Error in handling XFR request: " << err.what()
<< endl;
}
- makeErrorMessage(message, buffer, Rcode::SERVFAIL(), verbose_mode_);
+ makeErrorMessage(message, buffer, Rcode::SERVFAIL(), verbose_mode_,
+ tsig_context);
return (true);
}
@@ -581,7 +627,8 @@ AuthSrvImpl::processAxfrQuery(const IOMessage& io_message, MessagePtr message,
bool
AuthSrvImpl::processNotify(const IOMessage& io_message, MessagePtr message,
- OutputBufferPtr buffer)
+ OutputBufferPtr buffer,
+ std::auto_ptr<TSIGContext> tsig_context)
{
// The incoming notify must contain exactly one question for SOA of the
// zone name.
@@ -590,7 +637,8 @@ AuthSrvImpl::processNotify(const IOMessage& io_message, MessagePtr message,
cerr << "[b10-auth] invalid number of questions in notify: "
<< message->getRRCount(Message::SECTION_QUESTION) << endl;
}
- makeErrorMessage(message, buffer, Rcode::FORMERR(), verbose_mode_);
+ makeErrorMessage(message, buffer, Rcode::FORMERR(), verbose_mode_,
+ tsig_context);
return (true);
}
ConstQuestionPtr question = *message->beginQuestion();
@@ -599,7 +647,8 @@ AuthSrvImpl::processNotify(const IOMessage& io_message, MessagePtr message,
cerr << "[b10-auth] invalid question RR type in notify: "
<< question->getType() << endl;
}
- makeErrorMessage(message, buffer, Rcode::FORMERR(), verbose_mode_);
+ makeErrorMessage(message, buffer, Rcode::FORMERR(), verbose_mode_,
+ tsig_context);
return (true);
}
@@ -662,7 +711,11 @@ AuthSrvImpl::processNotify(const IOMessage& io_message, MessagePtr message,
message->setRcode(Rcode::NOERROR());
MessageRenderer renderer(*buffer);
- message->toWire(renderer);
+ if (tsig_context.get() != NULL) {
+ message->toWire(renderer, *tsig_context);
+ } else {
+ message->toWire(renderer);
+ }
return (true);
}
@@ -772,3 +825,8 @@ void
AuthSrv::setDNSService(isc::asiodns::DNSService& dnss) {
dnss_ = &dnss;
}
+
+void
+AuthSrv::setTSIGKeyRing(const shared_ptr<TSIGKeyRing>* keyring) {
+ impl_->keyring_ = keyring;
+}
diff --git a/src/bin/auth/auth_srv.h b/src/bin/auth/auth_srv.h
index 88f00c1..19c97b5 100644
--- a/src/bin/auth/auth_srv.h
+++ b/src/bin/auth/auth_srv.h
@@ -44,6 +44,9 @@ class MemoryDataSrc;
namespace xfr {
class AbstractXfroutClient;
}
+namespace dns {
+class TSIGKeyRing;
+}
}
@@ -374,6 +377,14 @@ public:
/// \brief Assign an ASIO DNS Service queue to this Auth object
void setDNSService(isc::asiodns::DNSService& dnss);
+ /// \brief Sets the keyring used for verifying and signing
+ ///
+ /// The parameter is pointer to shared pointer, because the automatic
+ /// reloading routines of tsig keys replace the actual keyring object.
+ /// It is expected the pointer will point to some statically-allocated
+ /// object, it doesn't take ownership of it.
+ void setTSIGKeyRing(const boost::shared_ptr<isc::dns::TSIGKeyRing>*
+ keyring);
private:
AuthSrvImpl* impl_;
diff --git a/src/bin/auth/main.cc b/src/bin/auth/main.cc
index 480c2f7..0324c6e 100644
--- a/src/bin/auth/main.cc
+++ b/src/bin/auth/main.cc
@@ -47,6 +47,7 @@
#include <asiodns/asiodns.h>
#include <asiolink/asiolink.h>
#include <log/dummylog.h>
+#include <server_common/keyring.h>
using namespace std;
using namespace isc::data;
@@ -152,9 +153,14 @@ main(int argc, char* argv[]) {
cc_session = new Session(io_service.get_io_service());
cout << "[b10-auth] Configuration session channel created." << endl;
+ // We delay starting listening to new commands/config just before we
+ // go into the main loop to avoid confusion due to mixture of
+ // synchronous and asynchronous operations (this would happen in
+ // initializing TSIG keys below). Until then all operations on the
+ // CC session will take place synchronously.
config_session = new ModuleCCSession(specfile, *cc_session,
my_config_handler,
- my_command_handler);
+ my_command_handler, false);
cout << "[b10-auth] Configuration channel established." << endl;
xfrin_session = new Session(io_service.get_io_service());
@@ -190,6 +196,14 @@ main(int argc, char* argv[]) {
changeUser(uid);
}
+ cout << "[b10-auth] Loading TSIG keys" << endl;
+ isc::server_common::initKeyring(*config_session);
+ auth_server->setTSIGKeyRing(&isc::server_common::keyring);
+
+ // Now start asynchronous read.
+ config_session->start();
+ cout << "[b10-auth] Configuration channel started." << endl;
+
cout << "[b10-auth] Server started." << endl;
io_service.run();
diff --git a/src/bin/auth/tests/auth_srv_unittest.cc b/src/bin/auth/tests/auth_srv_unittest.cc
index a77f7e6..d922901 100644
--- a/src/bin/auth/tests/auth_srv_unittest.cc
+++ b/src/bin/auth/tests/auth_srv_unittest.cc
@@ -16,6 +16,8 @@
#include <vector>
+#include <boost/shared_ptr.hpp>
+
#include <gtest/gtest.h>
#include <dns/message.h>
@@ -25,8 +27,10 @@
#include <dns/rrtype.h>
#include <dns/rrttl.h>
#include <dns/rdataclass.h>
+#include <dns/tsig.h>
#include <server_common/portconfig.h>
+#include <server_common/keyring.h>
#include <datasrc/memory_datasrc.h>
#include <auth/auth_srv.h>
@@ -50,6 +54,7 @@ using namespace isc::asiolink;
using namespace isc::testutils;
using namespace isc::server_common::portconfig;
using isc::UnitTestUtil;
+using boost::shared_ptr;
namespace {
const char* const CONFIG_TESTDB =
@@ -242,6 +247,139 @@ TEST_F(AuthSrvTest, AXFRSuccess) {
EXPECT_TRUE(xfrout.isConnected());
}
+// Try giving the server a TSIG signed request and see it can anwer signed as
+// well
+TEST_F(AuthSrvTest, TSIGSigned) {
+ // Prepare key, the client message, etc
+ const TSIGKey key("key:c2VjcmV0Cg==:hmac-sha1");
+ TSIGContext context(key);
+ UnitTestUtil::createRequestMessage(request_message, opcode, default_qid,
+ Name("version.bind"), RRClass::CH(), RRType::TXT());
+ createRequestPacket(request_message, IPPROTO_UDP, &context);
+
+ // Run the message through the server
+ shared_ptr<TSIGKeyRing> keyring(new TSIGKeyRing);
+ keyring->add(key);
+ server.setTSIGKeyRing(&keyring);
+ server.processMessage(*io_message, parse_message, response_obuffer,
+ &dnsserv);
+
+ // What did we get?
+ EXPECT_TRUE(dnsserv.hasAnswer());
+ headerCheck(*parse_message, default_qid, Rcode::NOERROR(),
+ opcode.getCode(), QR_FLAG | AA_FLAG, 1, 1, 1, 0);
+ // We need to parse the message ourself, or getTSIGRecord won't work
+ InputBuffer ib(response_obuffer->getData(), response_obuffer->getLength());
+ Message m(Message::PARSE);
+ m.fromWire(ib);
+
+ const TSIGRecord* tsig = m.getTSIGRecord();
+ ASSERT_TRUE(tsig != NULL) << "Missing TSIG signature";
+ TSIGError error(context.verify(tsig, response_obuffer->getData(),
+ response_obuffer->getLength()));
+ EXPECT_EQ(TSIGError::NOERROR(), error) <<
+ "The server signed the response, but it doesn't seem to be valid";
+}
+
+// Give the server a signed request, but don't give it the key. It will
+// not be able to verify it, returning BADKEY
+TEST_F(AuthSrvTest, TSIGSignedBadKey) {
+ TSIGKey key("key:c2VjcmV0Cg==:hmac-sha1");
+ TSIGContext context(key);
+ UnitTestUtil::createRequestMessage(request_message, opcode, default_qid,
+ Name("version.bind"), RRClass::CH(), RRType::TXT());
+ createRequestPacket(request_message, IPPROTO_UDP, &context);
+
+ // Process the message, but use a different key there
+ shared_ptr<TSIGKeyRing> keyring(new TSIGKeyRing);
+ server.setTSIGKeyRing(&keyring);
+ server.processMessage(*io_message, parse_message, response_obuffer,
+ &dnsserv);
+
+ EXPECT_TRUE(dnsserv.hasAnswer());
+ headerCheck(*parse_message, default_qid, TSIGError::BAD_KEY().toRcode(),
+ opcode.getCode(), QR_FLAG, 1, 0, 0, 0);
+ // We need to parse the message ourself, or getTSIGRecord won't work
+ InputBuffer ib(response_obuffer->getData(), response_obuffer->getLength());
+ Message m(Message::PARSE);
+ m.fromWire(ib);
+
+ const TSIGRecord* tsig = m.getTSIGRecord();
+ ASSERT_TRUE(tsig != NULL) <<
+ "Missing TSIG signature (we should have one even at error)";
+ EXPECT_EQ(TSIGError::BAD_KEY_CODE, tsig->getRdata().getError());
+ EXPECT_EQ(0, tsig->getRdata().getMACSize()) <<
+ "It should be unsigned with this error";
+}
+
+// Give the server a signed request, but signed by a different key
+// (with the same name). It should return BADSIG
+TEST_F(AuthSrvTest, TSIGBadSig) {
+ TSIGKey key("key:c2VjcmV0Cg==:hmac-sha1");
+ TSIGContext context(key);
+ UnitTestUtil::createRequestMessage(request_message, opcode, default_qid,
+ Name("version.bind"), RRClass::CH(), RRType::TXT());
+ createRequestPacket(request_message, IPPROTO_UDP, &context);
+
+ // Process the message, but use a different key there
+ shared_ptr<TSIGKeyRing> keyring(new TSIGKeyRing);
+ keyring->add(TSIGKey("key:QkFECg==:hmac-sha1"));
+ server.setTSIGKeyRing(&keyring);
+ server.processMessage(*io_message, parse_message, response_obuffer,
+ &dnsserv);
+
+ EXPECT_TRUE(dnsserv.hasAnswer());
+ headerCheck(*parse_message, default_qid, TSIGError::BAD_SIG().toRcode(),
+ opcode.getCode(), QR_FLAG, 1, 0, 0, 0);
+ // We need to parse the message ourself, or getTSIGRecord won't work
+ InputBuffer ib(response_obuffer->getData(), response_obuffer->getLength());
+ Message m(Message::PARSE);
+ m.fromWire(ib);
+
+ const TSIGRecord* tsig = m.getTSIGRecord();
+ ASSERT_TRUE(tsig != NULL) <<
+ "Missing TSIG signature (we should have one even at error)";
+ EXPECT_EQ(TSIGError::BAD_SIG_CODE, tsig->getRdata().getError());
+ EXPECT_EQ(0, tsig->getRdata().getMACSize()) <<
+ "It should be unsigned with this error";
+}
+
+// Give the server a signed unsupported request with a bad signature.
+// This checks the server first verifies the signature before anything
+// else.
+TEST_F(AuthSrvTest, TSIGCheckFirst) {
+ TSIGKey key("key:c2VjcmV0Cg==:hmac-sha1");
+ TSIGContext context(key);
+ // Pass a wrong opcode there. The server shouldn't know what to do
+ // about it.
+ UnitTestUtil::createRequestMessage(request_message, Opcode::RESERVED14(),
+ default_qid, Name("version.bind"),
+ RRClass::CH(), RRType::TXT());
+ createRequestPacket(request_message, IPPROTO_UDP, &context);
+
+ // Process the message, but use a different key there
+ shared_ptr<TSIGKeyRing> keyring(new TSIGKeyRing);
+ keyring->add(TSIGKey("key:QkFECg==:hmac-sha1"));
+ server.setTSIGKeyRing(&keyring);
+ server.processMessage(*io_message, parse_message, response_obuffer,
+ &dnsserv);
+
+ EXPECT_TRUE(dnsserv.hasAnswer());
+ headerCheck(*parse_message, default_qid, TSIGError::BAD_SIG().toRcode(),
+ Opcode::RESERVED14().getCode(), QR_FLAG, 0, 0, 0, 0);
+ // We need to parse the message ourself, or getTSIGRecord won't work
+ InputBuffer ib(response_obuffer->getData(), response_obuffer->getLength());
+ Message m(Message::PARSE);
+ m.fromWire(ib);
+
+ const TSIGRecord* tsig = m.getTSIGRecord();
+ ASSERT_TRUE(tsig != NULL) <<
+ "Missing TSIG signature (we should have one even at error)";
+ EXPECT_EQ(TSIGError::BAD_SIG_CODE, tsig->getRdata().getError());
+ EXPECT_EQ(0, tsig->getRdata().getMACSize()) <<
+ "It should be unsigned with this error";
+}
+
TEST_F(AuthSrvTest, AXFRConnectFail) {
EXPECT_FALSE(xfrout.isConnected()); // check prerequisite
xfrout.disableConnect();
diff --git a/src/lib/config/ccsession.cc b/src/lib/config/ccsession.cc
index 45710e3..e443c00 100644
--- a/src/lib/config/ccsession.cc
+++ b/src/lib/config/ccsession.cc
@@ -192,8 +192,10 @@ ModuleCCSession::ModuleCCSession(
isc::data::ConstElementPtr(*config_handler)(
isc::data::ConstElementPtr new_config),
isc::data::ConstElementPtr(*command_handler)(
- const std::string& command, isc::data::ConstElementPtr args)
+ const std::string& command, isc::data::ConstElementPtr args),
+ bool start_immediately
) :
+ started_(false),
session_(session)
{
module_specification_ = readModuleSpecification(spec_file_name);
@@ -237,8 +239,21 @@ ModuleCCSession::ModuleCCSession(
}
}
+ if (start_immediately) {
+ start();
+ }
+}
+
+void
+ModuleCCSession::start() {
+ if (started_) {
+ isc_throw(CCSessionError, "Module CC session already started");
+ }
+
// register callback for asynchronous read
session_.startRead(boost::bind(&ModuleCCSession::startCheck, this));
+
+ started_ = true;
}
/// Validates the new config values, if they are correct,
@@ -355,45 +370,56 @@ ModuleCCSession::checkCommand() {
return (0);
}
-std::string
-ModuleCCSession::addRemoteConfig(const std::string& spec_name,
- void (*handler)(const std::string& module,
- ConstElementPtr),
- bool spec_is_filename)
-{
- std::string module_name;
- ModuleSpec rmod_spec;
- if (spec_is_filename) {
- // It's a file name, so load it
- rmod_spec = readModuleSpecification(spec_name);
- module_name =
- rmod_spec.getFullSpec()->get("module_name")->stringValue();
+ModuleSpec
+ModuleCCSession::fetchRemoteSpec(const std::string& module, bool is_filename) {
+ if (is_filename) {
+ // It is a filename, simply load it.
+ return (readModuleSpecification(module));
} else {
// It's module name, request it from config manager
- ConstElementPtr cmd = Element::fromJSON("{ \"command\": ["
- "\"get_module_spec\","
- "{\"module_name\": \"" +
- module_name + "\"} ] }");
- unsigned int seq = session_.group_sendmsg(cmd, "ConfigManager");
+
+ // Send the command
+ ConstElementPtr cmd(createCommand("get_module_spec",
+ Element::fromJSON("{\"module_name\": \"" + module +
+ "\"}")));
+ const unsigned int seq = session_.group_sendmsg(cmd, "ConfigManager");
+
+ // Wait for the answer
ConstElementPtr env, answer;
session_.group_recvmsg(env, answer, false, seq);
int rcode;
ConstElementPtr spec_data = parseAnswer(rcode, answer);
if (rcode == 0 && spec_data) {
- rmod_spec = ModuleSpec(spec_data);
- module_name = spec_name;
- if (module_name != rmod_spec.getModuleName()) {
+ // received OK, construct the spec out of it
+ ModuleSpec spec = ModuleSpec(spec_data);
+ if (module != spec.getModuleName()) {
+ // It's a different module!
isc_throw(CCSessionError, "Module name mismatch");
}
+ return (spec);
} else {
- isc_throw(CCSessionError, "Error getting config for " + module_name + ": " + answer->str());
+ isc_throw(CCSessionError, "Error getting config for " +
+ module + ": " + answer->str());
}
}
- ConfigData rmod_config = ConfigData(rmod_spec);
+}
+
+std::string
+ModuleCCSession::addRemoteConfig(const std::string& spec_name,
+ void (*handler)(const std::string& module,
+ ConstElementPtr),
+ bool spec_is_filename)
+{
+ // First get the module name, specification and default config
+ const ModuleSpec rmod_spec(fetchRemoteSpec(spec_name, spec_is_filename));
+ const std::string module_name(rmod_spec.getModuleName());
+ ConfigData rmod_config(rmod_spec);
- // Get the current configuration values for that module
- ConstElementPtr cmd = Element::fromJSON("{ \"command\": [\"get_config\", {\"module_name\":\"" + module_name + "\"} ] }");
- unsigned int seq = session_.group_sendmsg(cmd, "ConfigManager");
+ // Get the current configuration values from config manager
+ ConstElementPtr cmd(createCommand("get_config",
+ Element::fromJSON("{\"module_name\": \"" +
+ module_name + "\"}")));
+ const unsigned int seq = session_.group_sendmsg(cmd, "ConfigManager");
ConstElementPtr env, answer;
session_.group_recvmsg(env, answer, false, seq);
@@ -401,6 +427,7 @@ ModuleCCSession::addRemoteConfig(const std::string& spec_name,
ConstElementPtr new_config = parseAnswer(rcode, answer);
ElementPtr local_config;
if (rcode == 0 && new_config) {
+ // Merge the received config into existing local config
local_config = rmod_config.getLocalConfig();
isc::data::merge(local_config, new_config);
rmod_config.setLocalConfig(local_config);
@@ -414,6 +441,8 @@ ModuleCCSession::addRemoteConfig(const std::string& spec_name,
remote_module_handlers_[module_name] = handler;
handler(module_name, local_config);
}
+
+ // Make sure we get updates in future
session_.subscribe(module_name);
return (module_name);
}
diff --git a/src/lib/config/ccsession.h b/src/lib/config/ccsession.h
index c845b8f..e677146 100644
--- a/src/lib/config/ccsession.h
+++ b/src/lib/config/ccsession.h
@@ -171,6 +171,11 @@ public:
* @param command_handler A callback function pointer to be called when
* a control command from a remote agent needs to be performed on the
* local module.
+ * @start_immediately If true (default), start listening to new commands
+ * and configuration changes asynchronously at the end of the constructor;
+ * if false, it will be delayed until the start() method is explicitly
+ * called. (This is a short term workaround for an initialization trouble.
+ * We'll need to develop a cleaner solution, and then remove this knob)
*/
ModuleCCSession(const std::string& spec_file_name,
isc::cc::AbstractSession& session,
@@ -178,9 +183,20 @@ public:
isc::data::ConstElementPtr new_config) = NULL,
isc::data::ConstElementPtr(*command_handler)(
const std::string& command,
- isc::data::ConstElementPtr args) = NULL
+ isc::data::ConstElementPtr args) = NULL,
+ bool start_immediately = true
);
+ /// Start receiving new commands and configuration changes asynchronously.
+ ///
+ /// This method must be called only once, and only when the ModuleCCSession
+ /// was constructed with start_immediately being false. Otherwise
+ /// CCSessionError will be thrown.
+ ///
+ /// As noted in the constructor, this method should be considered a short
+ /// term workaround and will be removed in future.
+ void start();
+
/**
* Optional optimization for checkCommand loop; returns true
* if there are unhandled queued messages in the cc session.
@@ -240,12 +256,16 @@ public:
* for those changes. This function will subscribe to the relevant module
* channel.
*
+ * This method must be called before calling the \c start() method on the
+ * ModuleCCSession (it also implies the ModuleCCSession must have been
+ * constructed with start_immediately being false).
+ *
* \param spec_name This specifies the module to add. It is either a
* filename of the spec file to use or a name of module
* (in case it's a module name, the spec data is
* downloaded from the configuration manager, therefore
* the configuration manager must know it). If
- * spec_is_filenabe is true (the default), then a
+ * spec_is_filename is true (the default), then a
* filename is assumed, otherwise a module name.
* \param handler The handler function called whenever there's a change.
* Called once initally from this function. May be NULL
@@ -293,7 +313,8 @@ public:
private:
ModuleSpec readModuleSpecification(const std::string& filename);
void startCheck();
-
+
+ bool started_;
std::string module_name_;
isc::cc::AbstractSession& session_;
ModuleSpec module_specification_;
@@ -322,6 +343,8 @@ private:
void updateRemoteConfig(const std::string& module_name,
isc::data::ConstElementPtr new_config);
+
+ ModuleSpec fetchRemoteSpec(const std::string& module, bool is_filename);
};
}
diff --git a/src/lib/config/tests/ccsession_unittests.cc b/src/lib/config/tests/ccsession_unittests.cc
index 3564d4b..46132b3 100644
--- a/src/lib/config/tests/ccsession_unittests.cc
+++ b/src/lib/config/tests/ccsession_unittests.cc
@@ -362,7 +362,7 @@ TEST_F(CCSessionTest, remoteConfig) {
std::string module_name;
int item1;
- ModuleCCSession mccs(ccspecfile("spec1.spec"), session, NULL, NULL);
+ ModuleCCSession mccs(ccspecfile("spec1.spec"), session, NULL, NULL, false);
EXPECT_TRUE(session.haveSubscription("Spec1", "*"));
// first simply connect, with no config values, and see we get
@@ -415,7 +415,16 @@ TEST_F(CCSessionTest, remoteConfig) {
EXPECT_NO_THROW(module_name = mccs.addRemoteConfig("Spec2", NULL,
false));
+ const size_t qsize(session.getMsgQueue()->size());
+ EXPECT_TRUE(session.getMsgQueue()->get(qsize - 2)->equals(*el(
+ "[ \"ConfigManager\", \"*\", { \"command\": ["
+ "\"get_module_spec\", { \"module_name\": \"Spec2\" } ] } ]")));
+ EXPECT_TRUE(session.getMsgQueue()->get(qsize - 1)->equals(*el(
+ "[ \"ConfigManager\", \"*\", { \"command\": [ \"get_config\","
+ "{ \"module_name\": \"Spec2\" } ] } ]")));
EXPECT_EQ("Spec2", module_name);
+ // Since we returned an empty local config above, the default value
+ // for "item1", which is 1, should be used.
EXPECT_NO_THROW(item1 =
mccs.getRemoteConfigValue(module_name,
"item1")->intValue());
@@ -425,6 +434,18 @@ TEST_F(CCSessionTest, remoteConfig) {
}
{
+ SCOPED_TRACE("With bad module name");
+ // It is almost the same as above, but we supply wrong module name.
+ // It should fail.
+ // Try adding it with downloading the spec from config manager
+ ModuleSpec spec(moduleSpecFromFile(ccspecfile("spec2.spec")));
+ session.getMessages()->add(createAnswer(0, spec.getFullSpec()));
+
+ EXPECT_THROW(module_name = mccs.addRemoteConfig("Spec1", NULL, false),
+ CCSessionError);
+ }
+
+ {
// Try adding it with a handler.
// Pass non-default value to see the handler is called after
// downloading the configuration, not too soon.
@@ -496,7 +517,8 @@ TEST_F(CCSessionTest, ignoreRemoteConfigCommands) {
session.getMessages()->add(createAnswer(0, el("{ }")));
EXPECT_FALSE(session.haveSubscription("Spec29", "*"));
- ModuleCCSession mccs(ccspecfile("spec29.spec"), session, my_config_handler, my_command_handler);
+ ModuleCCSession mccs(ccspecfile("spec29.spec"), session, my_config_handler,
+ my_command_handler, false);
EXPECT_TRUE(session.haveSubscription("Spec29", "*"));
EXPECT_EQ(2, session.getMsgQueue()->size());
@@ -546,4 +568,41 @@ TEST_F(CCSessionTest, initializationFail) {
EXPECT_TRUE(session.haveSubscription("Spec29", "*"));
}
+// Test it throws when we try to start it twice (once from the constructor)
+TEST_F(CCSessionTest, doubleStartImplicit) {
+ ModuleCCSession mccs(ccspecfile("spec29.spec"), session, NULL, NULL);
+ EXPECT_THROW(mccs.start(), CCSessionError);
+}
+
+// The same, but both starts are explicit
+TEST_F(CCSessionTest, doubleStartExplicit) {
+ ModuleCCSession mccs(ccspecfile("spec29.spec"), session, NULL, NULL,
+ false);
+ mccs.start();
+ EXPECT_THROW(mccs.start(), CCSessionError);
+}
+
+// Test we can request synchronous receive before we start the session,
+// and check there's the mechanism if we do it after
+TEST_F(CCSessionTest, delayedStart) {
+ ModuleCCSession mccs(ccspecfile("spec2.spec"), session, NULL, NULL, false);
+ session.getMessages()->add(createAnswer());
+ ConstElementPtr env, answer;
+ EXPECT_NO_THROW(session.group_recvmsg(env, answer, false, 3));
+ mccs.start();
+ session.getMessages()->add(createAnswer());
+ EXPECT_THROW(session.group_recvmsg(env, answer, false, 3),
+ FakeSession::DoubleRead);
+}
+
+// Similar to the above, but more implicitly by calling addRemoteConfig().
+// We should construct ModuleCCSession with start_immediately being false
+// if we need to call addRemoteConfig().
+// The correct cases are covered in remoteConfig test.
+TEST_F(CCSessionTest, doubleStartWithAddRemoteConfig) {
+ ModuleCCSession mccs(ccspecfile("spec29.spec"), session, NULL, NULL);
+ session.getMessages()->add(createAnswer(0, el("{}")));
+ EXPECT_THROW(mccs.addRemoteConfig(ccspecfile("spec2.spec")),
+ FakeSession::DoubleRead);
+}
}
diff --git a/src/lib/config/tests/fake_session.cc b/src/lib/config/tests/fake_session.cc
index 5f79d48..2b216e7 100644
--- a/src/lib/config/tests/fake_session.cc
+++ b/src/lib/config/tests/fake_session.cc
@@ -71,7 +71,8 @@ FakeSession::FakeSession(isc::data::ElementPtr initial_messages,
isc::data::ElementPtr msg_queue) :
messages_(initial_messages),
subscriptions_(subscriptions),
- msg_queue_(msg_queue)
+ msg_queue_(msg_queue),
+ started_(false)
{
}
@@ -84,6 +85,7 @@ FakeSession::disconnect() {
void
FakeSession::startRead(boost::function<void()>) {
+ started_ = true;
}
void
@@ -91,7 +93,13 @@ FakeSession::establish(const char*) {
}
bool
-FakeSession::recvmsg(ConstElementPtr& msg, bool, int) {
+FakeSession::recvmsg(ConstElementPtr& msg, bool nonblock, int) {
+ if (started_ && !nonblock) {
+ // This would schedule another read for length, leading to
+ // corputed data
+ isc_throw(DoubleRead, "Second read scheduled from recvmsg");
+ }
+
//cout << "[XX] client asks for message " << endl;
if (messages_ &&
messages_->getType() == Element::list &&
@@ -105,7 +113,15 @@ FakeSession::recvmsg(ConstElementPtr& msg, bool, int) {
}
bool
-FakeSession::recvmsg(ConstElementPtr& env, ConstElementPtr& msg, bool, int) {
+FakeSession::recvmsg(ConstElementPtr& env, ConstElementPtr& msg, bool nonblock,
+ int)
+{
+ if (started_ && !nonblock) {
+ // This would schedule another read for length, leading to
+ // corputed data
+ isc_throw(DoubleRead, "Second read scheduled from recvmsg");
+ }
+
//cout << "[XX] client asks for message and env" << endl;
env = ElementPtr();
if (messages_ &&
diff --git a/src/lib/config/tests/fake_session.h b/src/lib/config/tests/fake_session.h
index ac8e291..85e47d5 100644
--- a/src/lib/config/tests/fake_session.h
+++ b/src/lib/config/tests/fake_session.h
@@ -42,6 +42,14 @@ public:
isc::data::ElementPtr msg_queue);
virtual ~FakeSession();
+ // This is thrown if two reads for length at once are scheduled at once.
+ // Such thing does bad things currently (see discussion in ticket #931).
+ class DoubleRead : public Exception {
+ public:
+ DoubleRead(const char* file, size_t line, const char* what) :
+ Exception(file, line, what) {}
+ };
+
virtual void startRead(boost::function<void()> read_callback);
virtual void establish(const char* socket_file = NULL);
@@ -89,6 +97,7 @@ private:
const isc::data::ElementPtr messages_;
isc::data::ElementPtr subscriptions_;
isc::data::ElementPtr msg_queue_;
+ bool started_;
};
} // namespace cc
} // namespace isc
diff --git a/src/lib/python/bind10_config.py.in b/src/lib/python/bind10_config.py.in
index 4a38903..69b17ed 100644
--- a/src/lib/python/bind10_config.py.in
+++ b/src/lib/python/bind10_config.py.in
@@ -42,7 +42,8 @@ def reload():
DATA_PATH = os.environ["B10_FROM_SOURCE_LOCALSTATEDIR"]
else:
DATA_PATH = os.environ["B10_FROM_SOURCE"]
- PLUGIN_PATHS = [DATA_PATH + '/src/bin/cfgmgr/plugins']
+ PLUGIN_PATHS = [os.environ["B10_FROM_SOURCE"] +
+ '/src/bin/cfgmgr/plugins']
else:
DATA_PATH = "@localstatedir@/@PACKAGE@".replace("${prefix}", PREFIX)
PLUGIN_PATHS = ["@prefix@/share/@PACKAGE@/config_plugins"]
diff --git a/src/lib/python/isc/config/cfgmgr.py b/src/lib/python/isc/config/cfgmgr.py
index 88a93e1..5c2af08 100644
--- a/src/lib/python/isc/config/cfgmgr.py
+++ b/src/lib/python/isc/config/cfgmgr.py
@@ -272,7 +272,12 @@ class ConfigManager:
if type(cmd) == dict:
if 'module_name' in cmd and cmd['module_name'] != '':
module_name = cmd['module_name']
- answer = ccsession.create_answer(0, self.get_module_spec(module_name))
+ spec = self.get_module_spec(cmd['module_name'])
+ if type(spec) != type({}):
+ # this is a ModuleSpec object. Extract the
+ # internal spec.
+ spec = spec.get_full_spec()
+ answer = ccsession.create_answer(0, spec)
else:
answer = ccsession.create_answer(1, "Bad module_name in get_module_spec command")
else:
diff --git a/src/lib/python/isc/config/tests/cfgmgr_test.py b/src/lib/python/isc/config/tests/cfgmgr_test.py
index b06db31..647f2d3 100644
--- a/src/lib/python/isc/config/tests/cfgmgr_test.py
+++ b/src/lib/python/isc/config/tests/cfgmgr_test.py
@@ -178,6 +178,8 @@ class TestConfigManager(unittest.TestCase):
module_spec2 = self.cm.get_module_spec(module_spec.get_module_name())
self.assertEqual(module_spec, module_spec2)
+ self.assertEqual({}, self.cm.get_module_spec("nosuchmodule"))
+
def test_get_config_spec(self):
config_spec = self.cm.get_config_spec()
self.assertEqual(config_spec, {})
@@ -323,6 +325,9 @@ class TestConfigManager(unittest.TestCase):
self._handle_msg_helper({ "command": [ "module_spec", { 'foo': 1 } ] },
{'result': [1, 'Error in data definition: no module_name in module_spec']})
self._handle_msg_helper({ "command": [ "get_module_spec" ] }, { 'result': [ 0, { self.spec.get_module_name(): self.spec.get_full_spec() } ]})
+ self._handle_msg_helper({ "command": [ "get_module_spec",
+ { "module_name" : "Spec2" } ] },
+ { 'result': [ 0, self.spec.get_full_spec() ] })
self._handle_msg_helper({ "command": [ "get_commands_spec" ] }, { 'result': [ 0, { self.spec.get_module_name(): self.spec.get_commands_spec() } ]})
# re-add this once we have new way to propagate spec changes (1 instead of the current 2 messages)
#self.assertEqual(len(self.fake_session.message_queue), 2)
diff --git a/src/lib/server_common/keyring.cc b/src/lib/server_common/keyring.cc
index f68db70..4bc9296 100644
--- a/src/lib/server_common/keyring.cc
+++ b/src/lib/server_common/keyring.cc
@@ -30,7 +30,12 @@ void
updateKeyring(const std::string&, ConstElementPtr data) {
ConstElementPtr list(data->get("keys"));
KeyringPtr load(new TSIGKeyRing);
- for (size_t i(0); i < list->size(); ++ i) {
+
+ // Note that 'data' only contains explicitly configured config parameters.
+ // So if we use the default list is NULL, rather than an empty list, and
+ // we must explicitly expect that case (and handle it just like an empty
+ // list).
+ for (size_t i(0); list && i < list->size(); ++ i) {
load->add(TSIGKey(list->get(i)->stringValue()));
}
keyring.swap(load);
diff --git a/src/lib/server_common/keyring.h b/src/lib/server_common/keyring.h
index 8832095..9c067e9 100644
--- a/src/lib/server_common/keyring.h
+++ b/src/lib/server_common/keyring.h
@@ -28,14 +28,20 @@
* on updates.
*
* You simply initialize/load the keyring with isc::server_common::initKeyring
- * and then just use the key ring in in isc::server_common::keyring. It is
- * automatically reloaded, when the configuration updates, so you no longer
+ * and then just use the key ring referred to by isc::server_common::keyring. It
+ * is automatically reloaded, when the configuration updates, so you no longer
* needs to care about it.
*
* If you want to keep a key (or session) for longer time or your application
- * is multithreaded, you might want to have a copy of the shared pointer.
- * Otherwise an update might replace the keyring and delete the keys in the
- * old one.
+ * is multithreaded, you might want to have a copy of the shared pointer to
+ * hold a reference. Otherwise an update might replace the keyring and delete
+ * the keys in the old one.
+ *
+ * Also note that, while the interface doesn't prevent application from
+ * modifying the keyring, it is not a good idea to do so. As mentioned above,
+ * it might get reloaded at any time, which would replace the modified keyring.
+ * The possibility to modify it is side effect of simpler implementation and
+ * shorter code, not a goal.
*/
namespace isc {
diff --git a/src/lib/server_common/tests/keyring_test.cc b/src/lib/server_common/tests/keyring_test.cc
index 6d2f226..d79b541 100644
--- a/src/lib/server_common/tests/keyring_test.cc
+++ b/src/lib/server_common/tests/keyring_test.cc
@@ -38,20 +38,29 @@ public:
specfile(std::string(TEST_DATA_PATH) + "/spec.spec")
{
session.getMessages()->add(createAnswer());
- mccs.reset(new ModuleCCSession(specfile, session, NULL, NULL));
+ mccs.reset(new ModuleCCSession(specfile, session, NULL, NULL, false));
}
isc::cc::FakeSession session;
std::auto_ptr<ModuleCCSession> mccs;
std::string specfile;
- void doInit() {
+ void doInit(bool with_key = true) {
// Prepare the module specification for it and the config
session.getMessages()->
add(createAnswer(0,
moduleSpecFromFile(std::string(PLUGIN_DATA_PATH) +
"/tsig_keys.spec").
getFullSpec()));
- session.getMessages()->add(createAnswer(0, Element::fromJSON(
- "{\"keys\": [\"key:MTIzNAo=:hmac-sha1\"]}")));
+ if (with_key) {
+ session.getMessages()->add(
+ createAnswer(0, Element::fromJSON(
+ "{\"keys\": [\"key:MTIzNAo=:hmac-sha1\"]}")));
+ } else {
+ // This emulates the case of using the spec default. Note that
+ // the default value won't be passed to the config handler, so
+ // we'll pass an empty object, instead of {"keys": []}.
+ session.getMessages()->add(createAnswer(0,
+ Element::fromJSON("{}")));
+ }
// Now load it
EXPECT_NO_THROW(initKeyring(*mccs));
EXPECT_NE(keyring, boost::shared_ptr<TSIGKeyRing>()) <<
@@ -97,6 +106,14 @@ TEST_F(KeyringTest, keyring) {
}
}
+TEST_F(KeyringTest, keyringWithDefault) {
+ // If we don't explicitly specify a keyring, the default (no key) will
+ // be used.
+ doInit(false);
+ EXPECT_EQ(0, keyring->size());
+ deinitKeyring(*mccs);
+}
+
// Init twice
TEST_F(KeyringTest, initTwice) {
// It is NULL before
diff --git a/src/lib/testutils/srv_test.cc b/src/lib/testutils/srv_test.cc
index 1d79d71..dd3e425 100644
--- a/src/lib/testutils/srv_test.cc
+++ b/src/lib/testutils/srv_test.cc
@@ -72,9 +72,13 @@ SrvTestBase::createDataFromFile(const char* const datafile,
void
SrvTestBase::createRequestPacket(Message& message,
- const int protocol)
+ const int protocol, TSIGContext* context)
{
- message.toWire(request_renderer);
+ if (context == NULL) {
+ message.toWire(request_renderer);
+ } else {
+ message.toWire(request_renderer, *context);
+ }
delete io_message;
diff --git a/src/lib/testutils/srv_test.h b/src/lib/testutils/srv_test.h
index a848ffc..c92e876 100644
--- a/src/lib/testutils/srv_test.h
+++ b/src/lib/testutils/srv_test.h
@@ -84,7 +84,8 @@ protected:
/// form of \c IOMessage in \c io_message.
/// The existing content of \c io_message, if any, will be deleted.
void createRequestPacket(isc::dns::Message& message,
- const int protocol = IPPROTO_UDP);
+ const int protocol = IPPROTO_UDP,
+ isc::dns::TSIGContext* context = NULL);
MockSession notify_session;
MockServer dnsserv;
More information about the bind10-changes
mailing list