PQ: pad resumed UDP queries to the regular question size target

Also discard oversized tickets and fall back to TCP when a query
cannot fit a datagram.

Author: jedisct1

dnscrypt-proxy/crypto.go

@@ -146,7 +146,11 @@ func (proxy *Proxy) Encrypt(
 	proto string,
 ) (sharedKey *[32]byte, encrypted []byte, clientNonce []byte, queryEpoch uint64, err error) {
 	if serverInfo.CryptoConstruction == XWingPQ {
-		return proxy.encryptPQ(serverInfo, packet, proto)
+		sharedKey, encrypted, clientNonce, queryEpoch, err = proxy.encryptPQ(serverInfo, packet, proto)
+		if err == nil && proto == "udp" && len(encrypted) > pqUDPPacketLimit(serverInfo) {
+			err = errors.New("Question too large; cannot be padded")
+		}
+		return sharedKey, encrypted, clientNonce, queryEpoch, err
 	}
 	nonce, clientNonce := make([]byte, NonceSize), make([]byte, HalfNonceSize)
 	if _, err := crypto_rand.Read(clientNonce); err != nil {

dnscrypt-proxy/pq.go

@@ -25,6 +25,7 @@ const (
 	PQExtVersion          = 0x01
 	PQKdfID               = 0x01
 	PQAeadID              = 0x01
+	PQResumedPaddingFloor = 256
 )
 
 var (
@@ -113,16 +114,13 @@ func pqResumedSharedKey(resumeSecret [32]byte, clientMagic [8]byte, clientNonce,
 // pqPad applies ISO/IEC 7816-4 padding to the next multiple of 64, with a
 // minimum floor (itself a multiple of 64).
 func pqPad(packet []byte, floor int) []byte {
-	padded := make([]byte, len(packet), len(packet)+64)
-	copy(padded, packet)
-	padded = append(padded, 0x80)
-	target := (len(padded) + 63) &^ 63
+	target := (len(packet) + 1 + 63) &^ 63
 	if target < floor {
 		target = floor
 	}
-	for len(padded) < target {
-		padded = append(padded, 0)
-	}
+	padded := make([]byte, target)
+	copy(padded, packet)
+	padded[len(packet)] = 0x80
 	return padded
 }
 
@@ -208,13 +206,41 @@ func (s *pqSessionState) storeEncapsulation(ct []byte, key [32]byte, epoch uint6
 	s.encapEpoch = epoch
 }
 
+// pqResumedOverhead is the wire size of a resumed query around its encrypted
+// padded payload.
+func pqResumedOverhead(ticketLen int) int {
+	return len(PQResumeMagic) + 2 + ticketLen + HalfNonceSize + TagSize
+}
+
+// pqUDPPacketLimit is the largest UDP datagram worth sending to this server,
+// honoring the same fragmentation workaround as the classic path.
+func pqUDPPacketLimit(serverInfo *ServerInfo) int {
+	if serverInfo.knownBugs.fragmentsBlocked {
+		return MaxDNSUDPSafePacketSize
+	}
+	return MaxDNSUDPPacketSize
+}
+
+// pqResumedPaddingFloor returns the padding floor for a resumed query. Over
+// UDP, the encrypted response can never be larger than the query, so the
+// query is grown to the regular UDP question size target, leaving the
+// resolver the same response budget as a classic query.
+func (proxy *Proxy) pqResumedPaddingFloor(serverInfo *ServerInfo, ticketLen int, proto string) int {
+	if proto != "udp" {
+		return PQResumedPaddingFloor
+	}
+	overhead := pqResumedOverhead(ticketLen)
+	want := proxy.questionSizeEstimator.MinQuestionSize() - overhead
+	return Max(PQResumedPaddingFloor, Min((want+63)&^63, (pqUDPPacketLimit(serverInfo)-overhead)&^63))
+}
+
 // encryptPQ builds a PQ query: a resumed query when a valid ticket is held,
 // otherwise a query that carries an X-Wing ciphertext, reusing the cached
 // encapsulation when one is available for the current network epoch.
 func (proxy *Proxy) encryptPQ(
 	serverInfo *ServerInfo,
 	packet []byte,
-	_ string,
+	proto string,
 ) (sharedKey *[32]byte, encrypted []byte, clientNonce []byte, queryEpoch uint64, err error) {
 	queryEpoch = proxy.networkEpoch()
 	nonce := make([]byte, NonceSize)
@@ -226,7 +252,7 @@ func (proxy *Proxy) encryptPQ(
 
 	if ticket, resumeSecret, ok := serverInfo.pqSession.get(queryEpoch); ok {
 		key := pqResumedSharedKey(resumeSecret, serverInfo.MagicQuery, clientNonce, ticket)
-		padded := pqPad(packet, 256)
+		padded := pqPad(packet, proxy.pqResumedPaddingFloor(serverInfo, len(ticket), proto))
 		ct := xsecretbox.Seal(nil, nonce, padded, key[:])
 		out := make([]byte, 0, len(PQResumeMagic)+2+len(ticket)+HalfNonceSize+len(ct))
 		out = append(out, PQResumeMagic[:]...)
@@ -299,6 +325,10 @@ func (proxy *Proxy) pqProcessControl(
 		return
 	}
 	ticket := control[11 : 11+ticketLen]
+	if pqResumedOverhead(len(ticket))+PQResumedPaddingFloor > MaxDNSUDPPacketSize {
+		dlog.Debugf("[%v] discarded an oversized PQ resumption ticket (%d bytes)", serverInfo.Name, len(ticket))
+		return
+	}
 	if queryEpoch != proxy.networkEpoch() {
 		dlog.Debugf("[%v] discarded a PQ resumption ticket after a network change", serverInfo.Name)
 		return

dnscrypt-proxy/pq_test.go

@@ -28,6 +28,33 @@ func mustHex(s string) []byte {
 	return b
 }
 
+func newTestPQProxy(epoch uint64) *Proxy {
+	monitor := newNetworkMonitor()
+	monitor.epochValue.Store(epoch)
+	proxy := &Proxy{netMonitor: monitor}
+	proxy.questionSizeEstimator = NewQuestionSizeEstimator()
+	return proxy
+}
+
+func newTestPQServerInfo() *ServerInfo {
+	_, pk := xwing.DeriveKeyPair(iotaBytes(32, 0x20))
+	pkb, _ := pk.MarshalBinary()
+	return &ServerInfo{
+		Name:               "test",
+		CryptoConstruction: XWingPQ,
+		PqPublicKey:        pkb,
+		PqCertContext:      iotaBytes(48, 0x01),
+		MagicQuery:         [8]byte{1, 2, 3, 4, 5, 6, 7, 8},
+		pqSession:          newPqSessionState(XWingPQ),
+	}
+}
+
+// maxStorableTicketLen is the largest resumption ticket pqProcessControl
+// accepts; one byte more and a resumed query cannot fit a UDP datagram.
+func maxStorableTicketLen() int {
+	return MaxDNSUDPPacketSize - PQResumedPaddingFloor - pqResumedOverhead(0)
+}
+
 // TestPQAppendix3Vectors checks the client's PQ crypto against the pinned
 // values of Appendix 3 of the draft, the same anchor the server validates.
 func TestPQAppendix3Vectors(t *testing.T) {
@@ -147,20 +174,8 @@ func TestPQSessionResumptionEpoch(t *testing.T) {
 // single X-Wing key exchange instead of running a fresh one every time, and
 // that a network change forces a new one.
 func TestEncryptPQReusesEncapsulation(t *testing.T) {
-	monitor := newNetworkMonitor()
-	monitor.epochValue.Store(1)
-	proxy := &Proxy{netMonitor: monitor}
-
-	_, pk := xwing.DeriveKeyPair(iotaBytes(32, 0x20))
-	pkb, _ := pk.MarshalBinary()
-	serverInfo := &ServerInfo{
-		Name:               "test",
-		CryptoConstruction: XWingPQ,
-		PqPublicKey:        pkb,
-		PqCertContext:      iotaBytes(48, 0x01),
-		MagicQuery:         [8]byte{1, 2, 3, 4, 5, 6, 7, 8},
-		pqSession:          newPqSessionState(XWingPQ),
-	}
+	proxy := newTestPQProxy(1)
+	serverInfo := newTestPQServerInfo()
 	query := mustHex("12340100000100000000000003777777076578616d706c6503636f6d0000010001")
 
 	ctOf := func(out []byte) []byte {
@@ -182,7 +197,7 @@ func TestEncryptPQReusesEncapsulation(t *testing.T) {
 		t.Fatal("expected the shared key to be reused across queries")
 	}
 
-	monitor.epochValue.Store(2)
+	proxy.netMonitor.epochValue.Store(2)
 	_, out3, _, _, err := proxy.encryptPQ(serverInfo, query, "udp")
 	if err != nil {
 		t.Fatal(err)
@@ -192,14 +207,54 @@ func TestEncryptPQReusesEncapsulation(t *testing.T) {
 	}
 }
 
-func TestPQProcessControlDiscardsTicketAfterNetworkChange(t *testing.T) {
-	monitor := newNetworkMonitor()
-	monitor.epochValue.Store(2)
-	proxy := &Proxy{netMonitor: monitor}
-	serverInfo := &ServerInfo{
-		Name:      "test",
-		pqSession: newPqSessionState(XWingPQ),
+// TestPQResumedQueryMeetsUDPSizeTarget checks that a resumed query sent over
+// UDP is padded up to the regular question size target, since the resolver
+// cannot send a UDP response larger than the query. Servers that cannot
+// receive fragmented datagrams keep the classic reduced cap, and over TCP
+// the smaller fixed floor is kept.
+func TestPQResumedQueryMeetsUDPSizeTarget(t *testing.T) {
+	proxy := newTestPQProxy(1)
+	serverInfo := newTestPQServerInfo()
+	ticket := iotaBytes(130, 0x24)
+	serverInfo.pqSession.store(ticket, [32]byte{42}, time.Now().Add(time.Minute), 1)
+	query := mustHex("12340100000100000000000003777777076578616d706c6503636f6d0000010001")
+	overhead := pqResumedOverhead(len(ticket))
+
+	_, udpOut, _, _, err := proxy.encryptPQ(serverInfo, query, "udp")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(udpOut) < InitialMinQuestionSize {
+		t.Fatalf("resumed UDP query is only %d bytes, below the %d response budget", len(udpOut), InitialMinQuestionSize)
+	}
+	if plaintextLen := len(udpOut) - overhead; plaintextLen%64 != 0 {
+		t.Fatalf("resumed UDP query padding is not a multiple of 64: %d", plaintextLen)
+	}
+
+	_, tcpOut, _, _, err := proxy.encryptPQ(serverInfo, query, "tcp")
+	if err != nil {
+		t.Fatal(err)
 	}
+	if expected := overhead + PQResumedPaddingFloor; len(tcpOut) != expected {
+		t.Fatalf("resumed TCP query length %d, expected %d", len(tcpOut), expected)
+	}
+
+	serverInfo.knownBugs.fragmentsBlocked = true
+	for i := 0; i < 3; i++ {
+		proxy.questionSizeEstimator.blindAdjust()
+	}
+	_, safeOut, _, _, err := proxy.encryptPQ(serverInfo, query, "udp")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(safeOut) > MaxDNSUDPSafePacketSize {
+		t.Fatalf("resumed UDP query is %d bytes despite the fragmentation workaround", len(safeOut))
+	}
+}
+
+func TestPQProcessControlDiscardsTicketAfterNetworkChange(t *testing.T) {
+	proxy := newTestPQProxy(2)
+	serverInfo := newTestPQServerInfo()
 	sharedKey := &[32]byte{1, 2, 3}
 	clientNonce := []byte("abcdefghijkl")
 	control := buildTestPQControl([]byte("ticket"), 60)
@@ -215,6 +270,41 @@ func TestPQProcessControlDiscardsTicketAfterNetworkChange(t *testing.T) {
 	}
 }
 
+func TestPQQueryTooLargeForUDPFallsBackToTCP(t *testing.T) {
+	proxy := newTestPQProxy(1)
+	serverInfo := newTestPQServerInfo()
+
+	hugeQuery := iotaBytes(3000, 0x02)
+	if _, _, _, _, err := proxy.Encrypt(serverInfo, hugeQuery, "udp"); err == nil {
+		t.Fatal("expected an error for a full query that cannot fit in a UDP datagram")
+	}
+	if _, _, _, _, err := proxy.Encrypt(serverInfo, hugeQuery, "tcp"); err != nil {
+		t.Fatal(err)
+	}
+
+	ticket := iotaBytes(maxStorableTicketLen(), 0x01)
+	serverInfo.pqSession.store(ticket, [32]byte{42}, time.Now().Add(time.Minute), 1)
+	query := iotaBytes(300, 0x02)
+	if _, _, _, _, err := proxy.Encrypt(serverInfo, query, "udp"); err == nil {
+		t.Fatal("expected an error for a resumed query that cannot fit in a UDP datagram")
+	}
+	if _, _, _, _, err := proxy.Encrypt(serverInfo, query, "tcp"); err != nil {
+		t.Fatal(err)
+	}
+}
+
+func TestPQProcessControlDiscardsOversizedTicket(t *testing.T) {
+	proxy := newTestPQProxy(1)
+	serverInfo := newTestPQServerInfo()
+	oversized := iotaBytes(maxStorableTicketLen()+1, 0x01)
+	control := buildTestPQControl(oversized, 60)
+
+	proxy.pqProcessControl(serverInfo, &[32]byte{1}, []byte("abcdefghijkl"), control, 1)
+	if _, _, ok := serverInfo.pqSession.get(1); ok {
+		t.Fatal("expected an oversized ticket to be discarded")
+	}
+}
+
 func buildTestPQControl(ticket []byte, lifetime uint32) []byte {
 	control := append([]byte{}, PQControlMagic[:]...)
 	control = append(control, PQControlVersion)