IP check too strict

[bbrazil]

In trac/web/auth.py the IP check against cookies/sessions requries the exact same IP. This doesn't work for NAT over multiple IP addresses, or using multiple proxies which makes login impossible. The solution is to check only the /24.

Patch (implementation by Dinko Korunic):

--- trac/web/auth.py    (revision 3356)
+++ trac/web/auth.py    (working copy)
@@ -166,9 +166,11 @@
        db = self.env.get_db_cnx()
        cursor = db.cursor()
        if self.check_ip:
+            ipaddr = '.'.join(req.remote_addr.split('.')[:-1])
+            ipaddr = ipaddr + '%'
            cursor.execute("SELECT name FROM auth_cookie "
-                           "WHERE cookie=%s AND ipnr=%s",
-                           (cookie.value, req.remote_addr))
+                           "WHERE cookie=%s AND ipnr LIKE %s",
+                           (cookie.value, ipaddr))
        else:
            cursor.execute("SELECT name FROM auth_cookie WHERE cookie=%s",
                           (cookie.value,))

This is related to #1485

[Emmanuel Blot]

Assuming that the subnet is defined as 24.8 is network-specific.

This kind of hardcoded value cannot be used as-is.

[coderanger]

This can already be controled with the check_auth_ip option.

[Dinko Korunic <dinko.korunic@…>]

No, it cannot be controlled. There is a big difference between completely turning off the IP cookie check (via check_auth_ip) and comparing a part of IP address (and being less prone to cookie-based attacks).

[Dinko Korunic <dinko.korunic@…>]

Anyway, I absolutely agree that idea to hardcode /24 check is bad - I presume there shouldn't be to complex to add one more config notification setting which will define netmask width and use it here.

[Christian Boos]

A patch would probably help to revive this issue…

[Dalius Dobravolskas]

Here is alternate fix using mask. It is a little bit better than proposed because it is configurable to use either strict IP check or /24. By default it is /24 (mask=255.255.255.0).

My fix is for trac OpenID authentication plugin (​http://trac-hacks.org/wiki/AuthOpenIdPlugin) but I'm pretty sure it is not hard to adapt my fix to original Trac. Here is diff: ​http://hg.sandbox.lt/authopenid-plugin/rev/7f795537f6d2

[Christian Boos]

Milestone 2.0 deleted

[Remy Blank]

check_auth_ip has been false by default for some time now, and solves the initial issue. I don't think adding subnet checking is worth the additional code and configuration complexity.