[Home]

Summary:ASTERISK-18321: dynamic_exclude_static option with (temporary) unreachable DNS cause the abend
Reporter:Dan Lukes (dan_lukes)Labels:
Date Opened:2011-08-22 19:07:29Date Closed:2012-04-27 16:51:19
Priority:MajorRegression?
Status:Closed/CompleteComponents:Channels/chan_sip/General
Versions:1.8.5.0 Frequency of
Occurrence		Constant
Related
Issues:
is related toASTERISK-16435 [patch] dynamic_exclude_static option results in ACL errors
Environment:Attachments:( 0) ASTERISK-18321.patch
( 1) patch-DAN-channels-chan_sip.c
Description:Change introduced in ASTERISK-16435 doesn't solve the problems related to ACL creation in full. The same issues remain unresolved even after it when DNS resolution not available (temporary loss of connectivity do DNS server, for example).

chan_sip.c, function build_peer(), current fragment of code:

{code}
if (ast_dnsmgr_lookup(_srvlookup, &peer->addr, &peer->dnsmgr, sip_cfg.srvlookup && !peer->portinuri ? transport : NULL)) {
   ast_log(LOG_ERROR, "srvlookup failed for host: %s, on peer %s, removing peer\n", _srvlookup, peer->name);
   unref_peer(peer, "getting rid of a peer pointer");
   return NULL;
}

ast_string_field_set(peer, tohost, srvlookup);

if (global_dynamic_exclude_static) {
   int err = 0;
   sip_cfg.contact_ha = ast_append_ha("deny", ast_sockaddr_stringify_addr(&peer->addr),
       sip_cfg.contact_ha, &err);
   if (err) {
       ast_log(LOG_ERROR, "Bad ACL entry in configuration line %d : %s\n", v->lineno, v->value);
   }
}
{code}

When DNS server not avaiable, then lookup will fail. Such condition is not catched, so ast_append_ha is called with ast_sockaddr_stringify_addr(&peer->addr))==NULL and terminate with err==1

Finally, the {color:red}ast_log(LOG_ERROR, "Bad ACL entry in configuration line %d : %s\n", v->lineno, v->value);
{color} will cause abend of entire server because v == NULL here.

Messages on console:

{quote}
{color:blue}
[Aug 23 01:37:25] ERROR[68192]: netsock2.c:263 ast_sockaddr_resolve: getaddrinfo("a.sip.cz", "(null)", ...): hostname nor servname provided, or not known
[Aug 23 01:37:25] WARNING[68192]: acl.c:579 resolve_first: Unable to lookup 'a.sip.cz'
[Aug 23 01:37:25] WARNING[68192]: acl.c:422 ast_append_ha: Invalid IP address: (null)
{color}
{color:red}
ABEND of entire server HERE
{color}
{quote}

At the first, we need to correct broken {color:red}ast_log(LOG_ERROR,...);{color}. We need change it to something like:
{color:green}
ast_log(LOG_ERROR, "Can't add dynamic 'contactdeny' ACL for address %s\n", ast_sockaddr_stringify_addr(&peer->addr));
{color}

At the second, ast_append_ha should not be called when resolution not done. Unfortunatelly, I'm not so familiar with new netsock2 code, so I don't know the best way to recognize it.

We can change condition {color:red}if (global_dynamic_exclude_static) {color} to {color:green}if (global_dynamic_exclude_static && ast_sockaddr_stringify_addr(&peer->addr) != NULL) {color} but I assume there will be a better way.
Comments:By: Dan Lukes (dan_lukes) 2011-09-02 18:33:23.738-0500

Just side note - my patch has status "No License" not because I'm not willing to sign a license agreement but because my application has been rejected. The rejection has been anonymous (not signed by name of person nor department) sent from noreply@ email so I can't resolve the problem. If someone interested in such agreement, then ask me. I'm ready to resubmit it upon request.
By: Matt Jordan (mjordan) 2012-03-23 14:27:31.459-0500

Dan - I'm not sure what happened there.  Would you mind retrying?
By: Dan Lukes (dan_lukes) 2012-03-23 16:00:33.099-0500

Do you mean "license issue" ? I resubmitted it few minutes ago.
By: Matt Jordan (mjordan) 2012-03-27 13:23:51.020-0500

And it looks like it got accepted.  Thanks!
By: Mark Michelson (mmichelson) 2012-04-20 16:47:18.545-0500

So at some point between when this bug report was created and now, the log statement that caused the crash was altered. Rather than the change you proposed (printing the peer's address), the change was made to print the peer's name. To be specific, the line is now:

{code:language=C}
ast_log(LOG_ERROR, "Bad or unresolved host/IP entry in configuration for peer %s, cannot add to contact ACL\n", peer->name);
{code}

The other change in your patch was only to execute the block if {{ast_sockaddr_stringify_addr(&peer->addr)}} returns non-NULL. This is not a valid test because {{ast_sockaddr_stringify_addr()}} never returns NULL. It will either return an empty string, the proper address, or the literal string "(null)". The intent is good though. The proper way to check is to check {{ast_sockaddr_isnull(&peer->addr)}} instead. I will upload a new patch with this change.
By: Mark Michelson (mmichelson) 2012-04-20 16:52:27.452-0500

This patch was created against the tip of the 1.8 branch, so the log message has not been altered. However, the if statement has been updated to properly check for a null address. Let me know if this does the trick for you.

EDIT: Sorry, the patch is called ASTERISK-18321.patch
By: Dan Lukes (dan_lukes) 2012-04-24 10:34:45.961-0500

It looks good.