[sr-dev] Minor bug in modules/auth/auth_mod.c

Jorj Bauer jorj at isc.upenn.edu
Mon Apr 8 15:41:34 CEST 2013


[Apologies if this made it to the list last week; I got trapped in the return-email-address-doesnt-match moderation penalty box, and can't tell whether or not this was posted.]

There's a mismatch in the logic between the way that authentication headers are parsed (and stuffed in ->authorized) and the logic in consume_credentials.

In pre_auth (api.c), any ACK, CANCEL or PRACK bypasses the header parsing and setting of the ->authorized struct member:

        if (msg->REQ_METHOD & (METHOD_ACK|METHOD_CANCEL|METHOD_PRACK)) 
               return AUTHENTICATED;

but then in consume_credentials (auth_mod.c), an error is emitted if it's a PRACK:

[auth_mod.c:370]: auth:consume_credentials: No authorized credentials found (error in scripts) 

which is because the error is explicitly suppressed for ACK or CANCEL, but not PRACK:

               if (msg->REQ_METHOD != METHOD_ACK
                      && msg->REQ_METHOD != METHOD_CANCEL) {
                  LOG(...)

auth_mod should be amended.

Also, since ACK, CANCEL and PRACK can never have these headers, the function should be probably skip calling get_authorized_cred multiple times for those types of messages:



int consume_credentials(struct sip_msg* msg)
{
   struct hdr_field* h;
   int len;

   // Credentials aren't parsed for ACK/CANCEL/PRACK, so ignore those types
   // and return that no processing has occurred.
   if (msg->REQ_METHOD & (METHOD_ACK|METHOD_CANCEL|METHOD_PRACK))
       return -1;

   get_authorized_cred(msg->authorization, &h);
   if (!h) {
               get_authorized_cred(msg->proxy_auth, &h);
               if (!h) {
                       LOG(L_ERR, "auth:consume_credentials: No authorized "
                               "credentials found (error in scripts)\n");
                       }
                       return -1;
               }
   }

   len = h->len;

   if (del_lump(msg, h->name.s - msg->buf, len, 0) == 0) {
               LOG(L_ERR, "auth:consume_credentials: Can't remove credentials\n");
               return -1;
   }

   return 1;
}

Hope that's useful to someone. It would remove tens of thousands of error messages in our daily logs and avoid the hack we're staging in our proxy configuration, specifically for PRACKs, just to get rid of those messages. :)

-- Jorj




More information about the sr-dev mailing list