Closed
Bug 20122
Opened 25 years ago
Closed 22 years ago
Bugzilla requires new login if IP changes
Categories
(Bugzilla :: User Accounts, defect, P1)
Bugzilla
User Accounts
Tracking
()
RESOLVED
FIXED
Bugzilla 2.18
People
(Reporter: dbaron, Assigned: bbaetz)
References
Details
Attachments
(1 file, 6 obsolete files)
(deleted),
patch
|
bugreport
:
review+
|
Details | Diff | Splinter Review |
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.
Comment 1•25 years ago
|
||
It's a security tradeoff. If I don't do this, then it's much easier to spoof
someone's login.
Reporter | ||
Comment 2•25 years ago
|
||
OK. I suspected that was the reason...
Updated•25 years ago
|
Summary: Bugzilla requires new login everytime hostname changes → Bugzilla requires new login every time hostname changes
Comment 3•25 years ago
|
||
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.
Reporter | ||
Comment 4•25 years ago
|
||
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.
Comment 5•25 years ago
|
||
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!)
Updated•25 years ago
|
Status: NEW → ASSIGNED
Comment 7•25 years ago
|
||
I have the same problem with my autodialup/-down set at 1 min.
Comment 9•25 years ago
|
||
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?
Comment 10•25 years ago
|
||
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
Comment 12•25 years ago
|
||
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
Comment 13•24 years ago
|
||
> 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.
Comment 14•24 years ago
|
||
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
Comment 15•24 years ago
|
||
> 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.
Comment 16•24 years ago
|
||
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.
Updated•24 years ago
|
OS: Windows 95 → All
Hardware: PC → All
Comment 17•24 years ago
|
||
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!
Comment 18•24 years ago
|
||
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.
Comment 19•24 years ago
|
||
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?
Comment 20•24 years ago
|
||
>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.
Comment 21•24 years ago
|
||
Assuming it would work with Apache, I'd value the user's problems over the
choices of the admin (but I am biased :) ).
Comment 22•24 years ago
|
||
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/
Comment 23•24 years ago
|
||
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
Comment 24•24 years ago
|
||
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.
Comment 25•24 years ago
|
||
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.
Comment 26•24 years ago
|
||
*** Bug 38831 has been marked as a duplicate of this bug. ***
Comment 27•24 years ago
|
||
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...
Comment 28•24 years ago
|
||
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 [...].
Comment 29•24 years ago
|
||
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...)
Comment 30•24 years ago
|
||
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...
Comment 31•24 years ago
|
||
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.
Comment 32•24 years ago
|
||
The 'nph' prefix is because we use CGIWrap and is just because that is what
our RewriteRule for CGIWrap expects. It could be anything.
Comment 33•24 years ago
|
||
*** Bug 3614 has been marked as a duplicate of this bug. ***
Comment 34•24 years ago
|
||
*** Bug 31427 has been marked as a duplicate of this bug. ***
Comment 35•24 years ago
|
||
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)
Comment 36•24 years ago
|
||
Unless you use CGIwrap, then there should be nothing to worry about.
Comment 37•24 years ago
|
||
Comment 38•24 years ago
|
||
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.
Comment 39•24 years ago
|
||
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).
Comment 40•24 years ago
|
||
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.
Updated•24 years ago
|
Summary: Bugzilla requires new login every time hostname changes → Bugzilla requires new login every time hostname changes (ip address)
Comment 41•24 years ago
|
||
or
4) they can just sniff your network for your password when you next log in (since
the password is sent in clear text).
Comment 42•24 years ago
|
||
*** Bug 69955 has been marked as a duplicate of this bug. ***
Comment 43•24 years ago
|
||
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
Comment 44•24 years ago
|
||
Adding default QA contact to all open Webtools/Bugzilla bugs lacking one.
Sorry for the spam.
QA Contact: matty
Comment 45•23 years ago
|
||
*** Bug 80963 has been marked as a duplicate of this bug. ***
Comment 46•23 years ago
|
||
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?
Comment 47•23 years ago
|
||
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.
Comment 48•23 years ago
|
||
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?
Updated•23 years ago
|
Target Milestone: --- → Bugzilla 2.16
Comment 49•23 years ago
|
||
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?
Updated•23 years ago
|
Priority: P3 → P1
Comment 50•23 years ago
|
||
-> New Bugzilla Product
Component: Bugzilla → User Accounts
Product: Webtools → Bugzilla
Version: other → unspecified
Comment 51•23 years ago
|
||
*** Bug 104520 has been marked as a duplicate of this bug. ***
Comment 52•23 years ago
|
||
I don't think we need this for 2.16. Anyone care enough to write a patch?
Gerv
Comment 53•23 years ago
|
||
Absolutely. To me, this is our most needed feature.
Comment 54•23 years ago
|
||
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
Updated•23 years ago
|
Status: NEW → ASSIGNED
Comment 55•23 years ago
|
||
Comment 56•23 years ago
|
||
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
Comment 57•23 years ago
|
||
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.
Assignee | ||
Updated•23 years ago
|
Attachment #72962 -
Flags: review-
Assignee | ||
Comment 58•23 years ago
|
||
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
Assignee | ||
Comment 59•23 years ago
|
||
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.
Assignee | ||
Comment 60•23 years ago
|
||
Attachment #72962 -
Attachment is obsolete: true
Assignee | ||
Comment 61•23 years ago
|
||
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 62•23 years ago
|
||
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.
Assignee | ||
Comment 63•23 years ago
|
||
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.
Comment 64•23 years ago
|
||
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.
Comment 65•23 years ago
|
||
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.
Assignee | ||
Comment 66•23 years ago
|
||
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...
Comment 67•23 years ago
|
||
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.
Comment 68•23 years ago
|
||
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.
Comment 69•23 years ago
|
||
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".
Comment 70•23 years ago
|
||
Passwords increase the security by factors of millions.
Comment 71•23 years ago
|
||
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.
Comment 72•23 years ago
|
||
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? :/
Assignee | ||
Comment 73•23 years ago
|
||
Adds the missing ;
Attachment #74508 -
Attachment is obsolete: true
Comment 74•23 years ago
|
||
*** Bug 132914 has been marked as a duplicate of this bug. ***
Assignee | ||
Comment 75•23 years ago
|
||
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 76•23 years ago
|
||
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-
Assignee | ||
Comment 77•23 years ago
|
||
>(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!!!
Comment 78•23 years ago
|
||
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
Assignee | ||
Comment 79•23 years ago
|
||
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)
Comment 80•23 years ago
|
||
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
Assignee | ||
Comment 81•23 years ago
|
||
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
Comment 82•23 years ago
|
||
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
Assignee | ||
Comment 83•23 years ago
|
||
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.
Comment 84•23 years ago
|
||
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.
Assignee | ||
Comment 85•22 years ago
|
||
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.
Assignee | ||
Comment 86•22 years ago
|
||
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
Comment 87•22 years ago
|
||
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)
Assignee | ||
Comment 88•22 years ago
|
||
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
Comment 89•22 years ago
|
||
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.
Assignee | ||
Comment 90•22 years ago
|
||
Yeah, but thats really really trivially forgable. I think that the attached
patch should just be reviewed, hint, hint...
justdave? matty?
Comment 91•22 years ago
|
||
*** Bug 154644 has been marked as a duplicate of this bug. ***
Comment 92•22 years ago
|
||
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-
Assignee | ||
Comment 93•22 years ago
|
||
> 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.
Assignee | ||
Comment 94•22 years ago
|
||
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
Assignee | ||
Comment 95•22 years ago
|
||
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
Comment 96•22 years ago
|
||
yikes
Enabling this really leads to crummy security. Could this at least use
something less predictable than the existing cookie?
Assignee | ||
Comment 97•22 years ago
|
||
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 98•22 years ago
|
||
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+
Assignee | ||
Comment 99•22 years ago
|
||
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: 22 years ago
Resolution: --- → FIXED
Summary: Bugzilla requires new login if IP changes or cookies off → Bugzilla requires new login if IP changes
Comment 100•19 years ago
|
||
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).
Updated•12 years ago
|
QA Contact: matty_is_a_geek → default-qa
You need to log in
before you can comment on or make changes to this bug.
Description
•