Skip to contentSkip to conversation

: Add (missing) option parsing to request_sock processing

PATCH In review Gerrit Renker 2 participants 2 messages Sep 25, 2007 Networking

lore
Share & tools
lore
#0 lore raw

PATCH 6/8 : Add (missing) option parsing to request_sock processing

[DCCP]: Add (missing) option parsing to request_sock processing

This adds option-parsing code to processing of Acks in the listening state on request_socks on the server, serving two purposes
(i) resolves a FIXME (removed);
(ii) paves the way for feature-negotiation during connection-setup.

There is an intended subtlety here with regard to dccp_check_req:

Parsing options happens only after testing whether the received packet is a retransmitted Request. Otherwise, if the Request contained (a possibly large number of) feature-negotiation options, recomputing state would have to happen each time a retransmitted Request arrives, which opens the door to an easy DoS attack. Since in a genuine retransmission the options should not be different from the original, reusing the already computed state seems better.

The other point is - if there are timestamp options on the Request, they will not be answered; which means that in the presence of retransmission (likely due to loss and/or other problems), the use of Request/Response RTT sampling is suspended, so that startup problems here do not propagate.

---
 net/dccp/minisocks.c |   12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)
net/dccp/minisocks.c +6 -6
@@ -200,10 +200,10 @@ struct sock *dccp_check_req(struct sock 
 			    struct request_sock **prev)
 {
 	struct sock *child = NULL;
+	struct dccp_request_sock *dreq = dccp_rsk(req);
 
 	/* Check for retransmitted REQUEST */
 	if (dccp_hdr(skb)->dccph_type = DCCP_PKT_REQUEST) {
-		struct dccp_request_sock *dreq = dccp_rsk(req);
 
 		if (after48(DCCP_SKB_CB(skb)->dccpd_seq, dreq->dreq_isr)) {
 			dccp_pr_debug("Retransmitted REQUEST\n");
@@ -227,22 +227,22 @@ struct sock *dccp_check_req(struct sock 
 		goto drop;
 
 	/* Invalid ACK */
-	if (DCCP_SKB_CB(skb)->dccpd_ack_seq != dccp_rsk(req)->dreq_iss) {
+	if (DCCP_SKB_CB(skb)->dccpd_ack_seq != dreq->dreq_iss) {
 		dccp_pr_debug("Invalid ACK number: ack_seq=%llu, "
 			      "dreq_iss=%llu\n",
 			      (unsigned long long)
 			      DCCP_SKB_CB(skb)->dccpd_ack_seq,
-			      (unsigned long long)
-			      dccp_rsk(req)->dreq_iss);
+			      (unsigned long long) dreq->dreq_iss);
 		goto drop;
 	}
 
+	if (dccp_parse_options(sk, dreq, skb))
+		 goto drop;
+
 	child = inet_csk(sk)->icsk_af_ops->syn_recv_sock(sk, skb, req, NULL);
 	if (child = NULL)
 		goto listen_overflow;
 
-	/* FIXME: deal with options */
-
 	inet_csk_reqsk_queue_unlink(sk, req, prev);
 	inet_csk_reqsk_queue_removed(sk, req);
 	inet_csk_reqsk_queue_add(sk, req, child);
#1 lore raw
Quoted 16 lines · Gerrit Renker

[DCCP]: Add (missing) option parsing to request_sock processing

This adds option-parsing code to processing of Acks in the listening state on request_socks on the server, serving two purposes
(i) resolves a FIXME (removed);
(ii) paves the way for feature-negotiation during connection-setup.

There is an intended subtlety here with regard to dccp_check_req:

Parsing options happens only after testing whether the received packet is a retransmitted Request. Otherwise, if the Request contained (a possibly large number of) feature-negotiation options, recomputing state would have to happen each time a retransmitted Request arrives, which opens the door to an easy DoS attack. Since in a genuine retransmission the options should not be different from the original, reusing the already computed state seems better.

Quoted 6 lines

The other point is - if there are timestamp options on the Request, they will not be answered; which means that in the presence of retransmission (likely due to loss and/or other problems), the use of Request/Response RTT sampling is suspended, so that startup problems here do not propagate.

Signature · 3 lines
Web1: http://wand.net.nz/~iam4/
Web2: http://www.jandi.co.nz
Blog: http://iansblog.jandi.co.nz