From 665a94cb7622dd5f9591df5d68962e41266ee47b Mon Sep 17 00:00:00 2001 From: Ben Ford Date: Mon, 21 Oct 2019 14:55:06 -0500 Subject: [PATCH] chan_sip.c: Prevent address change on unauthenticated SIP request. If the name of a peer is known and a SIP request is sent using that peer's name, the address of the peer will change even if the request fails the authentication challenge. This means that an endpoint can be altered and even rendered unusuable, even if it was in a working state previously. This can only occur when the nat option is set to the default, or auto_force_rport. This change checks the result of authentication first to ensure it is successful before setting the address and the nat option. ASTERISK-28589 #close Change-Id: I581c5ed1da60ca89f590bd70872de2b660de02df --- channels/chan_sip.c | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/channels/chan_sip.c b/channels/chan_sip.c index 80040aee42..72b285123e 100644 --- a/channels/chan_sip.c +++ b/channels/chan_sip.c @@ -19245,18 +19245,6 @@ static enum check_auth_result check_peer_ok(struct sip_pvt *p, char *of, bogus_peer = NULL; } - /* build_peer, called through sip_find_peer, is not able to check the - * sip_pvt->natdetected flag in order to determine if the peer is behind - * NAT or not when SIP_PAGE3_NAT_AUTO_RPORT or SIP_PAGE3_NAT_AUTO_COMEDIA - * are set on the peer. So we check for that here and set the peer's - * address accordingly. - */ - set_peer_nat(p, peer); - - if (p->natdetected && ast_test_flag(&peer->flags[2], SIP_PAGE3_NAT_AUTO_RPORT)) { - ast_sockaddr_copy(&peer->addr, &p->recv); - } - if (!ast_apply_acl(peer->acl, addr, "SIP Peer ACL: ")) { ast_debug(2, "Found peer '%s' for '%s', but fails host access\n", peer->name, of); sip_unref_peer(peer, "sip_unref_peer: check_peer_ok: from sip_find_peer call, early return of AUTH_ACL_FAILED"); @@ -19325,6 +19313,21 @@ static enum check_auth_result check_peer_ok(struct sip_pvt *p, char *of, ast_string_field_set(p, peermd5secret, NULL); } if (!(res = check_auth(p, req, peer->name, p->peersecret, p->peermd5secret, sipmethod, uri2, reliable))) { + + /* build_peer, called through sip_find_peer, is not able to check the + * sip_pvt->natdetected flag in order to determine if the peer is behind + * NAT or not when SIP_PAGE3_NAT_AUTO_RPORT or SIP_PAGE3_NAT_AUTO_COMEDIA + * are set on the peer. So we check for that here and set the peer's + * address accordingly. The address should ONLY be set once we are sure + * authentication was a success. If, for example, an INVITE was sent that + * matched the peer name but failed the authentication check, the address + * would be updated, which is bad. + */ + set_peer_nat(p, peer); + if (p->natdetected && ast_test_flag(&peer->flags[2], SIP_PAGE3_NAT_AUTO_RPORT)) { + ast_sockaddr_copy(&peer->addr, &p->recv); + } + /* If we have a call limit, set flag */ if (peer->call_limit) ast_set_flag(&p->flags[0], SIP_CALL_LIMIT); @@ -19424,6 +19427,7 @@ static enum check_auth_result check_peer_ok(struct sip_pvt *p, char *of, } } sip_unref_peer(peer, "check_peer_ok: sip_unref_peer: tossing temp ptr to peer from sip_find_peer"); + return res; }