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

From: Qing Luo <[email protected]>

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 <[email protected]>
---
 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
>> 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?

Best regards,