From: JINMEI Tatuya Date: Tue, 29 May 2012 21:51:12 +0000 (-0700) Subject: [2013] added TSIG handling X-Git-Tag: trac2351_base~226^2~60^2^2~2^2~18 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=100ee5216c3f6402ee08756f35c7af01fa363100;p=thirdparty%2Fkea.git [2013] added TSIG handling --- diff --git a/src/bin/ddns/ddns.py.in b/src/bin/ddns/ddns.py.in index d1511f902e..29ce2693a8 100755 --- a/src/bin/ddns/ddns.py.in +++ b/src/bin/ddns/ddns.py.in @@ -25,6 +25,7 @@ from isc.config.ccsession import * from isc.cc import SessionError, SessionTimeout import isc.util.process import isc.util.cio.socketsession +import isc.server_common.tsig_keyring import select import errno @@ -121,10 +122,14 @@ class DDNSServer: self._listen_socket.bind(SOCKET_FILE) self._listen_socket.listen(16) - # DDNS Protocol handling class. Parameterized for convenience of - # tests. Essentially private (and constant), but for tests' - # convenience defined as "protected". + # The following attributes are essentially private and constant, + # but defined as "protected" so that test code can customize them. + # They should not be overridden for any other purposes. + # + # DDNS Protocol handling class. self._UpdateSessionClass = isc.ddns.session.UpdateSession + # A callabale that returns effective TSIG key ring. + self._get_tsig_keyring = isc.server_common.tsig_keyring.get_keyring class SessionError(Exception): '''Exception for internal errors in an update session. @@ -195,6 +200,29 @@ class DDNSServer: # continue with the rest logger.error(DDNS_ACCEPT_FAILURE, e) + def __check_request_tsig(self, msg, req_data): + '''TSIG checker for update requests. + + This is a helper method for handle_request() below. It examines + the given update request message to see if it contains a TSIG RR, + and verifies the signature if it does. It returs the TSIG context + used for the verification, or None if the request doesn't contain + a TSIG. If the verification fails it simply raises an exception + as handle_request() assumes it should succeed. + + ''' + tsig_record = msg.get_tsig_record() + if tsig_record is None: + return None + tsig_ctx = TSIGContext(tsig_record.get_name(), + tsig_record.get_rdata().get_algorithm(), + self._get_tsig_keyring()) + tsig_error = tsig_ctx.verify(tsig_record, req_data) + if tsig_error != TSIGError.NOERROR: + raise SessionError("Failed to verify request's TSIG: " + + str(tsig_error)) + return tsig_ctx + def handle_request(self, req_session): """ This is the place where the actual DDNS processing is done. Other @@ -222,18 +250,28 @@ class DDNSServer: if msg.get_opcode() != Opcode.UPDATE(): raise SessionError('Update request has unexpected opcode: ' + str(msg.get_opcode())) - # TODO: TSIG check + tsig_ctx = self.__check_request_tsig(msg, req_data) except Exception as ex: + logger.error(DDNS_REQUEST_PARSE_FAIL, ex) return False # TODO: Don't propagate most of the exceptions (like datasrc errors), # just drop the packet. + # Let an update session object handle the request. update_session = self._UpdateSessionClass(msg, remote_addr, None) result, zname, zclass = update_session.handle() + + # If the request should be dropped, we're done; otherwise, send the + # response generated by the session object. + if result == isc.ddns.session.UPDATE_DROP: + return False msg = update_session.get_message() renderer = MessageRenderer() - msg.to_wire(renderer) + if tsig_ctx is not None: + msg.to_wire(renderer, tsig_ctx) + else: + msg.to_wire(renderer) sock.sendto(renderer.get_data(), remote_addr) return True diff --git a/src/bin/ddns/ddns_messages.mes b/src/bin/ddns/ddns_messages.mes index 996e663eb9..be05c65812 100644 --- a/src/bin/ddns/ddns_messages.mes +++ b/src/bin/ddns/ddns_messages.mes @@ -88,3 +88,14 @@ process will now shut down. The b10-ddns process encountered an uncaught exception and will now shut down. This is indicative of a programming error and should not happen under normal circumstances. The exception type and message are printed. + +% DDNS_REQUEST_PARSE_FAIL failed to parse update request: %1 +b10-ddns received an update request via b10-auth, but the received +data failed to pass minimum validation: it was either totally broken +as an any valid DNS message (there can be many reasons for that), or +the opcode is not update, or TSIG is included in the request but it +fails to validate. Since b10-auth should have performed this level of +checks, such an error shouldn't be detected at this stage and should +rather be considered an internal bug. This event is therefore logged +at the error level, and the request is simply dropped. Additional +information of the error is also logged. diff --git a/src/bin/ddns/tests/ddns_test.py b/src/bin/ddns/tests/ddns_test.py index 1bb7684036..afb2d4346f 100755 --- a/src/bin/ddns/tests/ddns_test.py +++ b/src/bin/ddns/tests/ddns_test.py @@ -34,7 +34,16 @@ TEST_QID = 5353 # arbitrary chosen TEST_RRCLASS = RRClass.IN() TEST_SERVER6 = ('2001:db8::53', 53, 0, 0) TEST_CLIENT6 = ('2001:db8::1', 53000, 0, 0) +TEST_SERVER4 = ('192.0.2.53', 53) +TEST_CLIENT4 = ('192.0.2.1', 53534) TEST_ZONE_RECORD = Question(TEST_ZONE_NAME, TEST_RRCLASS, UPDATE_RRTYPE) +# TSIG key for tests when needed. The key name is TEST_ZONE_NAME. +TEST_TSIG_KEY = TSIGKey("example.org:SFuWd/q99SzF8Yzd1QbB9g==") +# TSIG keyring that contanins the test key +TEST_TSIG_KEYRING = TSIGKeyRing() +TEST_TSIG_KEYRING.add(TEST_TSIG_KEY) +# Another TSIG key not in the keyring, making verification fail +BAD_TSIG_KEY = TSIGKey("example.com:SFuWd/q99SzF8Yzd1QbB9g==") class FakeSocket: """ @@ -53,6 +62,10 @@ class FakeSocket: def sendto(self, data, addr): self._sent_data = data self._sent_addr = addr + def clear(self): + '''Clear internal instrumental data.''' + self._sent_data = None + self._sent_addr = None class FakeSessionReceiver: """ @@ -68,16 +81,37 @@ class FakeSessionReceiver: return self._socket class FakeUpdateSession: - def __init__(self, msg, client_addr, zone_config): + '''A fake update session, emulating isc.ddns.session.UpdateSession. + + It provides the same interfaces as UpdateSession with skipping complicated + internal protocol processing and returning given faked results. This + will help simplify test setups. + + ''' + def __init__(self, msg, client_addr, zone_config, faked_result): + '''Faked constructor. + + It takes an additional faked_result parameter. It will be used + as the result value of handle(). If its value is UPDATE_ERROR, + get_message() will create a response message whose Rcode is + REFUSED. + + ''' self.__msg = msg + self.__faked_result = faked_result def handle(self): - return UPDATE_SUCCESS, TEST_ZONE_NAME, TEST_RRCLASS + if self.__faked_result == UPDATE_SUCCESS: + return self.__faked_result, TEST_ZONE_NAME, TEST_RRCLASS + return self.__faked_result, None, None def get_message(self): self.__msg.make_response() self.__msg.clear_section(SECTION_ZONE) - self.__msg.set_rcode(Rcode.NOERROR()) + if self.__faked_result == UPDATE_SUCCESS: + self.__msg.set_rcode(Rcode.NOERROR()) + else: + self.__msg.set_rcode(Rcode.REFUSED()) return self.__msg class MyCCSession(isc.config.ConfigData): @@ -390,7 +424,7 @@ class TestDDNSServer(unittest.TestCase): self.__select_expected = ([1, 2], [], [], None) self.assertRaises(select.error, self.ddns_server.run) -def create_msg(opcode=Opcode.UPDATE(), zones=[TEST_ZONE_RECORD], tsig_key=None): +def create_msg(opcode=Opcode.UPDATE(), zones=[TEST_ZONE_RECORD], tsigctx=None): msg = Message(Message.RENDER) msg.set_qid(TEST_QID) msg.set_opcode(opcode) @@ -399,8 +433,8 @@ def create_msg(opcode=Opcode.UPDATE(), zones=[TEST_ZONE_RECORD], tsig_key=None): msg.add_question(z) renderer = MessageRenderer() - if tsig_key is not None: - msg.to_wire(renderer, TSIGContext(tsig_key)) + if tsigctx is not None: + msg.to_wire(renderer, tsigctx) else: msg.to_wire(renderer) @@ -416,30 +450,79 @@ class TestDDNSession(unittest.TestCase): cc_session = MyCCSession() self.assertFalse(cc_session._started) self.server = ddns.DDNSServer(cc_session) - self.server._UpdateSessionClass = FakeUpdateSession + self.server._UpdateSessionClass = self.__fake_session_creator + self.server._get_tsig_keyring = self.__fake_keyring_getter + self.__faked_result = UPDATE_SUCCESS # will be returned by fake session self.__sock = FakeSocket(-1) - def check_update_response(self, resp_wire, expected_rcode=Rcode.NOERROR()): + def __fake_session_creator(self, req_message, client_addr, zone_config): + return FakeUpdateSession(req_message, client_addr, zone_config, + self.__faked_result) + + def __fake_keyring_getter(self): + return TEST_TSIG_KEYRING + + def check_update_response(self, resp_wire, expected_rcode=Rcode.NOERROR(), + tsig_ctx=None): '''Check if given wire data are valid form of update response. In this implementation, zone/prerequisite/update sections should be empty in responses. + If tsig_ctx (isc.dns.TSIGContext) is not None, the response should + be TSIG signed and the signature should be verifiable with the context + that has signed the corresponding request. + ''' msg = Message(Message.PARSE) msg.from_wire(resp_wire) + if tsig_ctx is not None: + tsig_record = msg.get_tsig_record() + self.assertNotEqual(None, tsig_record) + self.assertEqual(TSIGError.NOERROR, + tsig_ctx.verify(tsig_record, resp_wire)) self.assertEqual(Opcode.UPDATE(), msg.get_opcode()) self.assertEqual(expected_rcode, msg.get_rcode()) self.assertEqual(TEST_QID, msg.get_qid()) for section in [SECTION_ZONE, SECTION_PREREQUISITE, SECTION_UPDATE]: self.assertEqual(0, msg.get_rr_count(section)) + def check_session(self, result=UPDATE_SUCCESS, ipv6=True, tsig_key=None): + # reset test parameters + self.__sock.clear() + self.__faked_result = result + + server_addr = TEST_SERVER6 if ipv6 else TEST_SERVER4 + client_addr = TEST_CLIENT6 if ipv6 else TEST_CLIENT4 + tsig = TSIGContext(tsig_key) if tsig_key is not None else None + rcode = Rcode.NOERROR() if result == UPDATE_SUCCESS else Rcode.REFUSED() + has_response = False if result == UPDATE_DROP else True + + self.assertEqual(has_response, + self.server.handle_request((self.__sock, + server_addr, client_addr, + create_msg(tsigctx=tsig)))) + if has_response: + self.assertEqual(client_addr, self.__sock._sent_addr) + self.check_update_response(self.__sock._sent_data, rcode) + else: + self.assertEqual((None, None), (self.__sock._sent_addr, + self.__sock._sent_data)) + def test_handle_request(self): - self.assertTrue(self.server.handle_request((self.__sock, - TEST_SERVER6, TEST_CLIENT6, - create_msg()))) - self.assertEqual(TEST_CLIENT6, self.__sock._sent_addr) - self.check_update_response(self.__sock._sent_data) + '''Basic request handling without any unexpected errors.''' + # Success, without TSIG + self.check_session() + # Update will be refused with a response. + self.check_session(UPDATE_ERROR, ipv6=False) + # Update will be refused and dropped + self.check_session(UPDATE_DROP) + # Success, with TSIG + self.check_session(ipv6=False, tsig_key=TEST_TSIG_KEY) + # Update will be refused with a response, with TSIG. + self.check_session(UPDATE_ERROR, tsig_key=TEST_TSIG_KEY) + # Update will be refused and dropped, with TSIG (doesn't matter though) + self.check_session(UPDATE_DROP, ipv6=False, tsig_key=TEST_TSIG_KEY) def test_broken_request(self): # Message data too short @@ -453,6 +536,11 @@ class TestDDNSession(unittest.TestCase): (self.__sock, None, None, create_msg(opcode=Opcode.QUERY())))) self.assertEqual((None, None), (s._sent_data, s._sent_addr)) + # TSIG verification error. We use UPDATE_DROP to signal check_session + # that no response should be given. + self.check_session(result=UPDATE_DROP, ipv6=False, + tsig_key=BAD_TSIG_KEY) + class TestMain(unittest.TestCase): def setUp(self): self._server = MyDDNSServer()