Re: [PATCH net v3 2/2] sctp: auth: verify auth requirement when auth_chunk is NULL

From: Qing Luo <redacted>

sctp_auth_chunk_verify() currently returns true unconditionally
when chunk->auth_chunk is NULL, which means authentication is
silently skipped. This is incorrect in two scenarios:

1. skb_clone() failed in the BH receive path, leaving auth_chunk
   NULL. Although the previous fix avoids setting auth=1 in this
   case, the chunk can still reach sctp_auth_chunk_verify() via
   sctp_endpoint_bh_rcv() where asoc is NULL for new connections,
   bypassing the early sctp_auth_recv_cid() check.

2. No AUTH chunk precedes COOKIE-ECHO in the packet. In this case
   skb_clone() is never called and auth_chunk remains NULL. Again,
   in sctp_endpoint_bh_rcv() the early check cannot catch this
   because asoc is NULL and sctp_auth_recv_cid() returns 0.

Fix by checking sctp_auth_recv_cid() when auth_chunk is NULL:
if authentication is required for this chunk type, return false
to drop the chunk; otherwise, continue normally.

Fixes: bbd0d59809f9 ("[SCTP]: Implement the receive and verification of AUTH chunk")
Signed-off-by: Qing Luo <redacted>
---
 net/sctp/sm_statefuns.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c
index d23d935e128e..89ed618b1de3 100644
--- a/net/sctp/sm_statefuns.c
+++ b/net/sctp/sm_statefuns.c

@@ -642,7 +642,7 @@ static bool sctp_auth_chunk_verify(struct net *net, struct sctp_chunk *chunk,
        struct sctp_chunk auth;

        if (!chunk->auth_chunk)
-               return true;
+               return !sctp_auth_recv_cid(chunk->chunk_hdr->type, asoc);

        /* SCTP-AUTH:  auth_chunk pointer is only set when the cookie-echo
         * is supposed to be authenticated and we have to do delayed
--

2.25.1

quoted

quoted

A better fix would be:

Add a check in sctp_auth_chunk_verify() at the point where the COOKIE-ECHO
chunk is actually being processed:


        if (!chunk->auth_chunk)
                return !sctp_auth_recv_cid(chunk->chunk_hdr->type, asoc);

This ensures that if chunk->auth_chunk is missing while authentication is
required for the COOKIE-ECHO chunk, the verification fails and the chunk is
dropped. Otherwise, when authentication is not required, processing can
continue normally.

Please give it a try.

Also, please add a extra Fixes tag in your next post:

Fixes: bbd0d59809f9 ("[SCTP]: Implement the receive and verification
of AUTH chunk")

which introduces chunk->auth_chunk and calls skb_clone() in
sctp_endpoint_bh_rcv().

Hi,

Thanks for the review. I’ve reworked the fix into two patches:

Patch 1/2: In sctp_assoc_bh_rcv() and sctp_endpoint_bh_rcv(), only set chunk->auth = 1 when skb_clone() succeeds.

Patch 2/2: In sctp_auth_chunk_verify(), when auth_chunk is NULL, check sctp_auth_recv_cid() to decide whether authentication is required. This covers both cases from the review.

I’d like to discuss whether Patch 1 is necessary. Patch 2 alone is sufficient for correctness — even with auth == 1 and auth_chunk == NULL, Patch 2 catches it at the verification point. Patch 1 only provides semantic cleanliness (not setting auth = 1 without a valid auth_chunk), but closes no additional gap.

Should I keep Patch 1 as a defensive cleanup, or drop it and submit only Patch 2?