Closed
Bug 139930
Opened 23 years ago
Closed 23 years ago
checksetup.pl fails if data/params doesn't exist
Categories
(Bugzilla :: Installation & Upgrading, defect)
Tracking
()
RESOLVED
FIXED
Bugzilla 2.16
People
(Reporter: bbaetz, Assigned: zach)
Details
Attachments
(1 file, 3 obsolete files)
2.34 KB,
patch
|
bbaetz
:
review+
gerv
:
review+
|
Details | Diff | Splinter Review |
Bug 126571 went in, but that requries data/parmas without checking that it
exists. that block should be wrapped in an |if (-e 'data/params')| check
Note that if it does exist, the graphviz check will have brought it in earlier.
Thats a separate very minor optimisation issue, though
2.16 blocker, since we do want new installations to work
Reporter | ||
Updated•23 years ago
|
Target Milestone: --- → Bugzilla 2.16
Comment 1•23 years ago
|
||
Assignee | ||
Comment 2•23 years ago
|
||
Comment on attachment 80893 [details] [diff] [review]
patch v1: rectifies situation
r1=zach
Attachment #80893 -
Flags: review+
Reporter | ||
Comment 3•23 years ago
|
||
Comment on attachment 80893 [details] [diff] [review]
patch v1: rectifies situation
r=bbaetz, obvious
Attachment #80893 -
Flags: review+
Reporter | ||
Comment 4•23 years ago
|
||
This is necessary, but not sufficient - checksetup.pl now calls WriteParams,
which runs as the user running the script. So the params file is now:
-rw-rw---- 1 root root 7397 Apr 25 21:46 data/params
and since my apache doesn't run as root, we fail.
Comment 5•23 years ago
|
||
Comment on attachment 80893 [details] [diff] [review]
patch v1: rectifies situation
Marking needs-work per comment #4.
Attachment #80893 -
Flags: review-
Comment 6•23 years ago
|
||
This patch includes patch 80893 and fixes additionally the permissions for the
group of data/params.
Comment 7•23 years ago
|
||
Comment on attachment 80981 [details] [diff] [review]
patch v2: rectifies situation + change group
>Index: checksetup.pl
>+ my $gid = (split " ", $()[0];
>+ fixPerms("data/params",$<,$gid,017);
This needs to be 011 or the web server user won't be able
to read from or write to the file.
Attachment #80981 -
Flags: review-
Comment 8•23 years ago
|
||
Attachment #80893 -
Attachment is obsolete: true
Attachment #80981 -
Attachment is obsolete: true
Comment 9•23 years ago
|
||
Attachment #81050 -
Attachment is obsolete: true
Reporter | ||
Comment 10•23 years ago
|
||
Comment on attachment 81058 [details] [diff] [review]
patch v4: puts permissions fixing in the right place
r=bbaetz Looks fine, not in a position to test now, though.
Attachment #81058 -
Flags: review+
Updated•23 years ago
|
Comment 11•23 years ago
|
||
> >+ my $gid = (split " ", $()[0];
> >+ fixPerms("data/params",$<,$gid,017);
>
> This needs to be 011 or the web server user won't be able
> to read from or write to the file.
Is than this ok? (attachment 81058 [details] [diff] [review])
| + fixPerms('data/params', $<, $webservergid, 017);
| @@ -875,6 +888,7 @@
| + fixPerms('data/params', $<, $gid, 011);
Comment 12•23 years ago
|
||
> >+ my $gid = (split " ", $()[0];
> >+ fixPerms("data/params",$<,$gid,017);
>
> This needs to be 011 or the web server user won't be able
> to read from or write to the file.
Well I get:
017: -rw-rw---- root www-data
011: -rw-rw-rw- root www-data
I actually prefer 017 to a world writable file!!
Reporter | ||
Comment 13•23 years ago
|
||
The entire data directory is world writable if you don't have a webserver group
enabled. So technically, we could get away with the file only being world
readable, since the webserver will be able to overwrite the file anyway. That
doens't affect the security, of course.
This used to be +t, but that broke stuff - see bug 107513.
You really should use a webservergroup on any real installation, though, for
security reasons. Maybe we should document this better, along the lines of "YOU
REALLY REALLY REALLY WANT TO USE THIS OPTION!!!!", and say this in checksetup.
OTOH, if your webserver does run as root, then thats not needed, I suppose.
However, if your webserver does run as root for all cgi scripts, then no matter
what the permissions are, anyone could hack up a quick cgi script to do whatever
they wanted. If it only runs as root for some of them, that implies that you
have acccess to the root account, and can set the webservergroup to www-data and
run checksetup as root.
Or was the ls -l output from you running checksetup as root?
gerv, justdave, myk: comments on the above?
Comment 14•23 years ago
|
||
> The entire data directory is world writable if you don't have a webserver
> group enabled. So technically, we could get away with the file only being
> world readable, since the webserver will be able to overwrite the file anyway.
> That doens't affect the security, of course.
Ok. I've missed that such permissions are set in general. But I still don't like
the idea of having world writable files. I think a warning should be printed
every time ./checksetup is running. (And of cause I have my webservergroup set
-- I observed this only when testing this patch and didn't know/realise that
this was intentional.)
Comment 15•23 years ago
|
||
Warnings are good, filed as bug 140355. Now can someone give this bug a second
review?
Comment 16•23 years ago
|
||
Comment on attachment 81058 [details] [diff] [review]
patch v4: puts permissions fixing in the right place
r=gerv.
Gerv
Attachment #81058 -
Flags: review+
Comment 17•23 years ago
|
||
Checking in checksetup.pl;
/cvsroot/mozilla/webtools/bugzilla/checksetup.pl,v <-- checksetup.pl
new revision: 1.138; previous revision: 1.137
done
Status: NEW → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
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
•