Details

      Description

      So.. I was trying if I could alter the SIP security framework messages to differentiate between auth failures for any UDP packet and those with a valid nonce. Those with a valid nonce would probably not have a spoofed IP, so I can use fail2ban on them with more peace of mind.

      But, then I saw the different handling of the alwaysauthreject-challenge and the "normal" challenge code. These differences can be observed by an attacker sniffing for valid usernames.

      VICTIM$ sudo asterisk -nrx 'sip show peers' | head -n4
      Name/username...
      100...
      101...
      102...
      
      VICTIM$ sudo asterisk -nrx 'core show version'
      Asterisk SVN-branch-11-r380384M
      
      ATTACKER$ sipp -m 1 -sf register-username-disclosure.xml VICTIM -s 000 -ap badpass >/dev/null 
      000 is NOT a valid username
      ATTACKER$ sipp -m 1 -sf register-username-disclosure.xml VICTIM -s 001 -ap badpass >/dev/null 
      001 is NOT a valid username
      ATTACKER$ sipp -m 1 -sf register-username-disclosure.xml VICTIM -s 100 -ap badpass >/dev/null 
      100 is a valid username
      ATTACKER$ sipp -m 1 -sf register-username-disclosure.xml VICTIM -s 101 -ap badpass >/dev/null 
      101 is a valid username
      

      I haven't done any work on fixing the issue. But it's likely that the right fix would be to follow the normal challenge code path as much as possible.

      Regards,
      Walter Doekes
      OSSO B.V.
      (my employer wouldn't mind if OSSO B.V. is mentioned in a security bulletin if that were to be produced)

      1. AST-2013-003-1.8.diff
        14 kB
        Matt Jordan
      2. AST-2013-003-10.diff
        16 kB
        Matt Jordan
      3. AST-2013-003-11.diff
        16 kB
        Matt Jordan
      4. ASTERISK-21013.diff
        16 kB
        Kinsey Moore
      5. ASTERISK-21013.diff
        8 kB
        Kinsey Moore
      6. ASTERISK-21013.diff
        2 kB
        Kinsey Moore
      7. invite-username-disclosure-1.xml
        2 kB
        Walter Doekes
      8. invite-username-disclosure-1.xml
        2 kB
        Walter Doekes
      9. invite-username-disclosure-2.xml
        3 kB
        Walter Doekes
      10. invite-username-disclosure-2.xml
        3 kB
        Walter Doekes
      11. invite-username-disclosure-3.xml
        4 kB
        Walter Doekes
      12. invite-username-disclosure-3.xml
        4 kB
        Walter Doekes
      13. issueA21013_better_but_not_there_yet.patch
        4 kB
        Walter Doekes
      14. issueA21013_bogopeer_still_needs_alwaysauthreject_cleanup.patch
        6 kB
        Walter Doekes
      15. issueA21013_more_cleanup_more_fixes.patch
        14 kB
        Walter Doekes
      16. issueA21013_with_null_check.patch
        16 kB
        Walter Doekes
      17. register-username-disclosure.xml
        2 kB
        Walter Doekes
      18. register-username-disclosure.xml
        2 kB
        Walter Doekes
      19. register-username-disclosure-2.xml
        2 kB
        Walter Doekes

        Issue Links

          Activity

          No builds found.
          Walter Doekes created issue -
          Matt Jordan made changes -
          Field Original Value New Value
          Security None [ 10113 ] Reporter, Bug Marshals, and Digium [ 10110 ]
          Walter Doekes made changes -
          Summary DUMMY (be patient.. still updating this issue)
          Walter Doekes made changes -
          Affects Version/s 11.2.1 [ 11983 ]
          Walter Doekes made changes -
          Component/s Channels/chan_sip/General [ 10919 ]
          Walter Doekes made changes -
          Attachment register-username-disclosure.xml [ 46224 ]
          Walter Doekes made changes -
          Description FIXME So.. I was trying if I could alter the SIP security framework messages to differentiate between auth failures for any UDP packet and those with a valid nonce. Those with a valid nonce would probably not have a spoofed IP, so I can use fail2ban on them with more peace of mind.

          But, then I saw the different handling of the alwaysauthreject-challenge and the "normal" challenge code. These differences can be observed by an attacker sniffing for valid usernames.

          {noformat}
          VICTIM$ sudo asterisk -nrx 'sip show peers' | head -n4
          Name/username...
          100...
          101...
          102...

          VICTIM$ sudo asterisk -nrx 'core show version'
          Asterisk SVN-branch-11-r380384M
          {noformat}

          {noformat}
          ATTACKER$ sipp -m 1 -sf register-username-disclosure.xml VICTIM -s 000 -ap badpass >/dev/null
          000 is NOT a valid username
          ATTACKER$ sipp -m 1 -sf register-username-disclosure.xml VICTIM -s 001 -ap badpass >/dev/null
          001 is NOT a valid username
          ATTACKER$ sipp -m 1 -sf register-username-disclosure.xml VICTIM -s 100 -ap badpass >/dev/null
          100 is a valid username
          ATTACKER$ sipp -m 1 -sf register-username-disclosure.xml VICTIM -s 101 -ap badpass >/dev/null
          101 is a valid username
          {noformat}

          I haven't done any work on fixing the issue. But it's likely that the right fix would be to follow the normal challenge code path as much as possible.

          Regards,
          Walter Doekes
          OSSO B.V.
          (my employer wouldn't mind if OSSO B.V. is mentioned in a security bulletin if that were to be produced)
          Walter Doekes made changes -
          Comment [ So.. I was trying if I could alter the SIP security framework messages to differentiate between auth failures for any UDP packet and those with a valid nonce. Those with a valid nonce would probably not have a spoofed IP, so I can use fail2ban on them with more peace of mind.

          But, then I saw the different handling of the alwaysauthreject-challenge and the "normal" challenge code. These differences can be observed by an attacker sniffing for valid usernames.

          {noformat}
          VICTIM$ sudo asterisk -nrx 'sip show peers' | head -n4
          Name/username...
          100...
          101...
          102...

          VICTIM$ sudo asterisk -nrx 'core show version'
          Asterisk SVN-branch-11-r380384M
          {noformat}

          {noformat}
          ATTACKER$ sipp -m 1 -sf register-username-disclosure.xml VICTIM -s 000 -ap badpass >/dev/null
          000 is NOT a valid username
          ATTACKER$ sipp -m 1 -sf register-username-disclosure.xml VICTIM -s 001 -ap badpass >/dev/null
          001 is NOT a valid username
          ATTACKER$ sipp -m 1 -sf register-username-disclosure.xml VICTIM -s 100 -ap badpass >/dev/null
          100 is a valid username
          ATTACKER$ sipp -m 1 -sf register-username-disclosure.xml VICTIM -s 101 -ap badpass >/dev/null
          101 is a valid username
          {noformat}

          I haven't done any work on fixing the issue. But it's likely that the right fix would be to follow the normal challenge code path as much as possible.

          Regards,
          Walter Doekes
          OSSO B.V.
          (my employer wouldn't mind if OSSO B.V. is mentioned in a security bulletin if that were to be produced) ]
          Walter Doekes made changes -
          Summary (be patient.. still updating this issue) sip username disclosure
          Rusty Newton made changes -
          Status Triage [ 10000 ] Open [ 1 ]
          Rusty Newton made changes -
          Link This issue is the original version of this clone: ASTERISK-21018 [ ASTERISK-21018 ]
          Kinsey Moore made changes -
          Assignee Kinsey Moore [ kmoore ]
          Hide
          Kinsey Moore added a comment -

          Would you mind attaching the sip.conf you used with this SIPp scenario? I know it's likely to be trivial, but I want to ensure I have exactly the same setup for testing.

          Show
          Kinsey Moore added a comment - Would you mind attaching the sip.conf you used with this SIPp scenario? I know it's likely to be trivial, but I want to ensure I have exactly the same setup for testing.
          Hide
          Walter Doekes added a comment -
          $ cat /etc/asterisk/sip.conf
          [user](!)
          type=friend
          host=dynamic
          secret=notemptysecret
          [alice](user)
          [bob](user)
          
          $ sipp -m 1 -sf register-username-disclosure.xml THE_HOST -s alice >/dev/null 
          alice is a valid username
          
          $ sipp -m 1 -sf register-username-disclosure.xml THE_HOST -s kirk >/dev/null 
          kirk is NOT a valid username
          
          Show
          Walter Doekes added a comment - $ cat /etc/asterisk/sip.conf [user](!) type=friend host=dynamic secret=notemptysecret [alice](user) [bob](user) $ sipp -m 1 -sf register-username-disclosure.xml THE_HOST -s alice >/dev/null alice is a valid username $ sipp -m 1 -sf register-username-disclosure.xml THE_HOST -s kirk >/dev/null kirk is NOT a valid username
          Hide
          Walter Doekes added a comment -

          P.S. I retract my "making the fake path follow the normal one". A 403 should be the right response after the retransmission of the 2nd CSeq 2 REGISTER, not a 401 like the valid user now gets.

          Show
          Walter Doekes added a comment - P.S. I retract my "making the fake path follow the normal one". A 403 should be the right response after the retransmission of the 2nd CSeq 2 REGISTER, not a 401 like the valid user now gets.
          Hide
          Kinsey Moore added a comment -

          Could you give the attached patch a spin? It corrects the issue demonstrated by the test provided.

          Show
          Kinsey Moore added a comment - Could you give the attached patch a spin? It corrects the issue demonstrated by the test provided.
          Kinsey Moore made changes -
          Attachment ASTERISK-21013.diff [ 46274 ]
          Kinsey Moore made changes -
          Attachment ASTERISK-21013.diff [ 46274 ]
          Hide
          Kinsey Moore added a comment -

          Uploaded new version of the diff. This is a little more narrow in scope and showed no issues in the SIP tests I ran on it from the testsuite while still passing the test case you provided.

          Show
          Kinsey Moore added a comment - Uploaded new version of the diff. This is a little more narrow in scope and showed no issues in the SIP tests I ran on it from the testsuite while still passing the test case you provided.
          Kinsey Moore made changes -
          Attachment ASTERISK-21013.diff [ 46280 ]
          Hide
          Walter Doekes added a comment - - edited

          Your patch fixes the issue for the register. But continuing, I see issues for the invite.

          • register-username-disclosure.xml different handling for destruction of authenticated register transaction tuple (retransmission of 2nd register yields 401 when user exists instead of 403)
          • invite-username-disclosure-1.xml different www response code (401 when user exists, 407 when user doesn't)
          • invite-username-disclosure-2.xml different 403 message (Forbidden when user exists, Forbidden Bad Auth when user doesn't)
          • invite-username-disclosure-3.xml different handling of destruction again, but this time for invites

          Let's allowguest=no, so we can check.

          VICTIM# cat /etc/asterisk/sip.conf
          [general]
          allowguest=no
          
          [user](!)
          type=friend
          host=dynamic
          secret=notemptysecret
          
          [alice](user)
          [bob](user)
          

          Before patch:

          $ for scenario in register-username-disclosure.xml invite-username-disclosure-{1,2,3}.xml
              do for username in alice bob kirk alice
              do sipp -m 1 -sf $scenario $VICTIM -s $username 3>&1 >/dev/null 2>&1 | sed -e s/^/$scenario:\ /
            done; echo; done
          register-username-disclosure.xml: alice is a valid username
          register-username-disclosure.xml: bob is a valid username
          register-username-disclosure.xml: kirk is NOT a valid username
          register-username-disclosure.xml: alice is a valid username
          
          invite-username-disclosure-1.xml: alice is a valid username
          invite-username-disclosure-1.xml: bob is a valid username
          invite-username-disclosure-1.xml: kirk is NOT a valid username
          invite-username-disclosure-1.xml: alice is a valid username
          
          invite-username-disclosure-2.xml: alice is a valid username
          invite-username-disclosure-2.xml: bob is a valid username
          invite-username-disclosure-2.xml: kirk is NOT a valid username
          invite-username-disclosure-2.xml: alice is a valid username
          
          invite-username-disclosure-3.xml: alice is a valid username
          invite-username-disclosure-3.xml: bob is a valid username
          invite-username-disclosure-3.xml: kirk is NOT a valid username
          invite-username-disclosure-3.xml: alice is a valid username
          

          After patch, the first issue is fixed indeed:

          $ for..loop..like..above
          register-username-disclosure.xml: alice is NOT a valid username
          register-username-disclosure.xml: bob is NOT a valid username
          register-username-disclosure.xml: kirk is NOT a valid username
          register-username-disclosure.xml: alice is NOT a valid username
          
          ...unchanged...
          

          I'm guessing we can adapt the scenarios to use MESSAGE and SUBSCRIBE requests as well.

          Show
          Walter Doekes added a comment - - edited Your patch fixes the issue for the register. But continuing, I see issues for the invite. register-username-disclosure.xml different handling for destruction of authenticated register transaction tuple (retransmission of 2nd register yields 401 when user exists instead of 403) invite-username-disclosure-1.xml different www response code (401 when user exists, 407 when user doesn't) invite-username-disclosure-2.xml different 403 message (Forbidden when user exists, Forbidden Bad Auth when user doesn't) invite-username-disclosure-3.xml different handling of destruction again, but this time for invites Let's allowguest=no, so we can check. VICTIM# cat /etc/asterisk/sip.conf [general] allowguest=no [user](!) type=friend host=dynamic secret=notemptysecret [alice](user) [bob](user) Before patch: $ for scenario in register-username-disclosure.xml invite-username-disclosure-{1,2,3}.xml do for username in alice bob kirk alice do sipp -m 1 -sf $scenario $VICTIM -s $username 3>&1 >/dev/null 2>&1 | sed -e s/^/$scenario:\ / done; echo; done register-username-disclosure.xml: alice is a valid username register-username-disclosure.xml: bob is a valid username register-username-disclosure.xml: kirk is NOT a valid username register-username-disclosure.xml: alice is a valid username invite-username-disclosure-1.xml: alice is a valid username invite-username-disclosure-1.xml: bob is a valid username invite-username-disclosure-1.xml: kirk is NOT a valid username invite-username-disclosure-1.xml: alice is a valid username invite-username-disclosure-2.xml: alice is a valid username invite-username-disclosure-2.xml: bob is a valid username invite-username-disclosure-2.xml: kirk is NOT a valid username invite-username-disclosure-2.xml: alice is a valid username invite-username-disclosure-3.xml: alice is a valid username invite-username-disclosure-3.xml: bob is a valid username invite-username-disclosure-3.xml: kirk is NOT a valid username invite-username-disclosure-3.xml: alice is a valid username After patch, the first issue is fixed indeed: $ for..loop..like..above register-username-disclosure.xml: alice is NOT a valid username register-username-disclosure.xml: bob is NOT a valid username register-username-disclosure.xml: kirk is NOT a valid username register-username-disclosure.xml: alice is NOT a valid username ...unchanged... I'm guessing we can adapt the scenarios to use MESSAGE and SUBSCRIBE requests as well.
          Walter Doekes made changes -
          Attachment invite-username-disclosure-1.xml [ 46283 ]
          Walter Doekes made changes -
          Attachment invite-username-disclosure-2.xml [ 46284 ]
          Walter Doekes made changes -
          Attachment invite-username-disclosure-1.xml [ 46283 ]
          Walter Doekes made changes -
          Attachment invite-username-disclosure-2.xml [ 46284 ]
          Walter Doekes made changes -
          Attachment register-username-disclosure.xml [ 46224 ]
          Walter Doekes made changes -
          Attachment invite-username-disclosure-1.xml [ 46285 ]
          Walter Doekes made changes -
          Attachment invite-username-disclosure-2.xml [ 46286 ]
          Walter Doekes made changes -
          Attachment invite-username-disclosure-3.xml [ 46287 ]
          Walter Doekes made changes -
          Attachment register-username-disclosure.xml [ 46288 ]
          Hide
          Walter Doekes added a comment -

          In conclusion: the fact that there is a "fake" response, makes this happen. Perhaps a quickfix could make use of a "bogopeer" which always has the wrong password/acl. Then we would never hit the fake response generation and the code paths for existing and nonexistent users would always be equal.

          Show
          Walter Doekes added a comment - In conclusion: the fact that there is a "fake" response, makes this happen. Perhaps a quickfix could make use of a "bogopeer" which always has the wrong password/acl. Then we would never hit the fake response generation and the code paths for existing and nonexistent users would always be equal.
          Kinsey Moore made changes -
          Attachment ASTERISK-21013.diff [ 46280 ]
          Hide
          Kinsey Moore added a comment - - edited

          The version of the patch I just uploaded seems to handle the additional test cases with the one change. I'm still looking into your suggestion on an alternative fix.

          Show
          Kinsey Moore added a comment - - edited The version of the patch I just uploaded seems to handle the additional test cases with the one change. I'm still looking into your suggestion on an alternative fix.
          Kinsey Moore made changes -
          Attachment ASTERISK-21013.diff [ 46298 ]
          Hide
          Walter Doekes added a comment -

          Yes the addional tests. But the register-username-disclosure now only shows kirk. (Because the others get a 407 instead of a 401.) Same for a SUBSCRIBE.

          Show
          Walter Doekes added a comment - Yes the addional tests. But the register-username-disclosure now only shows kirk. (Because the others get a 407 instead of a 401.) Same for a SUBSCRIBE.
          Hide
          Walter Doekes added a comment -

          In fact, only test invite-username test 1 and 2 were fixed. Test 3 gives errors which we don't see. It only appears fixed.

          If I change this:

           	/* We have to emulate EXACTLY what we'd get with a good peer
           	 * and a bad password, or else we leak information. */
          -	const char *response = "407 Proxy Authentication Required";
          -	const char *reqheader = "Proxy-Authorization";
          -	const char *respheader = "Proxy-Authenticate";
          +	const char *response = "401 Unauthorized";
          +	const char *reqheader = "Authorization";
          +	const char *respheader = "WWW-Authenticate";
          

          Things get better, but now invite tests 2 fails too. It gets "(Bad auth)" for the failed user only.

          So.. start removing the alternative headers.

          -		transmit_response(p, "403 Forbidden (Bad auth)", &p->initreq);
          +		transmit_response(p, "403 Forbidden", &p->initreq);
          

          (You can now remove all optional 407 handling from the *xml.)

          Now I get success on the first three, but now I still have failure on invite-username-disclosure-3.xml and a subscribe*.xml test shows me that unknown users get a 401 instead of a 403 after failed auth.

          Show
          Walter Doekes added a comment - In fact, only test invite-username test 1 and 2 were fixed. Test 3 gives errors which we don't see. It only appears fixed. If I change this: /* We have to emulate EXACTLY what we'd get with a good peer * and a bad password, or else we leak information. */ - const char *response = "407 Proxy Authentication Required"; - const char *reqheader = "Proxy-Authorization"; - const char *respheader = "Proxy-Authenticate"; + const char *response = "401 Unauthorized"; + const char *reqheader = "Authorization"; + const char *respheader = "WWW-Authenticate"; Things get better, but now invite tests 2 fails too. It gets "(Bad auth)" for the failed user only. So.. start removing the alternative headers. - transmit_response(p, "403 Forbidden (Bad auth)", &p->initreq); + transmit_response(p, "403 Forbidden", &p->initreq); (You can now remove all optional 407 handling from the *xml.) Now I get success on the first three, but now I still have failure on invite-username-disclosure-3.xml and a subscribe*.xml test shows me that unknown users get a 401 instead of a 403 after failed auth.
          Walter Doekes made changes -
          Walter Doekes made changes -
          Walter Doekes made changes -
          Attachment register-username-disclosure.xml [ 46345 ]
          Walter Doekes made changes -
          Attachment invite-username-disclosure-1.xml [ 46346 ]
          Walter Doekes made changes -
          Attachment invite-username-disclosure-2.xml [ 46347 ]
          Walter Doekes made changes -
          Attachment invite-username-disclosure-3.xml [ 46348 ]
          Hide
          Walter Doekes added a comment -

          Ok. I uploaded something with a bogus peer which should never authenticate:

          issueA21013_bogopeer_still_needs_alwaysauthreject_cleanup.patch

          I updated the XML scenarios that go with it. I left the old ones still available to test against unpatched asterisken (note that invite-username-disclosure-

          {2,3}

          .xml had bad CSeqs in the ACKs / was missing a final ACK).

          (And yes.. some say now say that the user is or is not valid even when it is/not. The point is that the answers are the same.)

          The patch could use some review as to wether it's 100% safe
          Further, we can now remove lots of if(alwaysauthreject) blocks and do that check in check_peer_ok only and return AUTH_DONT_KNOW if !alwaysauthreject.

          Show
          Walter Doekes added a comment - Ok. I uploaded something with a bogus peer which should never authenticate: issueA21013_bogopeer_still_needs_alwaysauthreject_cleanup.patch I updated the XML scenarios that go with it. I left the old ones still available to test against unpatched asterisken (note that invite-username-disclosure- {2,3} .xml had bad CSeqs in the ACKs / was missing a final ACK). (And yes.. some say now say that the user is or is not valid even when it is/not. The point is that the answers are the same.) The patch could use some review as to wether it's 100% safe Further, we can now remove lots of if(alwaysauthreject) blocks and do that check in check_peer_ok only and return AUTH_DONT_KNOW if !alwaysauthreject.
          Hide
          Kinsey Moore added a comment -

          Hi Walter,
          I just wanted to let you know that I'm still working on this. I'm finding other information leaks along the way to reviewing your scenarios and patches mostly in the way of transmitting things reliably when they shouldn't be.

          Show
          Kinsey Moore added a comment - Hi Walter, I just wanted to let you know that I'm still working on this. I'm finding other information leaks along the way to reviewing your scenarios and patches mostly in the way of transmitting things reliably when they shouldn't be.
          Hide
          Kinsey Moore added a comment -

          Updated ASTERISK-21013.diff to merge your bogopeer changes and added some other changes that I noticed along the way. I was seeing some weird behavior on some "not a real user" transactions vs "real user" transactions with regards to retransmission which could have given away details about users.

          Show
          Kinsey Moore added a comment - Updated ASTERISK-21013 .diff to merge your bogopeer changes and added some other changes that I noticed along the way. I was seeing some weird behavior on some "not a real user" transactions vs "real user" transactions with regards to retransmission which could have given away details about users.
          Kinsey Moore made changes -
          Attachment ASTERISK-21013.diff [ 46363 ]
          Walter Doekes made changes -
          Hide
          Walter Doekes added a comment - - edited

          I take your ASTERiSK-21013.diff and I raise you issueA21013_more_cleanup_more_fixes.patch.

          (a) Don't rely on the MD5 hash not matching. You could use it to bypass auth. (Yes.. not timing attack safe.. but don't think the rest of the code qualifies either.)

          (b) Fix sip_cfg.allowguest usage.

          (c) Fix minor verbose log.

          (d) Remove some sip_cfg.alwaysauthreject code which we won't reach anymore.

          (e) Remove all AUTH_FAKE_AUTH occurrences.

          (f) Make sure a global insecure=invite doesn't open up everything for everyone. NOTE: a sip reload where someone switches the default NAT type would now leave the bogopeer in the previous state. I didn't feel like fixing this.

          (g) Fix so we're safe against register-username-disclosure-2.xml: using CSeq 3 now yielded different results for REGISTER.

          Further:

          (h) The CHANGES should probably mention that all auth requests now do WWW-Auth and never Proxy-Auth.

          The fact that there still exists calls to transmit_fake_auth_response suggests that not all potential avenues are closed.

          (j) Someone should probably test that the guests (for invites) and temppeers (for registers) still work like they should.

          Show
          Walter Doekes added a comment - - edited I take your ASTERiSK-21013.diff and I raise you issueA21013_more_cleanup_more_fixes.patch . (a) Don't rely on the MD5 hash not matching. You could use it to bypass auth. (Yes.. not timing attack safe.. but don't think the rest of the code qualifies either.) (b) Fix sip_cfg.allowguest usage. (c) Fix minor verbose log. (d) Remove some sip_cfg.alwaysauthreject code which we won't reach anymore. (e) Remove all AUTH_FAKE_AUTH occurrences. (f) Make sure a global insecure=invite doesn't open up everything for everyone. NOTE: a sip reload where someone switches the default NAT type would now leave the bogopeer in the previous state. I didn't feel like fixing this. (g) Fix so we're safe against register-username-disclosure-2.xml : using CSeq 3 now yielded different results for REGISTER. Further: (h) The CHANGES should probably mention that all auth requests now do WWW-Auth and never Proxy-Auth. The fact that there still exists calls to transmit_fake_auth_response suggests that not all potential avenues are closed. (j) Someone should probably test that the guests (for invites) and temppeers (for registers) still work like they should.
          Walter Doekes made changes -
          Attachment register-username-disclosure-2.xml [ 46366 ]
          Hide
          Kinsey Moore added a comment - - edited

          (f) Fixed by refreshing the bogopeer in sip_reload.

          (h) This was apparently removed in 1.8 in relation to AST-2011-011 in r325279. It was not applied to trunk at that time. It will have to go into 10+.

          This is now contained to register_verify which seems to be a combination of registration business logic and auth code. I primarily just undid the changes to check_auth() and retested with the scenarios provided for register, invite, and subscribe. They all passed as far as I could tell. Removing this from register_verify is going to be too invasive for existing branches, so I'm leaving it as is since it is currently working and may attempt a refactor for trunk.

          Did some cleanup in transmit_fake_auth_response since sipmethod is no longer necessary. Changed bogopeer to bogus_peer.

          Show
          Kinsey Moore added a comment - - edited (f) Fixed by refreshing the bogopeer in sip_reload. (h) This was apparently removed in 1.8 in relation to AST-2011-011 in r325279. It was not applied to trunk at that time. It will have to go into 10+. This is now contained to register_verify which seems to be a combination of registration business logic and auth code. I primarily just undid the changes to check_auth() and retested with the scenarios provided for register, invite, and subscribe. They all passed as far as I could tell. Removing this from register_verify is going to be too invasive for existing branches, so I'm leaving it as is since it is currently working and may attempt a refactor for trunk. Did some cleanup in transmit_fake_auth_response since sipmethod is no longer necessary. Changed bogopeer to bogus_peer.
          Kinsey Moore made changes -
          Attachment ASTERISK-21013.diff [ 46376 ]
          Hide
          Walter Doekes added a comment -

          Yuck at reload() calling the cli sip_reload() instead of the other way around. But you didn't do that

          Ok. Your patch looks sane. I added the mandatory null-check for the sip reload in issueA21013_with_null_check.patch.

          I quickly ran the tests against 11.x, and it all seems fixed. Better get a 3rd set of eyes on it and run tests of regular behaviour. Did you run any allowguest and autocreatepeer checks?

          Show
          Walter Doekes added a comment - Yuck at reload() calling the cli sip_reload() instead of the other way around. But you didn't do that Ok. Your patch looks sane. I added the mandatory null-check for the sip reload in issueA21013_with_null_check.patch . I quickly ran the tests against 11.x, and it all seems fixed. Better get a 3rd set of eyes on it and run tests of regular behaviour. Did you run any allowguest and autocreatepeer checks?
          Walter Doekes made changes -
          Attachment issueA21013_with_null_check.patch [ 46378 ]
          Hide
          Kinsey Moore added a comment -

          The existing tests in the integration testsuite report no problems. allowguest and autocreatepeer look good code-wise after a double check. I have yet to run manual tests on those two settings, but I don't expect any issues to come up for them. I'll get to those tests when I can.

          Show
          Kinsey Moore added a comment - The existing tests in the integration testsuite report no problems. allowguest and autocreatepeer look good code-wise after a double check. I have yet to run manual tests on those two settings, but I don't expect any issues to come up for them. I'll get to those tests when I can.
          Hide
          Kinsey Moore added a comment -

          Testing with allowguest and autocreatepeer looks good with alwaysauthreject enabled.

          Show
          Kinsey Moore added a comment - Testing with allowguest and autocreatepeer looks good with alwaysauthreject enabled.
          Hide
          Walter Doekes added a comment -

          Good. I've been running the patch on a couple of 10.x production machines a couple of days now. Haven't noticed any odd behaviour or customer complaints.

          Show
          Walter Doekes added a comment - Good. I've been running the patch on a couple of 10.x production machines a couple of days now. Haven't noticed any odd behaviour or customer complaints.
          Hide
          Matt Jordan added a comment -

          FYI: The planned security patch date is Thursday, March 7th.

          Show
          Matt Jordan added a comment - FYI: The planned security patch date is Thursday, March 7th.
          Matt Jordan made changes -
          Summary sip username disclosure Security Vulnerability: sip username disclosure
          Matt Jordan made changes -
          Link This issue is a clone of ASTERISK-21212 [ ASTERISK-21212 ]
          Matt Jordan made changes -
          Link This issue must be completed before resolving ASTERISK-21005 [ ASTERISK-21005 ]
          Matt Jordan made changes -
          Link This issue must be completed before resolving ASTERISK-21004 [ ASTERISK-21004 ]
          Matt Jordan made changes -
          Attachment AST-2013-003-1.8.diff [ 46826 ]
          Attachment AST-2013-003-10.diff [ 46827 ]
          Attachment AST-2013-003-11.diff [ 46828 ]
          Matt Jordan made changes -
          Status Open [ 1 ] Closed [ 6 ]
          Resolution Fixed [ 1 ]
          Matt Jordan made changes -
          Security Reporter, Bug Marshals, and Digium [ 10110 ]
          Matt Jordan made changes -
          Target Release Version/s 11.2.2 [ 12193 ]
          Target Release Version/s 10.12.2-digiumphones [ 12192 ]
          Target Release Version/s 10.12.2 [ 12191 ]
          Target Release Version/s 1.8.20.2 [ 12190 ]
          Matt Jordan made changes -
          Target Release Version/s 1.8.22.0 [ 12197 ]
          Matt Jordan made changes -
          Target Release Version/s 11.4.0 [ 12198 ]
          Transition Time In Source Status Execution Times Last Executer Last Execution Date
          Triage Triage Open Open
          1d 8h 52m 1 Rusty Newton 31/Jan/13 2:53 PM
          Open Open Closed Closed
          54d 19h 1 Matt Jordan 27/Mar/13 10:54 AM

            People

            • Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development