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...

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!)

*** 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 .)

I'm really tired of this bug.

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.

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.

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.

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.

Adding default QA contact to all open Webtools/Bugzilla bugs lacking one. Sorry for the spam.

*** 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?

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?

-> New Bugzilla Product

*** 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.

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

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.

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.

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? :/

Adds the missing ;

*** 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.

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

>(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

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.

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.

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)

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.

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

> 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.

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?

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.

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.

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.

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).