Make mDNS server response more compatible with RFC 6762 spec.

- dns.Msg construction is now explicit, with comments pointing to the RFC about
  the values of each field.
- dns.Msg responses no longer contain a Question section, which is not allowed
  according to the spec.
- handleQuestion's interface returns both unicast and multicast answer records.
  (Note: These are not yet treated differently by the sendResponse method.)
- removed unused error return value of handleQuestion

server.go

@@ -2,16 +2,19 @@ package mdns
 
 import (
 	"fmt"
-	"github.com/miekg/dns"
 	"log"
 	"net"
+	"strings"
 	"sync"
+
+	"github.com/miekg/dns"
 )
 
 const (
 	ipv4mdns              = "224.0.0.251"
 	ipv6mdns              = "ff02::fb"
 	mdnsPort              = 5353
+	forceUnicastResponses = false
 )
 
 var (
@@ -127,41 +130,145 @@ func (s *Server) parsePacket(packet []byte, from net.Addr) error {
 
 // handleQuery is used to handle an incoming query
 func (s *Server) handleQuery(query *dns.Msg, from net.Addr) error {
-	var resp dns.Msg
+	if query.Opcode != dns.OpcodeQuery {
-	resp.SetReply(query)
+		// "In both multicast query and multicast response messages, the OPCODE MUST
+		// be zero on transmission (only standard queries are currently supported
+		// over multicast).  Multicast DNS messages received with an OPCODE other
+		// than zero MUST be silently ignored."  Note: OpcodeQuery == 0
+		return fmt.Errorf("mdns: received query with non-zero Opcode %v: %v", query.Opcode, *query)
+	}
+	if query.Rcode != 0 {
+		// "In both multicast query and multicast response messages, the Response
+		// Code MUST be zero on transmission.  Multicast DNS messages received with
+		// non-zero Response Codes MUST be silently ignored."
+		return fmt.Errorf("mdns: received query with non-zero Rcode %v: %v", query.Rcode, *query)
+	}
+
+	// TODO(reddaly): Handle "TC (Truncated) Bit":
+	//    In query messages, if the TC bit is set, it means that additional
+	//    Known-Answer records may be following shortly.  A responder SHOULD
+	//    record this fact, and wait for those additional Known-Answer records,
+	//    before deciding whether to respond.  If the TC bit is clear, it means
+	//    that the querying host has no additional Known Answers.
+	if query.Truncated {
+		return fmt.Errorf("[ERR] mdns: support for DNS requests with high truncated bit not implemented: %v", *query)
+	}
+
+	var unicastAnswer, multicastAnswer []dns.RR
 
 	// Handle each question
-	if len(query.Question) > 0 {
+	for _, q := range query.Question {
-		for i, _ := range query.Question {
+		mrecs, urecs := s.handleQuestion(q)
-			if err := s.handleQuestion(query.Question[i], &resp); err != nil {
+		multicastAnswer = append(multicastAnswer, mrecs...)
-				log.Printf("[ERR] mdns: failed to handle question %v: %v",
+		unicastAnswer = append(unicastAnswer, urecs...)
-					query.Question[i], err)
 	}
+
+	// See section 18 of RFC 6762 for rules about DNS headers.
+	resp := func(unicast bool) *dns.Msg {
+		// 18.1: ID (Query Identifier)
+		// 0 for multicast response, query.Id for unicast response
+		id := uint16(0)
+		if unicast {
+			id = query.Id
+		}
+
+		var answer []dns.RR
+		if unicast {
+			answer = unicastAnswer
+		} else {
+			answer = multicastAnswer
+		}
+		if len(answer) == 0 {
+			return nil
+		}
+
+		return &dns.Msg{
+			MsgHdr: dns.MsgHdr{
+				Id: id,
+
+				// 18.2: QR (Query/Response) Bit - must be set to 1 in response.
+				Response: true,
+
+				// 18.3: OPCODE - must be zero in response (OpcodeQuery == 0)
+				Opcode: dns.OpcodeQuery,
+
+				// 18.4: AA (Authoritative Answer) Bit - must be set to 1
+				Authoritative: true,
+
+				// The following fields must all be set to 0:
+				// 18.5: TC (TRUNCATED) Bit
+				// 18.6: RD (Recursion Desired) Bit
+				// 18.7: RA (Recursion Available) Bit
+				// 18.8: Z (Zero) Bit
+				// 18.9: AD (Authentic Data) Bit
+				// 18.10: CD (Checking Disabled) Bit
+				// 18.11: RCODE (Response Code)
+			},
+			// 18.12 pertains to questions (handled by handleQuestion)
+			// 18.13 pertains to resource records (handled by handleQuestion)
+
+			// 18.14: Name Compression - responses should be compressed (though see
+			// caveats in the RFC), so set the Compress bit (part of the dns library
+			// API, not part of the DNS packet) to true.
+			Compress: true,
+
+			Answer: answer,
 		}
 	}
 
-	// Check if there is an answer
+	if len(multicastAnswer) == 0 && len(unicastAnswer) == 0 {
-	if len(resp.Answer) > 0 {
+		questions := make([]string, len(query.Question))
-		return s.sendResponse(&resp, from)
+		for i, q := range query.Question {
+			questions[i] = q.Name
+		}
+		log.Printf("no responses for query with questions: %s", strings.Join(questions, ", "))
+	}
+
+	if mresp := resp(false); mresp != nil {
+		if err := s.sendResponse(mresp, from, false); err != nil {
+			return fmt.Errorf("mdns: error sending multicast response: %v", err)
+		}
+	}
+	if uresp := resp(true); uresp != nil {
+		if err := s.sendResponse(uresp, from, true); err != nil {
+			return fmt.Errorf("mdns: error sending unicast response: %v", err)
+		}
 	}
 	return nil
 }
 
 // handleQuestion is used to handle an incoming question
-func (s *Server) handleQuestion(q dns.Question, resp *dns.Msg) error {
+//
-	// Bail if we have no zone
+// The response to a question may be transmitted over multicast, unicast, or
-	if s.config.Zone == nil {
+// both.  The return values are DNS records for each transmission type.
-		return nil
+func (s *Server) handleQuestion(q dns.Question) (multicastRecs, unicastRecs []dns.RR) {
+	records := s.config.Zone.Records(q)
+
+	if len(records) == 0 {
+		return nil, nil
 	}
 
-	// Add all the query answers
+	// Handle unicast and multicast responses.
-	records := s.config.Zone.Records(q)
+	// TODO(reddaly): The decision about sending over unicast vs. multicast is not
-	resp.Answer = append(resp.Answer, records...)
+	// yet fully compliant with RFC 6762.  For example, the unicast bit should be
-	return nil
+	// ignored if the records in question are close to TTL expiration.  For now,
+	// we just use the unicast bit to make the decision, as per the spec:
+	//     RFC 6762, section 18.12.  Repurposing of Top Bit of qclass in Question
+	//     Section
+	//
+	//     In the Question Section of a Multicast DNS query, the top bit of the
+	//     qclass field is used to indicate that unicast responses are preferred
+	//     for this particular question.  (See Section 5.4.)
+	if q.Qclass&(1<<15) != 0 || forceUnicastResponses {
+		return nil, records
+	}
+	return records, nil
 }
 
 // sendResponse is used to send a response packet
-func (s *Server) sendResponse(resp *dns.Msg, from net.Addr) error {
+func (s *Server) sendResponse(resp *dns.Msg, from net.Addr, unicast bool) error {
+	// TODO(reddaly): Respect the unicast argument, and allow sending responses
+	// over multicast.
 	buf, err := resp.Pack()
 	if err != nil {
 		return err