diff --git a/src/net/server/middleware/processors/edns.rs b/src/net/server/middleware/processors/edns.rs index 714e43303..7a51e2182 100644 --- a/src/net/server/middleware/processors/edns.rs +++ b/src/net/server/middleware/processors/edns.rs @@ -2,7 +2,7 @@ use core::ops::ControlFlow; use octseq::Octets; -use tracing::{debug, enabled, trace, warn, Level}; +use tracing::{debug, enabled, error, trace, warn, Level}; use crate::base::iana::OptRcode; use crate::base::message_builder::AdditionalBuilder; @@ -13,8 +13,8 @@ use crate::base::StreamTarget; use crate::net::server::message::{Request, TransportSpecificContext}; use crate::net::server::middleware::processor::MiddlewareProcessor; use crate::net::server::middleware::processors::mandatory::MINIMUM_RESPONSE_BYTE_LEN; -use crate::net::server::util::add_edns_options; use crate::net::server::util::start_reply; +use crate::net::server::util::{add_edns_options, remove_edns_opt_record}; /// EDNS version 0. /// @@ -52,6 +52,7 @@ impl EdnsMiddlewareProcessor { impl EdnsMiddlewareProcessor { /// Create a DNS error response to the given request with the given RCODE. fn error_response( + &self, request: &Request, rcode: OptRcode, ) -> AdditionalBuilder> @@ -72,6 +73,7 @@ impl EdnsMiddlewareProcessor { ); } + self.postprocess(request, &mut additional); additional } } @@ -100,10 +102,9 @@ where if iter.next().is_some() { // More than one OPT RR received. debug!("RFC 6891 6.1.1 violation: request contains more than one OPT RR."); - return ControlFlow::Break(Self::error_response( - request, - OptRcode::FormErr, - )); + return ControlFlow::Break( + self.error_response(request, OptRcode::FormErr), + ); } if let Ok(opt) = opt { @@ -116,10 +117,9 @@ where // RCODE=BADVERS." if opt_rec.version() > EDNS_VERSION_ZERO { debug!("RFC 6891 6.1.3 violation: request EDNS version {} > 0", opt_rec.version()); - return ControlFlow::Break(Self::error_response( - request, - OptRcode::BadVers, - )); + return ControlFlow::Break( + self.error_response(request, OptRcode::BadVers), + ); } match request.transport_ctx() { @@ -137,7 +137,7 @@ where if opt_rec.opt().tcp_keepalive().is_some() { debug!("RFC 7828 3.2.1 violation: edns-tcp-keepalive option received via UDP"); return ControlFlow::Break( - Self::error_response( + self.error_response( request, OptRcode::FormErr, ), @@ -215,7 +215,7 @@ where if keep_alive.timeout().is_some() { debug!("RFC 7828 3.2.1 violation: edns-tcp-keepalive option received via TCP contains timeout"); return ControlFlow::Break( - Self::error_response( + self.error_response( request, OptRcode::FormErr, ), @@ -236,6 +236,35 @@ where request: &Request, response: &mut AdditionalBuilder>, ) { + // https://www.rfc-editor.org/rfc/rfc6891.html#section-6.1.1 + // 6.1.1: Basic Elements + // ... + // "If an OPT record is present in a received request, compliant + // responders MUST include an OPT record in their respective + // responses." + // + // We don't do anything about this scenario at present. + + // https://www.rfc-editor.org/rfc/rfc6891.html#section-7 + // 7: Transport considerations + // ... + // "Lack of presence of an OPT record in a request MUST be taken as an + // indication that the requestor does not implement any part of this + // specification and that the responder MUST NOT include an OPT + // record in its response." + // + // So strip off any OPT record present if the query lacked an OPT + // record. + if request.message().opt().is_none() { + if let Err(err) = remove_edns_opt_record(response) { + error!( + "Error while stripping OPT record from response: {err}" + ); + *response = self.error_response(request, OptRcode::ServFail); + return; + } + } + // https://datatracker.ietf.org/doc/html/rfc7828#section-3.3.2 // 3.3.2. Sending Responses // "A DNS server that receives a query sent using TCP transport that @@ -280,29 +309,6 @@ where } } } - - // https://www.rfc-editor.org/rfc/rfc6891.html#section-6.1.1 - // 6.1.1: Basic Elements - // ... - // "If an OPT record is present in a received request, compliant - // responders MUST include an OPT record in their respective - // responses." - // - // We don't do anything about this scenario at present. - - // https://www.rfc-editor.org/rfc/rfc6891.html#section-7 - // 7: Transport considerations - // ... - // "Lack of presence of an OPT record in a request MUST be taken as an - // indication that the requestor does not implement any part of this - // specification and that the responder MUST NOT include an OPT - // record in its response." - // - // So strip off any OPT record present if the query lacked an OPT - // record. - - // TODO: How can we strip off the OPT record in the response if no OPT - // record is present in the request? } } diff --git a/src/net/server/util.rs b/src/net/server/util.rs index 6957e615d..9494e5721 100644 --- a/src/net/server/util.rs +++ b/src/net/server/util.rs @@ -224,7 +224,9 @@ where // delay response building until the complete set of differences to a base // response are known. Or a completely different builder approach that can // edit a partially built message. - if response.counts().arcount() > 0 { + if response.counts().arcount() > 0 + && response.as_message().opt().is_some() + { // Make a copy of the response. let copied_response = response.as_slice().to_vec(); let Ok(copied_response) = Message::from_octets(&copied_response) @@ -271,3 +273,48 @@ where // No existing OPT record in the additional section so build a new one. response.opt(op) } + +/// Removes any OPT records present in the response. +pub fn remove_edns_opt_record( + response: &mut AdditionalBuilder>, +) -> Result<(), PushError> +where + Target: Composer, +{ + // TODO: This function has the same less than ideal properties as the + // add_edns_options() function above that it is similar to, ideally we can + // avoid the need to copy the response. + if response.counts().arcount() > 0 + && response.as_message().opt().is_some() + { + // Make a copy of the response. + let copied_response = response.as_slice().to_vec(); + let Ok(copied_response) = Message::from_octets(&copied_response) + else { + warn!("Internal error: Unable to create message from octets while adding EDNS option"); + return Ok(()); + }; + + if copied_response.opt().is_some() { + // Discard the current records in the additional section of the + // response. + response.rewind(); + + // Copy the non-OPT records from the copied response to the + // current response. + if let Ok(current_additional) = copied_response.additional() { + for rr in current_additional.flatten() { + if rr.rtype() != Rtype::Opt { + if let Ok(Some(rr)) = rr + .into_record::>>() + { + response.push(rr)?; + } + } + } + } + } + } + + Ok(()) +}