[asterisk-dev] [Code Review]: Generate security events in chan_sip using new Security Events Framework

[Tilghman Lesher]

> On Aug. 12, 2011, 2:14 a.m., Tilghman Lesher wrote:
> > /branches/10/channels/chan_sip.c, line 1713
> > <https://reviewboard.asterisk.org/r/1362/diff/1/?file=18288#file18288line1713>
> >
> >     It is helpful to know whether the invalid password is different from the previous invalid password from this peer (if possible).  You don't need to know what that previous guess was, as a security watcher can be expected to keep history.  This can be important, to distinguish a misconfigured phone with the wrong password (false positive) from a scan attack.
> 
> elguero wrote:
>     Hmm... not sure how we would report that with the current framework as it is.
>     
>     We would have to somehow store the previous guess, so that we could compare the new guess and determine if they are the same.  Is that being done anywhere?  Any quick suggestions?
>     
>     Then we would need to add an element to this security event structure, such as, new_password_attempt and make it optional.
>     
>     Let me know if I am over thinking this.
> 
> Kevin Fleming wrote:
>     No, that's the right strategy, but I'm not sure it's achievable. All you have as a response in an authentication attempt is a digest computed from the shared secret password, a nonce and some other bits. When you get a second authentication attempt which fails, you don't have any way to determine whether they used the same wrong password or not, because the digest value is going to be different by virtue of the nonce being different. It would be possible to reuse the nonce in order to determine whether the attempts are using different passwords or not, but that's a larger change that shouldn't be included in this patch.
> 
> rmudgett wrote:
>     Reusing the nonce is a bad thing because it allows replay attacks.

Reusing the nonce is only a problem in the case of a successful authentication.  What would be the value in replaying a failed authentication?

Asterisk doesn't need to know the previous value, as long as it is delivering a constant algorithmic result (i.e. the same attempted password delivers the same hash) to the backend for comparison.  It can be safely expected that the monitoring system will keep a history state and do appropriate analysis of the stream.


> On Aug. 12, 2011, 2:14 a.m., Tilghman Lesher wrote:
> > /branches/10/channels/chan_sip.c, lines 25083-25085
> > <https://reviewboard.asterisk.org/r/1362/diff/1/?file=18288#file18288line25083>
> >
> >     Similarly, another case for invalid password.  In a scan attack, any found peer will likely get this same event multiple times, indicating a real problem to the security event watcher.
> 
> elguero wrote:
>     Same as prior comment.  What do you think of using failed_acl and passing in "Peer Not Dynamic"?

That's fine.  More differentiation is good.


- Tilghman


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/1362/#review4044
-----------------------------------------------------------


On Aug. 12, 2011, 1:07 a.m., elguero wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1362/
> -----------------------------------------------------------
> 
> (Updated Aug. 12, 2011, 1:07 a.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> Security Events Framework was added in 1.8 and support was added for AMI to generate events at that time.
> 
> This patch attempts to add support in chan_sip to generate security events.  Hopefully we can get this into Asterisk 10.
> 
> I am looking forward to hearing feedback on where this patch can be improved especially from those who have an intimate knowledge of chan_sip.
> 
> Thanks
> 
> 
> This addresses bug 18264.
>     https://issues.asterisk.org/jira/browse/18264
> 
> 
> Diffs
> -----
> 
>   /branches/10/channels/chan_sip.c 331633 
>   /branches/10/configs/logger.conf.sample 331633 
>   /branches/10/CHANGES 331633 
> 
> Diff: https://reviewboard.asterisk.org/r/1362/diff
> 
> 
> Testing
> -------
> 
> Local dev machine and a softphone.  Generated events by using the wrong username, wrong password, wrong auth name, successful authentication.
> 
> 
> Thanks,
> 
> elguero
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20110813/34da93b6/attachment.htm>