From: Jelte Jansen Date: Tue, 22 May 2012 12:57:15 +0000 (+0200) Subject: [1455] restrict empty rrsets to class ANY and NONE X-Git-Tag: trac2351_base~226^2~67^2~19 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=39fee4a9b55d84275c3fa6535e2c2bbb4a311f44;p=thirdparty%2Fkea.git [1455] restrict empty rrsets to class ANY and NONE also implement the rest of the handling (adding to messages, converting to text, etc) --- diff --git a/src/lib/dns/message.cc b/src/lib/dns/message.cc index d54a187d44..a9be8be75e 100644 --- a/src/lib/dns/message.cc +++ b/src/lib/dns/message.cc @@ -130,6 +130,11 @@ public: const RRClass& rrclass, const RRType& rrtype, const RRTTL& ttl, ConstRdataPtr rdata, Message::ParseOptions options); + // There are also times where an RR needs to be added that + // represents an empty RRset. There is no Rdata in that case + void addRR(Message::Section section, const Name& name, + const RRClass& rrclass, const RRType& rrtype, + const RRTTL& ttl, Message::ParseOptions options); void addEDNS(Message::Section section, const Name& name, const RRClass& rrclass, const RRType& rrtype, const RRTTL& ttl, const Rdata& rdata); @@ -740,6 +745,17 @@ MessageImpl::parseSection(const Message::Section section, const RRClass rrclass(buffer.readUint16()); const RRTTL ttl(buffer.readUint32()); const size_t rdlen = buffer.readUint16(); + + // If class is ANY or NONE, rdlength may be zero, to signal + // an empty RRset. + // (the class check must be done to differentiate from RRTypes + // that can have zero length rdata + if ((rrclass == RRClass::ANY() || rrclass == RRClass::NONE()) && + rdlen == 0) { + addRR(section, name, rrclass, rrtype, ttl, options); + ++added; + continue; + } ConstRdataPtr rdata = createRdata(rrtype, rrclass, buffer, rdlen); if (rrtype == RRType::OPT()) { @@ -777,6 +793,24 @@ MessageImpl::addRR(Message::Section section, const Name& name, rrsets_[section].push_back(rrset); } +void +MessageImpl::addRR(Message::Section section, const Name& name, + const RRClass& rrclass, const RRType& rrtype, + const RRTTL& ttl, Message::ParseOptions options) +{ + if ((options & Message::PRESERVE_ORDER) == 0) { + vector::iterator it = + find_if(rrsets_[section].begin(), rrsets_[section].end(), + MatchRR(name, rrtype, rrclass)); + if (it != rrsets_[section].end()) { + (*it)->setTTL(min((*it)->getTTL(), ttl)); + return; + } + } + RRsetPtr rrset(new RRset(name, rrclass, rrtype, ttl)); + rrsets_[section].push_back(rrset); +} + void MessageImpl::addEDNS(Message::Section section, const Name& name, const RRClass& rrclass, const RRType& rrtype, diff --git a/src/lib/dns/python/rrset_python.cc b/src/lib/dns/python/rrset_python.cc index cb85533cf2..299252289d 100644 --- a/src/lib/dns/python/rrset_python.cc +++ b/src/lib/dns/python/rrset_python.cc @@ -251,22 +251,28 @@ RRset_toWire(PyObject* self_p, PyObject* args) { PyObject* mr; const s_RRset* self(static_cast(self_p)); - if (PyArg_ParseTuple(args, "O", &bytes) && PySequence_Check(bytes)) { - PyObject* bytes_o = bytes; - - OutputBuffer buffer(4096); - self->cppobj->toWire(buffer); - PyObject* n = PyBytes_FromStringAndSize(static_cast(buffer.getData()), buffer.getLength()); - PyObject* result = PySequence_InPlaceConcat(bytes_o, n); - // We need to release the object we temporarily created here - // to prevent memory leak - Py_DECREF(n); - return (result); - } else if (PyArg_ParseTuple(args, "O!", &messagerenderer_type, &mr)) { - self->cppobj->toWire(PyMessageRenderer_ToMessageRenderer(mr)); - // If we return NULL it is seen as an error, so use this for - // None returns - Py_RETURN_NONE; + try { + if (PyArg_ParseTuple(args, "O", &bytes) && PySequence_Check(bytes)) { + PyObject* bytes_o = bytes; + + OutputBuffer buffer(4096); + self->cppobj->toWire(buffer); + PyObject* n = PyBytes_FromStringAndSize(static_cast(buffer.getData()), buffer.getLength()); + PyObject* result = PySequence_InPlaceConcat(bytes_o, n); + // We need to release the object we temporarily created here + // to prevent memory leak + Py_DECREF(n); + return (result); + } else if (PyArg_ParseTuple(args, "O!", &messagerenderer_type, &mr)) { + self->cppobj->toWire(PyMessageRenderer_ToMessageRenderer(mr)); + // If we return NULL it is seen as an error, so use this for + // None returns + Py_RETURN_NONE; + } + } catch (const EmptyRRset& ers) { + PyErr_Clear(); + PyErr_SetString(po_EmptyRRset, ers.what()); + return (NULL); } PyErr_Clear(); PyErr_SetString(PyExc_TypeError, diff --git a/src/lib/dns/python/tests/rrset_python_test.py b/src/lib/dns/python/tests/rrset_python_test.py index 153180e4e7..0544872d34 100644 --- a/src/lib/dns/python/tests/rrset_python_test.py +++ b/src/lib/dns/python/tests/rrset_python_test.py @@ -30,6 +30,7 @@ class TestModuleSpec(unittest.TestCase): self.test_nsname = Name("ns.example.com") self.rrset_a = RRset(self.test_name, RRClass("IN"), RRType("A"), RRTTL(3600)) self.rrset_a_empty = RRset(self.test_name, RRClass("IN"), RRType("A"), RRTTL(3600)) + self.rrset_any_a_empty = RRset(self.test_name, RRClass("ANY"), RRType("A"), RRTTL(3600)) self.rrset_ns = RRset(self.test_domain, RRClass("IN"), RRType("NS"), RRTTL(86400)) self.rrset_ch_txt = RRset(self.test_domain, RRClass("CH"), RRType("TXT"), RRTTL(0)) self.MAX_RDATA_COUNT = 100 @@ -90,19 +91,23 @@ class TestModuleSpec(unittest.TestCase): self.assertRaises(EmptyRRset, self.rrset_a_empty.to_text) + self.assertEqual("test.example.com. 3600 ANY A\n", + self.rrset_any_a_empty.to_text()) + def test_to_wire_buffer(self): exp_buffer = bytearray(b'\x04test\x07example\x03com\x00\x00\x01\x00\x01\x00\x00\x0e\x10\x00\x04\xc0\x00\x02\x01\x04test\x07example\x03com\x00\x00\x01\x00\x01\x00\x00\x0e\x10\x00\x04\xc0\x00\x02\x02') buffer = bytearray() self.rrset_a.to_wire(buffer) self.assertEqual(exp_buffer, buffer) - exp_buffer = bytearray(b'\x04test\x07example\x03com\x00\x00\x01\x00\x01\x00\x00\x0e\x10\x00\x00') + self.assertRaises(EmptyRRset, self.rrset_a_empty.to_wire, buffer); + self.assertRaises(TypeError, self.rrset_a.to_wire, 1) + + exp_buffer = bytearray(b'\x04test\x07example\x03com\x00\x00\x01\x00\xff\x00\x00\x0e\x10\x00\x00') buffer = bytearray() - self.rrset_a_empty.to_wire(buffer) + self.rrset_any_a_empty.to_wire(buffer) self.assertEqual(exp_buffer, buffer) - self.assertRaises(TypeError, self.rrset_a.to_wire, 1) - def test_to_wire_renderer(self): exp_buffer = bytearray(b'\x04test\x07example\x03com\x00\x00\x01\x00\x01\x00\x00\x0e\x10\x00\x04\xc0\x00\x02\x01\xc0\x00\x00\x01\x00\x01\x00\x00\x0e\x10\x00\x04\xc0\x00\x02\x02') mr = MessageRenderer() diff --git a/src/lib/dns/rrset.cc b/src/lib/dns/rrset.cc index 780ed7d1f2..b18fbe8f12 100644 --- a/src/lib/dns/rrset.cc +++ b/src/lib/dns/rrset.cc @@ -43,14 +43,24 @@ AbstractRRset::toText() const { string s; RdataIteratorPtr it = getRdataIterator(); + // In the case of an empty rrset, just print name, ttl, class, and + // type if (it->isLast()) { - isc_throw(EmptyRRset, "ToText() is attempted for an empty RRset"); + // But only for class ANY or NONE + if (getClass() != RRClass::ANY() && + getClass() != RRClass::NONE()) { + isc_throw(EmptyRRset, "toText() is attempted for an empty RRset"); + } + + s += getName().toText() + " " + getTTL().toText() + " " + + getClass().toText() + " " + getType().toText() + "\n"; + return (s); } do { s += getName().toText() + " " + getTTL().toText() + " " + - getClass().toText() + " " + getType().toText() + " " + - it->getCurrent().toText() + "\n"; + getClass().toText() + " " + getType().toText() + " " + + it->getCurrent().toText() + "\n"; it->next(); } while (!it->isLast()); @@ -65,6 +75,12 @@ rrsetToWire(const AbstractRRset& rrset, T& output, const size_t limit) { RdataIteratorPtr it = rrset.getRdataIterator(); if (it->isLast()) { + // empty rrsets are only allowed for classes ANY and NONE + if (rrset.getClass() != RRClass::ANY() && + rrset.getClass() != RRClass::NONE()) { + isc_throw(EmptyRRset, "toWire() is attempted for an empty RRset"); + } + // For an empty RRset, write the name, type, class and TTL once, // followed by empty rdata. rrset.getName().toWire(output); @@ -72,7 +88,8 @@ rrsetToWire(const AbstractRRset& rrset, T& output, const size_t limit) { rrset.getClass().toWire(output); rrset.getTTL().toWire(output); output.writeUint16(0); - return (n); + // Still counts as 1 'rr'; it does show up in the message + return (1); } // sort the set of Rdata based on rrset-order and sortlist, and possible @@ -243,6 +260,7 @@ public: ~BasicRdataIterator() {} virtual void first() { it_ = datavector_->begin(); } virtual void next() { ++it_; } + virtual bool currentEmpty() const { return (!(*it_)); } virtual const rdata::Rdata& getCurrent() const { return (**it_); } virtual bool isLast() const { return (it_ == datavector_->end()); } private: diff --git a/src/lib/dns/rrset.h b/src/lib/dns/rrset.h index 43ade58063..eee0c8eb64 100644 --- a/src/lib/dns/rrset.h +++ b/src/lib/dns/rrset.h @@ -267,8 +267,6 @@ public: /// the resulting string with a trailing newline character. /// (following the BIND9 convention) /// - /// The RRset must contain some RDATA; otherwise, an exception of class - /// \c EmptyRRset will be thrown. /// If resource allocation fails, a corresponding standard exception /// will be thrown. /// The default implementation may throw other exceptions if the @@ -299,8 +297,6 @@ public: /// /// If resource allocation fails, a corresponding standard exception /// will be thrown. - /// The RRset must contain some RDATA; otherwise, an exception of class - /// \c EmptyRRset will be thrown. /// The default implementation may throw other exceptions if the /// \c toWire() method of the RDATA objects throws. /// If a derived class of \c AbstractRRset overrides the default @@ -560,6 +556,11 @@ public: /// This method should never throw an exception. virtual void next() = 0; + /// \brief Check if 'current' is empty (in which case getCurrent() would + /// fail + /// \return True if the current Rdata field is NULL, false if not + virtual bool currentEmpty() const = 0; + /// \brief Return the current \c Rdata corresponding to the rdata cursor. /// /// \return A reference to an \c rdata::Rdata object corresponding diff --git a/src/lib/dns/tests/rrset_unittest.cc b/src/lib/dns/tests/rrset_unittest.cc index 5049dd6416..c37557950b 100644 --- a/src/lib/dns/tests/rrset_unittest.cc +++ b/src/lib/dns/tests/rrset_unittest.cc @@ -46,6 +46,10 @@ protected: rrset_a(test_name, RRClass::IN(), RRType::A(), RRTTL(3600)), rrset_a_empty(test_name, RRClass::IN(), RRType::A(), RRTTL(3600)), + rrset_any_a_empty(test_name, RRClass::ANY(), RRType::A(), + RRTTL(3600)), + rrset_none_a_empty(test_name, RRClass::NONE(), RRType::A(), + RRTTL(3600)), rrset_ns(test_domain, RRClass::IN(), RRType::NS(), RRTTL(86400)), rrset_ch_txt(test_domain, RRClass::CH(), RRType::TXT(), @@ -62,6 +66,8 @@ protected: Name test_nsname; RRset rrset_a; RRset rrset_a_empty; + RRset rrset_any_a_empty; + RRset rrset_none_a_empty; RRset rrset_ns; RRset rrset_ch_txt; std::vector wiredata; @@ -193,11 +199,16 @@ TEST_F(RRsetTest, toText) { "test.example.com. 3600 IN A 192.0.2.2\n", rrset_a.toText()); - // toText() cannot be performed for an empty RRset. + // toText() cannot be performed for an empty RRset EXPECT_THROW(rrset_a_empty.toText(), EmptyRRset); + + // Unless it is type ANY or NONE + EXPECT_EQ("test.example.com. 3600 ANY A\n", + rrset_any_a_empty.toText()); + EXPECT_EQ("test.example.com. 3600 CLASS254 A\n", + rrset_none_a_empty.toText()); } -#include TEST_F(RRsetTest, toWireBuffer) { rrset_a.toWire(buffer); @@ -207,11 +218,21 @@ TEST_F(RRsetTest, toWireBuffer) { // toWire() cannot be performed for an empty RRset. buffer.clear(); - rrset_a_empty.toWire(buffer); + EXPECT_THROW(rrset_a_empty.toWire(buffer), EmptyRRset); + + // Unless it is type ANY or None + buffer.clear(); + rrset_any_a_empty.toWire(buffer); wiredata.clear(); UnitTestUtil::readWireData("rrset_toWire3", wiredata); EXPECT_PRED_FORMAT4(UnitTestUtil::matchWireData, buffer.getData(), buffer.getLength(), &wiredata[0], wiredata.size()); + buffer.clear(); + rrset_none_a_empty.toWire(buffer); + wiredata.clear(); + UnitTestUtil::readWireData("rrset_toWire4", wiredata); + EXPECT_PRED_FORMAT4(UnitTestUtil::matchWireData, buffer.getData(), + buffer.getLength(), &wiredata[0], wiredata.size()); } TEST_F(RRsetTest, toWireRenderer) { @@ -224,13 +245,25 @@ TEST_F(RRsetTest, toWireRenderer) { EXPECT_PRED_FORMAT4(UnitTestUtil::matchWireData, renderer.getData(), renderer.getLength(), &wiredata[0], wiredata.size()); + // toWire() cannot be performed for an empty RRset. + buffer.clear(); + EXPECT_THROW(rrset_a_empty.toWire(buffer), EmptyRRset); + + // Unless it is type ANY or None // toWire() can also be performed for an empty RRset. buffer.clear(); - rrset_a_empty.toWire(buffer); + rrset_any_a_empty.toWire(buffer); wiredata.clear(); UnitTestUtil::readWireData("rrset_toWire3", wiredata); EXPECT_PRED_FORMAT4(UnitTestUtil::matchWireData, buffer.getData(), buffer.getLength(), &wiredata[0], wiredata.size()); + + buffer.clear(); + rrset_none_a_empty.toWire(buffer); + wiredata.clear(); + UnitTestUtil::readWireData("rrset_toWire4", wiredata); + EXPECT_PRED_FORMAT4(UnitTestUtil::matchWireData, buffer.getData(), + buffer.getLength(), &wiredata[0], wiredata.size()); } // test operator<<. We simply confirm it appends the result of toText(). diff --git a/src/lib/dns/tests/testdata/rrset_toWire3 b/src/lib/dns/tests/testdata/rrset_toWire3 index 8bf58e293a..47f8e6b949 100644 --- a/src/lib/dns/tests/testdata/rrset_toWire3 +++ b/src/lib/dns/tests/testdata/rrset_toWire3 @@ -3,8 +3,8 @@ # #(4) t e s t (7) e x a m p l e (3) c o m . 04 74 65 73 74 07 65 78 61 6d 70 6c 65 03 63 6f 6d 00 -# type/class: A = 1, IN = 1 -00 01 00 01 +# type/class: A = 1, ANY = 255 +00 01 00 ff # TTL: 3600 00 00 0e 10 #6 7 diff --git a/src/lib/dns/tests/testdata/rrset_toWire4 b/src/lib/dns/tests/testdata/rrset_toWire4 new file mode 100644 index 0000000000..6fb409cab1 --- /dev/null +++ b/src/lib/dns/tests/testdata/rrset_toWire4 @@ -0,0 +1,12 @@ +# +# Rendering an empty IN/A RRset +# +#(4) t e s t (7) e x a m p l e (3) c o m . + 04 74 65 73 74 07 65 78 61 6d 70 6c 65 03 63 6f 6d 00 +# type/class: A = 1, ANY = 255 +00 01 00 fe +# TTL: 3600 +00 00 0e 10 +#6 7 +# RDLENGTH: 0 +00 00 diff --git a/src/lib/python/isc/ddns/session.py b/src/lib/python/isc/ddns/session.py index 9fd1219a7f..3b515874c3 100644 --- a/src/lib/python/isc/ddns/session.py +++ b/src/lib/python/isc/ddns/session.py @@ -250,4 +250,21 @@ class UpdateSession: return not self.__check_prerequisite_name_in_use(datasrc_client, rrset) def __check_prerequisites(self, datasrc_client): + '''Check the prerequisites section of the UPDATE Message. + RFC2136 Section 2.4''' + for rrset in self.__message.get_section(SECTION_PREREQUISITE): + # called atm, but not 'handled' yet + if rrset.getClass() == RRClass.ANY(): + # Check for each RR in the 'set' XXX + self.__check_prerequisite_exists(rrset) + elif rrset.getClass() == datasrc_client.getClass(): + self.__check_prerequisite_exists_value(datasrc_client, rrset) + elif rrset.getClass() == RRClass.NONE(): + self.__check_prerequisite_does_not_exist(datasrc_client, rrset) + elif rrset.getClass() == RRClass.ANY() and rrset.getType() == RRType.ANY(): + self.__check_prerequisite_name_exists(datasrc_client, rrset) + elif rrset.getClass() == RRClass.NONE() and rrset.getType() == RRType.ANY(): + self.__check_prerequisite_name_does_not_exist(datasrc_client, rrset) + else: + print("[XX] ERROR! unknown prerequisite") pass diff --git a/src/lib/python/isc/ddns/tests/session_tests.py b/src/lib/python/isc/ddns/tests/session_tests.py index 2ed9f61cd5..2a1cd5dbaa 100644 --- a/src/lib/python/isc/ddns/tests/session_tests.py +++ b/src/lib/python/isc/ddns/tests/session_tests.py @@ -36,13 +36,15 @@ TEST_ZONE_RECORD = Question(TEST_ZONE_NAME, TEST_RRCLASS, UPDATE_RRTYPE) TEST_CLIENT6 = ('2001:db8::1', 53, 0, 0) TEST_CLIENT4 = ('192.0.2.1', 53) -def create_update_msg(zones=[TEST_ZONE_RECORD]): +def create_update_msg(zones=[TEST_ZONE_RECORD], prerequisites=[]): msg = Message(Message.RENDER) msg.set_qid(5353) # arbitrary chosen msg.set_opcode(Opcode.UPDATE()) msg.set_rcode(Rcode.NOERROR()) for z in zones: msg.add_question(z) + for p in prerequisites: + msg.add_rrset(SECTION_PREREQUISITE, p) renderer = MessageRenderer() msg.to_wire(renderer) @@ -469,7 +471,69 @@ class SessionTest(unittest.TestCase): self.__check_prerequisite_name_not_in_use(False, self.__datasrc_client, rrset) def test_check_prerequisites(self): - pass + # This test checks if the actual prerequisite-type-specific + # methods are called. Whether those succeed or not is tested above (or is it) + + # Let's first define a number of prereq's that should succeed + rrset_exists_yes = isc.dns.RRset(isc.dns.Name("example.org"), + isc.dns.RRClass.ANY(), + isc.dns.RRType.SOA(), + isc.dns.RRTTL(0)) + rrset_exists_no = isc.dns.RRset(isc.dns.Name("foo.example.org"), + isc.dns.RRClass.ANY(), + isc.dns.RRType.SOA(), + isc.dns.RRTTL(0)) + + rrset_exists_value_yes = isc.dns.RRset(isc.dns.Name("www.example.org"), + isc.dns.RRClass.IN(), + isc.dns.RRType.A(), + isc.dns.RRTTL(0)) + rrset_exists_value_yes.add_rdata(isc.dns.Rdata(isc.dns.RRType.A(), + isc.dns.RRClass.IN(), + "192.0.2.1")) + + rrset_exists_value_no = isc.dns.RRset(isc.dns.Name("www.example.org"), + isc.dns.RRClass.IN(), + isc.dns.RRType.A(), + isc.dns.RRTTL(0)) + rrset_exists_value_no.add_rdata(isc.dns.Rdata(isc.dns.RRType.A(), + isc.dns.RRClass.IN(), + "192.0.2.2")) + + rrset_does_not_exist_yes = isc.dns.RRset(isc.dns.Name("foo.example.org"), + isc.dns.RRClass.NONE(), + isc.dns.RRType.SOA(), + isc.dns.RRTTL(0)) + rrset_does_not_exist_no = isc.dns.RRset(isc.dns.Name("foo.example.org"), + isc.dns.RRClass.NONE(), + isc.dns.RRType.SOA(), + isc.dns.RRTTL(0)) + + name_in_use_yes = isc.dns.RRset(isc.dns.Name("www.example.org"), + isc.dns.RRClass.ANY(), + isc.dns.RRType.ANY(), + isc.dns.RRTTL(0)) + name_in_use_no = isc.dns.RRset(isc.dns.Name("foo.example.org"), + isc.dns.RRClass.ANY(), + isc.dns.RRType.ANY(), + isc.dns.RRTTL(0)) + + name_not_in_use_yes = isc.dns.RRset(isc.dns.Name("foo.example.org"), + isc.dns.RRClass.ANY(), + isc.dns.RRType.ANY(), + isc.dns.RRTTL(0)) + + # Create an UPDATE with all 5 'yes' prereqs + data, update = create_update_msg([TEST_ZONE_RECORD], + [ + rrset_exists_yes, + rrset_does_not_exist_yes, + name_in_use_yes, + name_not_in_use_yes, + rrset_exists_value_yes + ]) + print("[XX]") + print(update.to_text()) if __name__ == "__main__": isc.log.init("bind10")