This commit fixes setting Protocol Buffer meta keys on DNS response via Lua FFI:
the existing code was assuming it was possible to use the question methods on a
response object which is not true and would likely have ended in a crash at some
point.
It also propates meta keys set on a DNS question to the corresponding DNS response.
Before this commit the values were not passed along to the response which was quite
unexpected, especially for self-answered responses.
Signed-off-by: Remi Gacogne <remi.gacogne@powerdns.com>
static thread_local std::string data;
data.clear();
message.serialize(data);
- if (!dnsquestion->d_rawProtobufContent.empty()) {
- data.insert(data.end(), dnsquestion->d_rawProtobufContent.begin(), dnsquestion->d_rawProtobufContent.end());
+ if (!dnsquestion->ids.d_rawProtobufContent.empty()) {
+ data.insert(data.end(), dnsquestion->ids.d_rawProtobufContent.begin(), dnsquestion->ids.d_rawProtobufContent.end());
}
remoteLoggerQueueData(*d_logger, data);
static thread_local std::string data;
data.clear();
message.serialize(data);
- if (!response->d_rawProtobufContent.empty()) {
- data.insert(data.end(), response->d_rawProtobufContent.begin(), response->d_rawProtobufContent.end());
+ if (!response->ids.d_rawProtobufContent.empty()) {
+ data.insert(data.end(), response->ids.d_rawProtobufContent.begin(), response->ids.d_rawProtobufContent.end());
}
d_logger->queueData(data);
ids.cs = cs;
/* in case we want to support XFR over DoH, or the stream ID becomes used for QUIC */
ids.d_streamID = d_streamID;
+#if !defined(DISABLE_PROTOBUF)
+ ids.d_rawProtobufContent = d_rawProtobufContent;
+#endif
return ids;
}
boost::optional<Netmask> subnet{boost::none}; // 40
std::string poolName; // 32
+#if !defined(DISABLE_PROTOBUF)
+ std::string d_rawProtobufContent; // protobuf-encoded content to add to protobuf messages // 32
+#endif /* DISABLE_PROTOBUF */
ComboAddress origRemote; // 28
ComboAddress origDest; // 28
ComboAddress hopRemote;
static void addMetaKeyAndValuesToProtobufContent([[maybe_unused]] DNSQuestion& dnsQuestion, [[maybe_unused]] const std::string& key, [[maybe_unused]] const LuaArray<boost::variant<int64_t, std::string>>& values)
{
#if !defined(DISABLE_PROTOBUF)
- protozero::pbf_writer pbfWriter{dnsQuestion.d_rawProtobufContent};
+ protozero::pbf_writer pbfWriter{dnsQuestion.ids.d_rawProtobufContent};
protozero::pbf_writer pbfMetaWriter{pbfWriter, static_cast<protozero::pbf_tag_type>(pdns::ProtoZero::Message::Field::meta)};
pbfMetaWriter.add_string(static_cast<protozero::pbf_tag_type>(pdns::ProtoZero::Message::MetaField::key), key);
protozero::pbf_writer pbfMetaValueWriter{pbfMetaWriter, static_cast<protozero::pbf_tag_type>(pdns::ProtoZero::Message::MetaField::value)};
void dnsdist_ffi_dnsquestion_meta_add_int64_value_to_key(dnsdist_ffi_dnsquestion_t* dnsQuestion, int64_t value) __attribute__ ((visibility ("default")));
/* this function should never be called if dnsdist_ffi_dnsquestion_meta_begin_key has not been called first */
void dnsdist_ffi_dnsquestion_meta_end_key(dnsdist_ffi_dnsquestion_t* dnsQuestion) __attribute__ ((visibility ("default")));
+
+/* this function adds a new key to the raw meta buffer. It can only be called with the same key on a given query once, and dnsdist_ffi_dnsresponse_meta_end_key should always be called after values have been added */
+void dnsdist_ffi_dnsresponse_meta_begin_key(dnsdist_ffi_dnsresponse_t* dnsResponse, const char* key, size_t keyLen) __attribute__ ((visibility ("default")));
+/* this function should never be called if dnsdist_ffi_dnsresponse_meta_begin_key has not been called first */
+void dnsdist_ffi_dnsresponse_meta_add_str_value_to_key(dnsdist_ffi_dnsresponse_t* dnsResponse, const char* value, size_t valueLen) __attribute__ ((visibility ("default")));
+/* this function should never be called if dnsdist_ffi_dnsresponse_meta_begin_key has not been called first */
+void dnsdist_ffi_dnsresponse_meta_add_int64_value_to_key(dnsdist_ffi_dnsresponse_t* dnsResponse, int64_t value) __attribute__ ((visibility ("default")));
+/* this function should never be called if dnsdist_ffi_dnsresponse_meta_begin_key has not been called first */
+void dnsdist_ffi_dnsresponse_meta_end_key(dnsdist_ffi_dnsresponse_t* dnsResponse) __attribute__ ((visibility ("default")));
return;
}
- dnsQuestion->pbfWriter = protozero::pbf_writer{dnsQuestion->dq->d_rawProtobufContent};
+ dnsQuestion->pbfWriter = protozero::pbf_writer{dnsQuestion->dq->ids.d_rawProtobufContent};
dnsQuestion->pbfMetaWriter = protozero::pbf_writer{dnsQuestion->pbfWriter, static_cast<protozero::pbf_tag_type>(pdns::ProtoZero::Message::Field::meta)};
dnsQuestion->pbfMetaWriter.add_string(static_cast<protozero::pbf_tag_type>(pdns::ProtoZero::Message::MetaField::key), protozero::data_view(key, keyLen));
dnsQuestion->pbfMetaValueWriter = protozero::pbf_writer {dnsQuestion->pbfMetaWriter, static_cast<protozero::pbf_tag_type>(pdns::ProtoZero::Message::MetaField::value)};
if (dnsQuestion == nullptr) {
return;
}
+
if (!dnsQuestion->pbfWriter.valid()) {
vinfolog("Error in dnsdist_ffi_dnsquestion_meta_end_key: trying to end a key that has not been started");
return;
}
#endif /* DISABLE_PROTOBUF */
}
+
+void dnsdist_ffi_dnsresponse_meta_begin_key([[maybe_unused]] dnsdist_ffi_dnsresponse_t* dnsResponse, [[maybe_unused]] const char* key, [[maybe_unused]] size_t keyLen)
+{
+#if !defined(DISABLE_PROTOBUF)
+ if (dnsResponse == nullptr || key == nullptr || keyLen == 0) {
+ return;
+ }
+
+ if (dnsResponse->pbfWriter.valid()) {
+ vinfolog("Error in dnsdist_ffi_dnsresponse_meta_begin_key: the previous key has not been ended");
+ return;
+ }
+
+ dnsResponse->pbfWriter = protozero::pbf_writer{dnsResponse->dr->ids.d_rawProtobufContent};
+ dnsResponse->pbfMetaWriter = protozero::pbf_writer{dnsResponse->pbfWriter, static_cast<protozero::pbf_tag_type>(pdns::ProtoZero::Message::Field::meta)};
+ dnsResponse->pbfMetaWriter.add_string(static_cast<protozero::pbf_tag_type>(pdns::ProtoZero::Message::MetaField::key), protozero::data_view(key, keyLen));
+ dnsResponse->pbfMetaValueWriter = protozero::pbf_writer {dnsResponse->pbfMetaWriter, static_cast<protozero::pbf_tag_type>(pdns::ProtoZero::Message::MetaField::value)};
+#endif /* DISABLE_PROTOBUF */
+}
+
+void dnsdist_ffi_dnsresponse_meta_add_str_value_to_key([[maybe_unused]] dnsdist_ffi_dnsresponse_t* dnsResponse, [[maybe_unused]] const char* value, [[maybe_unused]] size_t valueLen)
+{
+#if !defined(DISABLE_PROTOBUF)
+ if (dnsResponse == nullptr || value == nullptr || valueLen == 0) {
+ return;
+ }
+
+ if (!dnsResponse->pbfMetaValueWriter.valid()) {
+ vinfolog("Error in dnsdist_ffi_dnsresponse_meta_add_str_value_to_key: trying to add a value without starting a key");
+ return;
+ }
+
+ dnsResponse->pbfMetaValueWriter.add_string(static_cast<protozero::pbf_tag_type>(pdns::ProtoZero::Message::MetaValueField::stringVal), protozero::data_view(value, valueLen));
+#endif /* DISABLE_PROTOBUF */
+}
+
+void dnsdist_ffi_dnsresponse_meta_add_int64_value_to_key([[maybe_unused]] dnsdist_ffi_dnsresponse_t* dnsResponse, [[maybe_unused]] int64_t value)
+{
+#if !defined(DISABLE_PROTOBUF)
+ if (dnsResponse == nullptr) {
+ return;
+ }
+
+ if (!dnsResponse->pbfMetaValueWriter.valid()) {
+ vinfolog("Error in dnsdist_ffi_dnsresponse_meta_add_int64_value_to_key: trying to add a value without starting a key");
+ return;
+ }
+
+ dnsResponse->pbfMetaValueWriter.add_uint64(static_cast<protozero::pbf_tag_type>(pdns::ProtoZero::Message::MetaValueField::intVal), value);
+#endif /* DISABLE_PROTOBUF */
+}
+
+void dnsdist_ffi_dnsresponse_meta_end_key([[maybe_unused]] dnsdist_ffi_dnsresponse_t* dnsResponse)
+{
+#if !defined(DISABLE_PROTOBUF)
+ if (dnsResponse == nullptr) {
+ return;
+ }
+
+ if (!dnsResponse->pbfWriter.valid()) {
+ vinfolog("Error in dnsdist_ffi_dnsresponse_meta_end_key: trying to end a key that has not been started");
+ return;
+ }
+
+ try {
+ /* reset the pbf writer so that the sizes are properly updated */
+ dnsResponse->pbfMetaValueWriter.commit();
+ dnsResponse->pbfMetaWriter.commit();
+ dnsResponse->pbfWriter = protozero::pbf_writer();
+ }
+ catch (const std::exception& exp) {
+ vinfolog("Error in dnsdist_ffi_dnsresponse_meta_end_key: %s", exp.what());
+ }
+#endif /* DISABLE_PROTOBUF */
+}
std::unique_ptr<std::vector<dnsdist_ffi_proxy_protocol_value_t>> proxyProtocolValuesVect;
std::unique_ptr<std::unordered_map<std::string, std::string>> httpHeaders;
#if !defined(DISABLE_PROTOBUF)
- protozero::pbf_writer pbfWriter;
- protozero::pbf_writer pbfMetaWriter;
- protozero::pbf_writer pbfMetaValueWriter;
+ protozero::pbf_writer pbfWriter{};
+ protozero::pbf_writer pbfMetaWriter{};
+ protozero::pbf_writer pbfMetaValueWriter{};
#endif /* DISABLE_PROTOBUF */
};
DNSResponse* dr{nullptr};
std::optional<std::string> result{std::nullopt};
+#if !defined(DISABLE_PROTOBUF)
+ protozero::pbf_writer pbfWriter{};
+ protozero::pbf_writer pbfMetaWriter{};
+ protozero::pbf_writer pbfMetaValueWriter{};
+#endif /* DISABLE_PROTOBUF */
};
// dnsdist_ffi_server_t is a lightuserdata
InternalQueryState& ids;
std::unique_ptr<Netmask> ecs{nullptr};
std::string sni; /* Server Name Indication, if any (DoT or DoH) */
-#if !defined(DISABLE_PROTOBUF)
- std::string d_rawProtobufContent; /* protobuf-encoded content to add to protobuf messages */
-#endif /* DISABLE_PROTOBUF */
mutable std::unique_ptr<EDNSOptionViewMap> ednsOptions; /* this needs to be mutable because it is parsed just in time, when DNSQuestion is read-only */
std::shared_ptr<IncomingTCPConnectionState> d_incomingTCPState{nullptr};
std::unique_ptr<std::vector<ProxyProtocolValue>> proxyProtocolValues{nullptr};
.. versionadded:: 2.0.0
+ .. versionchanged:: 2.0.2
+ Prior to 2.0.2 meta-data entries were not propagated from questions to responses, which was especially unexpected for self-answered responses.
+
Set a meta-data entry to be exported in the ``meta`` field of ProtoBuf messages.
:param string key: The key
DNSQuestion dnsQuestion(ids, query);
dnsdist_ffi_dnsquestion_t lightDQ(&dnsQuestion);
+ DNSResponse dnsResponse(ids, query, nullptr);
+ dnsdist_ffi_dnsresponse_t lightDR(&dnsResponse);
{
/* check invalid parameters */
dnsdist_ffi_dnsquestion_meta_add_str_value_to_key(&lightDQ, "some-str-value", 0);
dnsdist_ffi_dnsquestion_meta_add_int64_value_to_key(nullptr, 0);
dnsdist_ffi_dnsquestion_meta_end_key(nullptr);
+
+ dnsdist_ffi_dnsresponse_meta_begin_key(nullptr, nullptr, 0);
+ dnsdist_ffi_dnsresponse_meta_begin_key(&lightDR, nullptr, 0);
+ dnsdist_ffi_dnsresponse_meta_begin_key(&lightDR, "some-key", 0);
+ dnsdist_ffi_dnsresponse_meta_add_str_value_to_key(nullptr, nullptr, 0);
+ dnsdist_ffi_dnsresponse_meta_add_str_value_to_key(&lightDR, nullptr, 0);
+ dnsdist_ffi_dnsresponse_meta_add_str_value_to_key(&lightDR, "some-str-value", 0);
+ dnsdist_ffi_dnsresponse_meta_add_int64_value_to_key(nullptr, 0);
+ dnsdist_ffi_dnsresponse_meta_end_key(nullptr);
}
{
/* trying to end a key that has not been started */
dnsdist_ffi_dnsquestion_meta_end_key(&lightDQ);
+ dnsdist_ffi_dnsresponse_meta_end_key(&lightDR);
}
{
const std::string key{"some-key"};
const std::string value1{"first value"};
const std::string value2{"second value"};
- BOOST_CHECK_EQUAL(dnsQuestion.d_rawProtobufContent.size(), 0U);
+ BOOST_CHECK_EQUAL(dnsQuestion.ids.d_rawProtobufContent.size(), 0U);
dnsdist_ffi_dnsquestion_meta_begin_key(&lightDQ, key.data(), key.size());
/* we should not be able to begin a new key without ending it first */
dnsdist_ffi_dnsquestion_meta_begin_key(&lightDQ, key.data(), key.size());
dnsdist_ffi_dnsquestion_meta_add_str_value_to_key(&lightDQ, value2.data(), value2.size());
dnsdist_ffi_dnsquestion_meta_add_int64_value_to_key(&lightDQ, -42);
dnsdist_ffi_dnsquestion_meta_end_key(&lightDQ);
- BOOST_CHECK_EQUAL(dnsQuestion.d_rawProtobufContent.size(), 55U);
- BOOST_CHECK_EQUAL(Base64Encode(dnsQuestion.d_rawProtobufContent), "sgE0Cghzb21lLWtleRIoCgtmaXJzdCB2YWx1ZRAqCgxzZWNvbmQgdmFsdWUQ1v//////////AQ==");
+ BOOST_CHECK_EQUAL(dnsQuestion.ids.d_rawProtobufContent.size(), 55U);
+ BOOST_CHECK_EQUAL(Base64Encode(dnsQuestion.ids.d_rawProtobufContent), "sgE0Cghzb21lLWtleRIoCgtmaXJzdCB2YWx1ZRAqCgxzZWNvbmQgdmFsdWUQ1v//////////AQ==");
+ BOOST_CHECK_EQUAL(dnsResponse.ids.d_rawProtobufContent.size(), 55U);
+ BOOST_CHECK_EQUAL(Base64Encode(dnsResponse.ids.d_rawProtobufContent), "sgE0Cghzb21lLWtleRIoCgtmaXJzdCB2YWx1ZRAqCgxzZWNvbmQgdmFsdWUQ1v//////////AQ==");
+ }
+
+ {
+ const std::string key{"some-key"};
+ const std::string value1{"first value"};
+ const std::string value2{"second value"};
+ /* be careful: IDS is shared with the DNS question */
+ dnsResponse.ids.d_rawProtobufContent.clear();
+ dnsdist_ffi_dnsresponse_meta_begin_key(&lightDR, key.data(), key.size());
+ /* we should not be able to begin a new key without ending it first */
+ dnsdist_ffi_dnsresponse_meta_begin_key(&lightDR, key.data(), key.size());
+ dnsdist_ffi_dnsresponse_meta_add_str_value_to_key(&lightDR, value1.data(), value1.size());
+ dnsdist_ffi_dnsresponse_meta_add_int64_value_to_key(&lightDR, 42);
+ dnsdist_ffi_dnsresponse_meta_add_str_value_to_key(&lightDR, value2.data(), value2.size());
+ dnsdist_ffi_dnsresponse_meta_add_int64_value_to_key(&lightDR, -42);
+ dnsdist_ffi_dnsresponse_meta_end_key(&lightDR);
+ BOOST_CHECK_EQUAL(dnsResponse.ids.d_rawProtobufContent.size(), 55U);
+ BOOST_CHECK_EQUAL(Base64Encode(dnsResponse.ids.d_rawProtobufContent), "sgE0Cghzb21lLWtleRIoCgtmaXJzdCB2YWx1ZRAqCgxzZWNvbmQgdmFsdWUQ1v//////////AQ==");
}
}
#endif /* DISABLE_PROTOBUF */
local ffi = require("ffi")
local C = ffi.C
- function add_meta(dq)
+ function add_meta_to_query(dq)
local key = "my-meta-key-1"
- local key2 = "my-meta-key-2"
C.dnsdist_ffi_dnsquestion_meta_begin_key(dq, key, #key)
C.dnsdist_ffi_dnsquestion_meta_add_str_value_to_key(dq, "test", 4)
C.dnsdist_ffi_dnsquestion_meta_add_int64_value_to_key(dq, -42)
C.dnsdist_ffi_dnsquestion_meta_add_str_value_to_key(dq, "test2", 5)
C.dnsdist_ffi_dnsquestion_meta_end_key(dq)
+ return DNSAction.None
+ end
+ function add_meta_to_response(dr)
local key2 = "my-meta-key-2"
- C.dnsdist_ffi_dnsquestion_meta_begin_key(dq, key2, #key2)
- C.dnsdist_ffi_dnsquestion_meta_add_str_value_to_key(dq, "foo", 3)
- C.dnsdist_ffi_dnsquestion_meta_add_int64_value_to_key(dq, 42)
- C.dnsdist_ffi_dnsquestion_meta_add_str_value_to_key(dq, "bar", 3)
- C.dnsdist_ffi_dnsquestion_meta_end_key(dq)
+ C.dnsdist_ffi_dnsresponse_meta_begin_key(dr, key2, #key2)
+ C.dnsdist_ffi_dnsresponse_meta_add_str_value_to_key(dr, "foo", 3)
+ C.dnsdist_ffi_dnsresponse_meta_add_int64_value_to_key(dr, 42)
+ C.dnsdist_ffi_dnsresponse_meta_add_str_value_to_key(dr, "bar", 3)
+ C.dnsdist_ffi_dnsresponse_meta_end_key(dr)
+ return DNSResponseAction.None
+ end
+ function addMetaToQuery(dq)
+ dq:setMetaKey('my-meta-key-3', {'test', -42, 'test2'})
return DNSAction.None
end
function addMetaToResponse(dr)
- dr:setMetaKey('my-meta-key-1', {'test', -42, 'test2'})
- dr:setMetaKey('my-meta-key-2', {'foo', 42, 'bar'})
+ dr:setMetaKey('my-meta-key-4', {'foo', 42, 'bar'})
return DNSResponseAction.None
end
- addAction(AllRule(), LuaFFIAction(add_meta))
+ addAction(AllRule(), LuaFFIAction(add_meta_to_query))
addAction(AllRule(), SetTagAction('my-tag-key', 'my-tag-value'))
addAction(AllRule(), SetTagAction('my-empty-key', ''))
+ addAction(AllRule(), LuaAction(addMetaToQuery))
addAction(AllRule(), RemoteLogAction(rl, nil, {serverID='dnsdist-server-1', exportTags='*'}, {b64='b64-content', ['my-tag-export-name']='tag:my-tag-key'}))
+ addResponseAction(AllRule(), LuaFFIResponseAction(add_meta_to_response))
addResponseAction(AllRule(), LuaResponseAction(addMetaToResponse))
addResponseAction(AllRule(), SetTagResponseAction('my-tag-key2', 'my-tag-value2'))
addResponseAction(AllRule(), RemoteLogResponseAction(rl, nil, false, {serverID='dnsdist-server-1', exportTags='my-empty-key,my-tag-key2'}, {['my-tag-export-name']='tags'}))
self.assertIn('test2', msg.meta[2].value.stringVal)
self.assertIn(-42, msg.meta[2].value.intVal)
- self.assertEqual(msg.meta[3].key, 'my-meta-key-2')
- self.assertEqual(len(msg.meta[3].value.stringVal), 2)
- self.assertIn('foo', msg.meta[3].value.stringVal)
- self.assertIn('bar', msg.meta[3].value.stringVal)
- self.assertIn(42, msg.meta[3].value.intVal)
+ self.assertEqual(msg.meta[3].key, 'my-meta-key-3')
+ self.assertEqual(len(msg.meta[2].value.stringVal), 2)
+ self.assertIn('test', msg.meta[2].value.stringVal)
+ self.assertIn('test2', msg.meta[2].value.stringVal)
+ self.assertIn(-42, msg.meta[2].value.intVal)
b64EncodedQuery = base64.b64encode(query.to_wire()).decode('ascii')
self.assertEqual(tags['b64'], [b64EncodedQuery])
self.assertIn('my-tag-key2:my-tag-value2', msg.response.tags)
self.assertIn('my-empty-key', msg.response.tags)
# meta tags
- self.assertEqual(len(msg.meta), 3)
+ self.assertEqual(len(msg.meta), 5)
self.assertEqual(msg.meta[0].key, 'my-tag-export-name')
self.assertEqual(len(msg.meta[0].value.stringVal), 3)
self.assertIn('my-tag-key:my-tag-value', msg.meta[0].value.stringVal)
self.assertIn('test2', msg.meta[1].value.stringVal)
self.assertIn(-42, msg.meta[1].value.intVal)
- self.assertEqual(msg.meta[2].key, 'my-meta-key-2')
+ self.assertEqual(msg.meta[2].key, 'my-meta-key-3')
self.assertEqual(len(msg.meta[2].value.stringVal), 2)
- self.assertIn('foo', msg.meta[2].value.stringVal)
- self.assertIn('bar', msg.meta[2].value.stringVal)
- self.assertIn(42, msg.meta[2].value.intVal)
+ self.assertIn('test', msg.meta[2].value.stringVal)
+ self.assertIn('test2', msg.meta[2].value.stringVal)
+ self.assertIn(-42, msg.meta[2].value.intVal)
+
+ self.assertEqual(msg.meta[3].key, 'my-meta-key-2')
+ self.assertEqual(len(msg.meta[3].value.stringVal), 2)
+ self.assertIn('foo', msg.meta[3].value.stringVal)
+ self.assertIn('bar', msg.meta[3].value.stringVal)
+ self.assertIn(42, msg.meta[3].value.intVal)
+
+ self.assertEqual(msg.meta[4].key, 'my-meta-key-4')
+ self.assertEqual(len(msg.meta[4].value.stringVal), 2)
+ self.assertIn('foo', msg.meta[4].value.stringVal)
+ self.assertIn('bar', msg.meta[4].value.stringVal)
+ self.assertIn(42, msg.meta[4].value.intVal)
class TestProtobufExtendedDNSErrorTags(DNSDistProtobufTest):
_config_params = ['_testServerPort', '_protobufServerPort']