Closed Bug 20122 Opened 22 years ago Closed 19 years ago

Bugzilla requires new login if IP changes

Categories

(Bugzilla :: User Accounts, defect, P1)

Tracking

()

RESOLVED FIXED
Bugzilla 2.18

People

(Reporter: dbaron, Assigned: bbaetz)

References

Details

Attachments

(1 file, 6 obsolete files)

Bugzilla requires that I login again every time my hostname changes, even if I'm
on the same computer.  This is very annoying when on a dialup (and people on a
LAN don't notice the problem).

Could this be changed?  The cause seems to be in quietly_check_login in
http://lxr.mozilla.org/mozilla/source/webtools/bugzilla/CGI.pl , where SendSQL
is called.
It's a security tradeoff.  If I don't do this, then it's much easier to spoof
someone's login.
OK.  I suspected that was the reason...
Summary: Bugzilla requires new login everytime hostname changes → Bugzilla requires new login every time hostname changes
FWIW, I have the same problem because my local www cache is actually two
machines, and so I keep switching which machine bugzilla thinks I am logged in.
Of course, the other problem is that anyone can spoof a cookie with my username
in it and send that to bugzilla, and if they are lucky (that is, if they get
the same proxy server as I had last time I logged in) then they will be able to
do one thing as if they were me.

David: see bug 3641. If you know of a better way of doing things, please tell
me, as I have the same problem with my own authenticating scripts.
Ian - wrong bug#.  Also, if you want to make it a *lot* harder to spoof a
cookie, have 2 cookies:
 * your username
 * a GPG (or PGP) signature on your username

I think that's what you were asking.
Oops, bug 3614! Sorry.

So the second cookie would include a PGP-signed version of the username, signed
by the server? So my server would need to have a private key that it could
use to sign the username. Hmm. Unfortunately, my server is not (read) secure so
I can't do that. If my script can read the private key, then anyone can read
the private key.

Interesting idea though. If checking a PGP-signature is quick, then it might
work. (Don't forget this signature would need to be checked everytime a page
is accessed!)
Status: NEW → ASSIGNED
*** Bug 18720 has been marked as a duplicate of this bug. ***
I have the same problem with my autodialup/-down set at 1 min.
*** Bug 33232 has been marked as a duplicate of this bug. ***
Sourceforge (after a similar bug) checks only for a B class match (i.e. the
first 2 bytes have to be the same). That's less secure, but will fix this bug.

If anybody can catch a cookie, can't he/she also catch the password?
tara@tequilarista.org is the new owner of Bugzilla and Bonsai.  (For details,
see my posting in netscape.public.mozilla.webtools,
news://news.mozilla.org/38F5D90D.F40E8C1A%40geocast.com .)
Assignee: terry → tara
Status: ASSIGNED → NEW
I'm really tired of this bug.

Severity: normal → major
mozilla@bucksch.org, and anyone else with this problem - go to 
http://www.typingmaster.com and download a free utility called QuickPhrase. 
Then, bind e.g. F12 to e.g.
{TAB}{TAB}{TAB}{TAB}gervase.markham@univ.ox.ac.uk{TAB}password{TAB}

Problem nearly solved :-) This is what I used for ages.

Gerv
> QuickPhrase

I'm using Unix.

> Problem nearly solved

Yes, but still
- one extra page load
- one extra button press and one extra mouse click
- some fuctions won't work, e.g. attaching or changing multiple bugs at once.

All this for nearly *each and every change* in bugzilla.
Are there no text insertion utils for Unix? I thought it was big on that sort of
stuff.

FWIW, I solved this problem for myself (caused by the UK JANET National Web
Cache using a round robin of machines) by getting our site admins to put
bugzilla.mozilla.org on the exceptions list. Would that work for you? You didn't
mention what part of your setup causes your IP to change so often, but I assume
it's a web cache.

Would it also be possible to enable bugzilla access on port 81 of
bugzilla.mozilla.org as well as port 80? That might help people.

Gerv
> Are there no text insertion utils for Unix?

If really tried to (i.e. spend a day look for apps, Xresoucrs etc.), I'd
propably find a solution. It wouldn't solve the problems I mentioned above.

But I don't want a workaround on the client side. There're so many people having
this problem, that it should be fixed on bugzilla's side. Soon.

There is an easy workaround on Bugzilla's side (check only for B class match).

> You didn't mention what part of your setup causes your IP to change so often

> ------- Additional Comments From Ben Bucksch 2000-02-18 12:55 -------
> I have the same problem with my autodialup/-down set at 1 min.

I'm on a dialup line.
    Why don't you just use the HTTP Authentication mechanism? Is there some
reason I don't see yet?

    That way the browser would pop up *once* with a window asking for a
username and password, but would submit it each time the server asks for
it, without any further user interaction. The browser remembers that info
until I close it, no matter what IP I have or what web cache I use. Many
sites work this way, and I think that's much more convenient.

    By the way, I just wanted to vote for this bug, but Bugzilla didn't let
me, saying "wrong login", although I was logged in and made sure not to
hang up the line inbetween. But that's probably a different bug.

    By the way again, I think the platform and OS field is wrong and should
be all/all, but I'm obviously not allowed to change it.
OS: Windows 95 → All
Hardware: PC → All
I have a workaround for this bug: bookmark the following URL

http://bugzilla.mozilla.org/showvotes.cgi?Bugzilla_login=YOUR@EMAIL&Bugzilla_password=YOURPASSWORD

    and replace YOUR@EMAIL and YOURPASSWORD with the appropriate values.
Every time you connect to your ISP, load that URL in a new browser window.
As soon as some page content starts appearing, you can press "commit" in
the other window where you want to make some changes.

    You could also use query.cgi instead of showvotes, but this conserves a
lot more bandwidth.

    Of course this is insecure, since your password will be stored in
plaintext on your hard drive. Maybe you should consider this before you do
it on a machine where others have root access. :-)

    And of course it doesn't solve this bug at all. I'd still be really
glad if someone would fix it!
The problem with using HTTP authentication is that it requires the cooperation of 
the web server on the machine that is hosting Bugzilla.  You will need to teach 
the web server how to read your bugzilla user database.  This is not always easy, 
and would be a major setup chore for anyone installing Bugzilla, and would 
probably also limit which web servers you could use it with.
Stephan: It's insecure anyway, as your password is sent in plaintext across
the internet every time you log on...

Dave: Couldn't the script take care of the authentication part itself? Why
require the webserver to do any work?
>Couldn't the script take care of the authentication part itself? Why
>require the webserver to do any work?

That's the problem, the script *is* doing the authentication part NOW.  People 
don't like that because it requires cookies.  The CGI protocol only passes the 
authenticated username to the script, it doesn't pass along their password.  So 
the script has no idea what password they used, if they use the standard HTTP 
authentication.  The suggestion was to use the standard HTTP authentication.  If 
the server is doing this authentication, the script can easily tell who's logged 
in.

Although the script could certainly issue the authentication challenge, the 
response from the user's browser would get intercepted by the server on the way 
back, and the script would never see it.

I do have an idea though...  instead of teaching the web server how to read 
bugzilla's user database, how about teaching bugzilla to maintain standard 
.htaccess and htpasswd files as well as its own database?

That, again, will have additional compatibility problems, as not all webservers 
use .htaccess for authentication information.
Assuming it would work with Apache, I'd value the user's problems over the
choices of the admin (but I am biased :) ).
Yeah, it would work for Apache....

speaking of Apache...  I seem to recall mentioning above about teaching a web 
server how to read Bugzilla's user database...  well, someone already did it.  
Not to work with Bugzilla specifically, but with mySQL in general, and you can 
give it whatever database, table, and column you want for the usernames and 
passwords, so you could easily point it at the profiles table in the bugs 
database. :)

Look here: http://bourbon.netvision.net.il/mysql/mod_auth_mysql/
It turns out Apache doesn't steal the "Authorization:" line and prevent the
script from knowing it was given some credentials -- it puts the relevant
line into the environment variable HTTP_AUTHORIZATION.

So what Bugzilla needs to do instead of sending the login page with a form is
reply with

    HTTP/1.1 401 Unauthorized
    WWW-Authenticate: Basic realm="Bugzilla"

...and follow that with a page providing a button to send/create a password.

The browser with respond to this and the following code parses the reply:

   use MIME::Base64;
   my($user, $pass);

   # Did client provide credentials?
   if ($ENV{'HTTP_AUTHORIZATION'} =~ /^Basic (.*)$/os) {
       ($user, $pass) = split(/:/, decode_base64($1), 2);
   }

   # Can user log in?
   # The "authenticate" function should return true if $user has password
   # $password and false otherwise. It should return false if $user is
   # blank, too.
   if (&authenticate($user, $password)) {
       print "HTTP/1.1 200 OK\n";
   } else {
       print "HTTP/1.1 401 Unauthorized\n";
       print "WWW-Authenticate: Basic realm=\"Bugzilla\"\n";
   }

That should work.
Whiteboard: possible solution suggested
I suspect I have troubles attaching files to bug reports because of this bug...
When I try attaching a file to a bug report, I am prompted for my password over
and over again. Each time, 'create attachment' fails with the error message
that no attachment was supplied.
rbs, right. You must not change your hostname between loading the attachment
page and commiting/uploading. You're behind a proxy? Out of luck :(. (I don't
know any workaround. Somebody else?)

BTW: I like Ian's proposal. Can somebody get this to a point where it is ready
to be checked into the tree and activated on mozilla.org, please?
BTW2: I happen to have a new ISP plan, which makes this bug much less bad for
me.
*** Bug 38831 has been marked as a duplicate of this bug. ***
I spent a couple hours today experimenting with Ian's suggestion that the script 
should still get the HTTP_AUTHORIZATION environment variable.  I now have a copy 
of CGI.pl which would theoretically handles all of its authentication this way.

After my experiments, I'll stand on my original statement that the information 
never gets passed to the CGI script.  Apache 1.3.5 eats all the authorization 
stuff, and it never makes it anywhere the script can see it.  I can successfully 
return 401 Unauthorized headers that trick the browser into authenticating, and 
by logging headers on the server, I can see that the browser is indeed sending an 
Authentication: header, but when logging the environment variables avaialble to 
the script, there's nothing that even remotely sounds like it showing up in %ENV.

At least, this is the case with Apache 1.3.5 on Linux...
Quoted from http://hoohoo.ncsa.uiuc.edu/cgi/env.html:
[...] In addition to these, the header lines received from the client, if any,
are placed into the environment with the prefix HTTP_ followed by the header
name. [...] The server may exclude any headers which it has already processed,
such as Authorization [...].
Hmm. It works for me!

My proof of concept script is available online:
Source: 
   http://www.bath.ac.uk/%7Epy8ieh/cgi-source/development/tests/nph-auth_basic
Executable:
   http://www.bath.ac.uk/%7Epy8ieh/cgi/development/tests/nph-auth_basic

Is there some way to tell Apache to not bother authenticating? (There must be,
I assume...)
It must be the 'nph-*' that lets it through.  In order for that to work, we'd 
have to rename every script in here to nph-....  that'd be a pain in the neck.

I spent most of a day last week searching Apache's docs for a way to get it to 
not bother authenticating, and I couldn't find a way to do it without disabling 
it site-wide, and I have other parts of my site that use it the real way.

Sounds like the nph- trick would do it.  We could get around the filenaming thing 
as far as the users are concerned by using RewriteRule lines in the .htaccess 
file, assuming people have mod_rewrite installed.  Note that if we get into nph- 
files, then all of the CGI scripts will be required to emit full headers, and not 
just the Content-Type header...

Sounds like almost as much of a nightmare (though not quite as bad) as using 
session keys...
More thoughts....   I did find an Anonymous_UserID option for Apache that will 
allow you to let people into an authenticated area by just hitting the OK button 
on the password dialog without entering a UserID or a password.  However, if they 
do supply a username and password, it authenticates it to make sure it's them.  
Since the script could easily tell this by whether REMOTE_USER was blank or not, 
perhaps this would be the way to go.  This would necessitate placing the 
index.html file in a different directory, and explaining on the index page that 
you can just hit the OK button when prompted for a password when you go into the 
other pages (although I suppose you could mask the index.html file out of the 
authenticated zone with creative use of the .htaccess file)

Combine this with mod_auth_mysql in order to yank the passwords out of the 
existing bugzilla user database, and we have a working system.
The 'nph' prefix is because we use CGIWrap and is just because that is what
our RewriteRule for CGIWrap expects. It could be anything.
*** Bug 3614 has been marked as a duplicate of this bug. ***
*** Bug 31427 has been marked as a duplicate of this bug. ***
ok, how do you go about changing what prefix it uses?  Or make it use a 
siffix instead?  And can you do that only in the bugzilla directory, or would it 
have to be done for the whole server?  (i.e. if we told it that any script in 
the bugzilla directory should be treated as an nph script or soemthing)
Unless you use CGIwrap, then there should be nothing to worry about.
    This is still bugging me heavily. See my latest comments to bug 20122
and bug 36358 for an example. Are there any chances that this is going to
be fixed some day?
There is another thing I'm wondering about ... as I always log in from different
IP numbers, the bugzilla database must have tons of them stored for my user ID.
And if I click on "log out", I seem to only log out the current IP number. So
anybody who is using the same dialup ISP and who could grab my user ID and
password could make changes to Bugzilla on my behalf. This doesn't give me a
feeling of great security!

And it is still very annoying that I always have to load the voting page in
a different window for a workaround as I wrote in my 2000-06-15 comment.

Could someone *pleeeeeease* fix this? There have been solutions suggested here.
I don't think, bugzilla stores the IP-numbers. They are stored in client cookies
and there is only one (i.e. all previous ones are forgotten).
The IP number is not stored in the cookie.  If it were, it would be real easy to 
edit it. :)   The IP number *is* stored on the server for the IP that it sent 
each cookie to.  It only stores one IP per login ID.  As soon as you log in at a 
new IP, it forgets any other IPs you've logged in from.

For someone else to spoof you, three things have to happen:

1) They have to sniff your cookie and steal it.
2) They have to get the same IP address you were previously using
3) You would have to have not chosen the "Log Out" link in Bugzilla before
   logging out of that IP address.
Summary: Bugzilla requires new login every time hostname changes → Bugzilla requires new login every time hostname changes (ip address)
or
4) they can just sniff your network for your password when you next log in (since
   the password is sent in clear text).
*** Bug 69955 has been marked as a duplicate of this bug. ***
updating summary to reflect where the discussion on this bug was headed, and make 
it easier for people to find it.
Summary: Bugzilla requires new login every time hostname changes (ip address) → Bugzilla requires new login if IP changes or cookies off
Adding default QA contact to all open Webtools/Bugzilla bugs lacking one.
Sorry for the spam.
QA Contact: matty
*** Bug 80963 has been marked as a duplicate of this bug. ***
I did not notice this problem with redhat's bugzilla 2.8. My guess would be
either something introduced since mozilla's and redhat's bugzilla diverged, or
RH found a way around it.

is there any hope of a resolution?
2.8 was Released 11/19/99 and this bug was opened 11/26/99 so I'd say it's a
safe bet that this was an issue when RedHat forked :) My best guess is that
RedHat went for the security tradeoff that makes it easier to spoof somebody's
login.
Then perhaps it could be made a configuration option in the next release or so
of bugzilla, where the default is what it is now, but it can be made more lax,
so that people can make their own decisions as to which way to go on this?

Target Milestone: --- → Bugzilla 2.16
since there's people that have this problem on bugzilla.mozilla.org, why not make 
it an individual use preference?  Since people are responsible for their own 
accounts, if they want to take that risk, why stop them?

With the looser checks, you could still valid the IP address, but maybe just 
validate it against the subnet instead of the full IP.  I'd assume that the 
round-robin proxies are usually on the same subnet with each other, aren't they?
Priority: P3 → P1
-> New Bugzilla Product
Component: Bugzilla → User Accounts
Product: Webtools → Bugzilla
Version: other → unspecified
*** Bug 104520 has been marked as a duplicate of this bug. ***
I don't think we need this for 2.16. Anyone care enough to write a patch?

Gerv
Absolutely.  To me, this is our most needed feature.
Taking this.  I have a patch that allows you to specify an IP mask, but it
requires installation of the Net:IPv4Addr module and I'm still waiting for it to
be installed on landfill.
Assignee: tara → matty
QA Contact: matty → jake
Status: NEW → ASSIGNED
There's nothing in the works on this yet, and it'll likely require more review
than we have time for between now and 2.16, so I'm pushing to 2.18
Target Milestone: Bugzilla 2.16 → Bugzilla 2.18
Attached patch Rough, likely bitrotten patch. (obsolete) — Splinter Review
bbaetz wanted this attached.

In particular, he indicated this stuff should perhaps be fixed to always use
IPs, which I thought it always did, but apparently not, so this patch might
break then.

Also it doesn't support IPv6 at all.  The current model of specifying 24 bits,
ie a mask of 255.255.255.0 would not work for IPv6.  Perhaps it should be
changed to specify the 8 bits that are a part of the network.
Attachment #72962 - Flags: review-
Comment on attachment 72962 [details] [diff] [review]
Rough, likely bitrotten patch.

The check in globals.pl won't match if there is more than one entry for the
user in teh logincookies table, since you have removed teh where. I was just
going to store the network address, and do an update in userprefs to keep the
current login session working.

We can also probably avoid the dependance on another module unless we want to
handle ipv6
I'm going to attach a patch which works. It requires Net::IPv4 - if thats likely
to be a problem I can probably redo the required function myself without too
much trouble.

I'm not very happy with the way the userprefs screen looks. Matty's patch moved
it to a separate screen, but I don't think a screen with one option is worth it.
I'll take any improvments on this (which is why this patch is v0.9, not v1.0)

Resetting to 2.16 for consideration, since it is a Frequently Requested Feature.
It is very late for this though, so feel free to bump it.
Keywords: patch, review
Target Milestone: Bugzilla 2.18 → Bugzilla 2.16
Attached patch patch v0.9 (obsolete) — Splinter Review
Attachment #72962 - Attachment is obsolete: true
And my comment, which seems to have been misplaced:

So, here we go. We now store the network address in the logincookies table.
Since there isn't an sql function for doing this, this means that my query will
return only one value. I had to split up the query a bit to do this, since I
need the profiles table before I check the logincookies table.

Theres a param for the minimum value for the netmask, defaulting to 24. Setting
to 32 hides this option. Setting this value updates the profiles table with the
new info - should it instead keep it until the user changes it, and then CGI.pl
would just take the min of the param and the value from the db?

Also, I had to comment out the eval in get_netaddr, else I got syntax errors.
Any ideas? It is needed for dealing with addresses Net::IPv4 can't parse.
Comment on attachment 74508 [details] [diff] [review]
patch v0.9

>Index: CGI.pl

>+sub get_netaddr($$) {
>+    my ($addr, $mask) = (@_);
>+
>+    use Net::IPv4Addr qw(ipv4_network);
>+    
>+    my $ipaddr;
>+    #eval {
>+        ($ipaddr) = ipv4_network($addr, $mask);
>+    #}
>+    # Some addresses (ie ipv6) won't work. Thats ok - we'll
>+    # just ignore the user's netmask in that case and match on
>+    # the entire string
>+    # Warn to help track down issues with this
>+    if ($@) {
>+        warn $@;
>+        $ipaddr = $addr if $@;
>+    }
>+    return $ipaddr;
>+}

Just to fill me in here...  we have the eval commented out....
does ipv4_network() set $@ when it fails anyway?  If so, this is going to
confuse people (like me) to see that partially commented out.  Let's just
get rid of it if that's the case.
BEcause it didn't compile - see my comments.

Hixie pointed out that I was missing a ; at the end of that block. Adding that
makes it work correctly. Without the eval, it will throw an exception, and die,
on invalid input.
I also reminded bbaetz about my suggestion earlier in this bug of using HTTP
auth. That is, after all, the whole point of HTTP auth.
Making it use HTTP authentication will be a humungous amount of work (about on
order with the Templatization patches with how much it'll change the face of the
cvs repository).  See comment #30.
And I reminded hixie that while you can use a modrewrite rule to get the
password sent anyway (which he pointed out to me - really cool; I didn't know
you could do that), that won't work for IIS. And you'd be sending the password
in plain text on every page load. Although we do that currently anyway, so...
except we're not sending it in cleartext on every page load.  We're sending a
session cookie with every page load that is tied to the IP address (or a subnet
with this most recent patch).  The password is only sent cleartext when you
actually log in.
And I pointed out that this just gave us a false sense of security. There is
nothing secure about our current system, as we all know. Given that it is
completely insecure, I see no reason to not use another completely insecure but
at least more correct solution.

And if anyone develops a well-supported and secure alternative to HTTP Basic or
HTTP Digest, then we can use it.
Saying "the fact that the password only appears 1/20th as much gives us a false
sense of security so we shouldn't do this" is kind of like saying "the fact that
a password makes it harder to login gives us a false sense of security because a
password can be guessed so we shouldn't use them".
Passwords increase the security by factors of millions.
The factor isn't the issue here.  The point is that security is achieved through
multiple layers, each decreasing the likelihood of a break in.  Whether they
decrease it a little or a lot is really not relevant.  The only thing that
should prevent something that increases security being done is if it prevents
something else that increases it more.

I've heard this "false sense of security" argument before and it can be used to
nix just about anything if you want it to.
Ok. This is the most relevant place that I have found for this so here goes...

PLEASE. For the love of god. Fix this. If there's no patch that people want to
use then can we at least have bugzilla.mozilla.org respond on a port other then
80 (like 81) aswell as port 80 so that those of us stuck behind a rotating proxy
can use it without the frustration of having to log in each and every time we
make some kind of change to a bug.

And yes I do appreciate the security behind this. I just want to be able to use
bugzilla in a happy mood rather then in a frustrated mood that makes me want to
use it as little as possible. :/ Imagine having to go through unconfirms or
adding your expiriences to bugs talked about on #mozillazine and having to log
in each time you add a comment, cast a vote, add yourself to the CC list or
whatever. It can get nasty.

I just had to login again to cast my vote. Now I'll be doing it one more time to
add this comment.

So PLEASE PLEASE PLEASE can someone with the power to do something do something?
Please? :/
Attached patch v1 (obsolete) — Splinter Review
Adds the missing ;
Attachment #74508 - Attachment is obsolete: true
*** Bug 132914 has been marked as a duplicate of this bug. ***
I'll take this, because I wrote the current patch.

If this doesn't get reviewed RSN, then it should be pushed off.
Assignee: matty → bbaetz
Status: ASSIGNED → NEW
QA Contact: jake → matty
Whiteboard: possible solution suggested
Comment on attachment 76929 [details] [diff] [review]
v1

>+    use Net::IPv4Addr qw(ipv4_network);

<sigh> If we really must.

>+    # Warn to help track down issues with this
>+    if ($@) {
>+        warn $@;
>+        $ipaddr = $addr if $@;

Use ThrowCodeError() instead of warn.

>+       SendSQL("SELECT netmask FROM profiles WHERE login_name = " . 
>+               SqlQuote($enteredlogin));

This is _so_ geeky. Does the netmask have to be configurable
per-user? Can't it just be an admin option? I can see that
everyone having the same value might be less secure, but I
still think this is not something we should be exposing.

If it has to be configurable per user, could we just have a 
checkbox - "allow login from your entire network",
which sets it to /24 or something? 

And really, we don't need to have a param to enable or disable this - 
admins can just remove it from the template. (Yes, clever people
who read the source could still hack the HTML, but is that really
something to care about?)

Gerv
Attachment #76929 - Flags: review-
>(From update of attachment 76929 [details] [diff] [review])
>>+    use Net::IPv4Addr qw(ipv4_network);

><sigh> If we really must.

We don't have to, since I think that the server will always give us
dotted quad form, Its not required to by the cgi spec, though.

>+    # Warn to help track down issues with this
>+    if ($@) {
>+        warn $@;
>+        $ipaddr = $addr if $@;

Use ThrowCodeError() instead of warn.

Its not an unsolvable eror though - we can ignore it and things will still work.
It also makes the netmask stuff easier.

But yeah, you're propbably right.

>>+       SendSQL("SELECT netmask FROM profiles WHERE login_name = " . 
>>+               SqlQuote($enteredlogin));

>This is _so_ geeky. Does the netmask have to be configurable
>per-user? Can't it just be an admin option? I can see that
>everyone having the same value might be less secure, but I
>still think this is not something we should be exposing.
>
>If it has to be configurable per user, could we just have a 
>checkbox - "allow login from your entire network",
>which sets it to /24 or something? 

Thats one option. It has to be per user though - most won't want it/need it, and
there is a security penalty for enabling it.

But yes, I see your point.

>And really, we don't need to have a param to enable or disable this - 
>admins can just remove it from the template. (Yes, clever people
>who read the source could still hack the HTML, but is that really
>something to care about?)

YES! Of course we care about people sending data to override admin specified
security settings!!!
OK, fair enough on that last point :-) 

But can we do the checkbox thing? It's so much more user-friendly. Also, you
mentioned the possibility of rewriting the IPv4::Foo code to avoid Yet Another
Module Dependency. Are either of that, or the "just trust the server" option,
possible?

Gerv
Well, I can make sure that we work with apache in that case.... I mean, I could
rewrite those functions, but since they already exist for us, I don't see the
point. And we will need an external module for ipv6, unless you want to rewrite
Math::Base85 (No, I'm not joking, unofrtunately)
OK, forget the rewriting. But do the checkbox :-) (The param to turn this on can
be the number the checkbox reduces the mask to; i.e. 32 turns the thing off
completely, and any lower number allows that much loosening.)

Gerv
This has missed 2.16

->2.18, will tidy up sop that it can be applied after we release
Target Milestone: Bugzilla 2.16 → Bugzilla 2.18
BTW, if you go to:
http://www.blackcatnetworks.co.uk/cgi-bin/sqwebmail
you'll see that the developers of SQWebmail obviously hit the same problem, and
got around it with a checkbox at login. 

We could do that - instead of this being a per-account pref, have a checkbox at
login, and store whether we were being "loose" or not in the logincookies table.
This way, someone can be secure from work, where they have a fixed IP, but be
loose at home, because their home ISP has a round-robin proxy.

I suspect this would actually simplify the code, as well.

Gerv
I have this sort of vague idea to move the login cookies into the tokens table
at some point, just to remove redundancy.

Anyway, I'm unlikley to get to this withint the next couple of weeks, due to
other  commitments. If someone else wants to do this, be my guest.
Hmm, Brad, that's not a bad idea at all (moving it into the tokens table).  That
would satisfy the complains about the sequential session tokens, too, since we
could just use the "generatetoken" routine to create them.
Well, I can just try to match against either the IP _or_ the net addr when
logging in. Actually, thats simpler, I think, unless we want to encode what type
of login we used, and use an extra bit on the login cookies table.

SQmail appears to be an all or nothing affair. I'd like to have a checkbox for
the same, but still have the 'what netmask to use' thing be a param.

Or maybe a bit on the logincookies table would be the better way, anyway. Hmm -
I'll have to think about that.
Attached patch v2 (obsolete) — Splinter Review
OK, I prefer gerv's solution

This now doesn't have a schema change at all, and so the code is nicer, too.
The lack of flexability is probably irrelevent.
Attachment #76929 - Attachment is obsolete: true
The V2 patch provided is not against the latest build, I have just done the work
necesaary to make it work against 2.16rc1. If anyone wnats it please mail me (I
can't attach it because I suffer from this bug! (It's too long to put in comments)
Attached patch v2, take 2 (obsolete) — Splinter Review
The version I _made_ was agianst cvs. Unfortunately, that wasn't the version I
attached. Try this, instead, which has the different approach Gerv mentioned.
Attachment #83170 - Attachment is obsolete: true
As a temporary fix to this bug (for those behind proxies with changing IP
numbers) I've investigated the possibility of using HTTP_X_FORWARDED_FOR rather
than REMOTE_HOST if it is available. This seems to work OK, and fixes a few
problems I had behind a proxy. If anyone's interested in a patch, tell me.

Although, of course, this isn't a final solution - it doesn't solve the problem
of people who have *real* rotating IP numbers (e.g. auto-dialup/down). It just
makes things work a better for some people until a proper fix is applied.
Yeah, but thats really really trivially forgable. I think that the attached
patch should just be reviewed, hint, hint...

justdave? matty?
*** Bug 154644 has been marked as a duplicate of this bug. ***
Comment on attachment 83243 [details] [diff] [review]
v2, take 2

>+    # Some addresses (ie ipv6) won't work. Thats ok - we'll
>+    # just ignore the user's netmask in that case and match on
>+    # the entire string
>+    # Warn to help track down issues with this
>+    if ($@) {
>+        warn $@;
>+        $ipaddr = $addr if $@;
>+    }
>+    return $ipaddr;

We need a log, and a logging API.

Educate me: why is the seconf "if $@" necessary?

> sub CheckEmailSyntax {
>     my ($addr) = (@_);
>     my $match = Param('emailregexp');
>-    if ($addr !~ /$match/ || $addr =~ /[\\\(\)<>&,;:"\[\] \t\r\n]/) {
>+    if ($addr !~ /$match/ || $addr =~ /[\\\(\)<>&,;:\"\[\] \t\r\n]/) {

Is this a separate bug you are fixing?

>@@ -826,6 +853,18 @@
> <tr>
> <td align=right><b>Password:</b></td>
> <td><input type=password size=35 name=Bugzilla_password></td>";
>+        }
>+        if (Param("loginnetmask") < 32) {
>+            print "
>+</tr>
>+<tr>
>+<td align=\"right\">
>+  <b>Use this session<br>from different IPs:</b>
>+</td>
>+<td>
>+  <input type=\"checkbox\" name=\"Bugzilla_netlogin\">
>+  (Using this option decreases security)
>+</td>";
>         }
>         print "
> </tr>

Surely this is now templatised, right?

Also, "Using this option makes it easier for someone to impersonate you."

>-    "File::Spec"   => "0.82"
>+    "File::Spec"   => "0.82",
>+    "Net::IPv4Addr" => "0.10"

<sigh> Does it ship with Perl normally?

>+        return "an IPv4 netmask must be between 0 and 32";

I thought a netmask was specified as e.g. 255.255.255.0 . Why aren't we using
that form?

>+DefParam("loginnetmask",
>+         "The netmask used if a user chooses to allow a login to be valid
>+          from more than a single IP. Setting this to 32 disables this
>+          feature.",
>+         "t",
>+         '32',
>+         \&check_netmask);

I think allowing the admin to set any value just adds complexity. It's also
quite hard to explain exactly what it does, and "32 to disable" isn't 
particularly intuitive. Do we envisage anyone using any values other than 24 
and 32? 

Gerv
Attachment #83243 - Flags: review-
> We need a log, and a logging API.

That may be true, but thats well outside the scope of this bug. I'll just remove
the |warn| if you want to insist on this...

> Educate me: why is the seconf "if $@" necessary?

Its not - remnants of before the eval was added.

> Is this a separate bug you are fixing?

Oops, yeah - xemacs stuffs up the colors. I should file a separate bug on that..

> Surely this is now templatised, right?

Nope, we never templatised the login screen...

No, Net::IPv4Addr doesn't ship normally. I can remove that dependency if we
assume that Ips are always in dotted waud form, which probably isn't that
unreasonable.

> I thought a netmask was specified as e.g. 255.255.255.0 . Why aren't we using
> that form?

Its more complicated. Teh number of one bits is all you require, anyway

> Do we envisage anyone using any values other than 24 and 32? 

Not often, no, but its possible that an admin may use something more restrictive.
Keywords: review
Attached patch v3 (obsolete) — Splinter Review
OK, now try this one. I've removed the dependency on Net::IPv4Addr, mainly
because I discovered that it only handled dotted-quad addresses anyway.

There isn't any schema change for this. Should I have one? There can't be any
false matches, because anyone whose 'real' IP was equal to the masked IP would
have a match on the masked entry anyway, so there isn't a security issue there.


Thoughts?
Attachment #83243 - Attachment is obsolete: true
Attached patch v4Splinter Review
OK, I've inverted the login for the checkbox, to make the words slightly less
geeky. I could s/IP/computer, but thats not accurate, because of
NAT/proxies/etc.
Attachment #101059 - Attachment is obsolete: true
yikes

Enabling this really leads to crummy security.   Could this at least use
something less predictable than the existing cookie?

Well, you could argue that it provides greater security, by sending the password
arround in plain text less often

That said, this is optional, so people who have their own permenant IP wouldn't
enable this, and so wouldn't lose anything, and those who share via
NAT/transparent proxies/etc already appear to have the same IP as other users
anyway.

That said, there is a separate bug on moving the logincookies stuff to the token
table.
Comment on attachment 101087 [details] [diff] [review]
v4

r=joel if you update the text at the param to indicate that numbers less than
32 can make it easier for someone else to impersonate a logged-in user.
Attachment #101087 - Flags: review+
OK, checked in. This bug didn't deal with cookies, so I removed that bit from
the title. If someone wants to have bz deal with that, please file a separate bug.
Status: NEW → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Summary: Bugzilla requires new login if IP changes or cookies off → Bugzilla requires new login if IP changes
This doesn't seem to work here. Either with the checkbox unchecked or checked,
I'm  still asked to relogin on IP change.
To reproduce :
- login to bugzilla
- let the browser open and disconnect+reconnect (by browsing to the router page)
- return to bugzilla
Cookies are of course enabled (there is no problem if I close the browser
provided that my IP doesn't change).
QA Contact: matty_is_a_geek → default-qa
You need to log in before you can comment on or make changes to this bug.