Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
6621045:
Add cmdline switch for folks that really did want to run as root.... (Closed)
|
Created:
9 years, 9 months ago by Mike Belshe Modified:
9 years, 5 months ago Reviewers:
mbelshe, Lei Zhang, antoniop, Elliot Glaysher, Evan Martin, Evan Stade, a10n3.s7r1k3r, agl, pedro.quados CC:
chromium-reviews Base URL:
svn://chrome-svn/chrome/trunk/src/ Visibility:
Public. |
DescriptionAdd cmdline switch for folks that really did want to run as root.
BUG=none
TEST=none
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=77292
Patch Set 1 #Patch Set 2 : '' #Patch Set 3 : '' #
Total comments: 2
Patch Set 4 : '' #Messages
Total messages: 22 (0 generated)
+evanm,+estade,+thestig I don't know how to feel about this. The second we give users a way out of this, they will use it and they will screw up their profile.
What is the use case?
On 2011/03/07 19:33:06, Evan Martin wrote: > What is the use case? (We could extend the original logic to prevent "sudo" but not running as the real root user... we're only trying to protect a plain user profile, so as long as $HOME points to the right place we're ok -- though we could check that using uid vs euid or something.)
The use case: I've been using this on the daily perfbots for some time. As of last week, the perfbots all mysteriously died (they run headless, so it took a while to figure out the cause) Why run as root? Well, we run a number of network utilities through the test harness which muck with DNS and ipfw settings, which require root. Could we rewrite the harness to run chrome as a different user? I suppose. My test always uses --user-data-dir. Would you be okay with allowing running as root when the user provides their own --user-data-dir? I would prefer that. Mike On Mon, Mar 7, 2011 at 11:37 AM, <evan@chromium.org> wrote: > On 2011/03/07 19:33:06, Evan Martin wrote: > >> What is the use case? >> > > (We could extend the original logic to prevent "sudo" but not running as > the > real root user... we're only trying to protect a plain user profile, so as > long > as $HOME points to the right place we're ok -- though we could check that > using > uid vs euid or something.) > > > http://codereview.chromium.org/6621045/ >
On Mon, Mar 7, 2011 at 12:37 PM, Mike Belshe <mbelshe@google.com> wrote: > Would you be okay with allowing running as root when the user provides their > own --user-data-dir? I would prefer that. That's a good idea. That would keep users from destroying their profiles, and would allow users to override the run as root protection in a way that will probably not destroy their profiles. If you redo the patch this way, could you also update the IDS_REFUSE_TO_RUN_AS_ROOT_2 string? -- Elliot
On Mon, Mar 7, 2011 at 12:47 PM, Elliot Glaysher (Chromium) < erg@chromium.org> wrote: > On Mon, Mar 7, 2011 at 12:37 PM, Mike Belshe <mbelshe@google.com> wrote: > > Would you be okay with allowing running as root when the user provides > their > > own --user-data-dir? I would prefer that. > > That's a good idea. That would keep users from destroying their > profiles, and would allow users to override the run as root protection > in a way that will probably not destroy their profiles. > > If you redo the patch this way, could you also update the > IDS_REFUSE_TO_RUN_AS_ROOT_2 string? > Do you like this: "To run Google Chrome as root, you must specify an alternate location for your profile using the command line option '--user-data-dir=<profile directory>'." Mike
On Mon, Mar 7, 2011 at 12:51 PM, Mike Belshe <mbelshe@google.com> wrote: > Do you like this: > "To run Google Chrome as root, you must specify an alternate location for > your profile using the command line option '--user-data-dir=<profile > directory>'." Yeah, that sounds good. -- Elliot
I fear that it will just make people who run "sudo google-chrome" then manually enter in their user's user-data-dir. :( Can't we allow it to run when you've logged in as root (presumably as the bots do) and disallow it when you're using sudo?
On Mon, Mar 7, 2011 at 1:49 PM, <evan@chromium.org> wrote: > I fear that it will just make people who run "sudo google-chrome" then > manually > enter in their user's user-data-dir. :( > This message will show every time they start chrome, so the annoyance factor will be large. Why is this such a problem? Why not detect when we have permissions problems on the profile and properly alert for that? ("Looks like you ran chrome as root in the past, and now I can't open my config! Please insert quarter to play again.") > > Can't we allow it to run when you've logged in as root (presumably as the > bots > do) and disallow it when you're using sudo? I run it through sudo all the time for convenience to debug (in fact, I sort of had to that to debug this). If I made this change, debugging this problem would be virtually impossible, because to login as root, I'd have to be on the physical machine - take a monitor to the lab, install it on the failing machine, login as root, reproduce, fix, etc.... Mike > > > http://codereview.chromium.org/6621045/ >
On Mon, Mar 7, 2011 at 3:09 PM, Mike Belshe <mbelshe@google.com> wrote: > On Mon, Mar 7, 2011 at 1:49 PM, <evan@chromium.org> wrote: > >> I fear that it will just make people who run "sudo google-chrome" then >> manually >> enter in their user's user-data-dir. :( >> > > This message will show every time they start chrome, so the annoyance > factor will be large. > Nevermind - thats not true :-) And if it were true, it would break my tests!! :-) Mike > > Why is this such a problem? Why not detect when we have permissions > problems on the profile and properly alert for that? ("Looks like you ran > chrome as root in the past, and now I can't open my config! Please insert > quarter to play again.") > > > >> >> Can't we allow it to run when you've logged in as root (presumably as the >> bots >> do) and disallow it when you're using sudo? > > > I run it through sudo all the time for convenience to debug (in fact, I > sort of had to that to debug this). If I made this change, debugging this > problem would be virtually impossible, because to login as root, I'd have to > be on the physical machine - take a monitor to the lab, install it on the > failing machine, login as root, reproduce, fix, etc.... > > Mike > > >> >> >> http://codereview.chromium.org/6621045/ >> > >
OK - here is a version which keys off of a user-data-dir being specified. I changed the text of the error message and also drop a log into the chrome_debug.log.
LGTM if you move this. Evan, do you think this is OK? http://codereview.chromium.org/6621045/diff/5003/chrome/browser/browser_main_... File chrome/browser/browser_main_gtk.cc (right): http://codereview.chromium.org/6621045/diff/5003/chrome/browser/browser_main_... chrome/browser/browser_main_gtk.cc:82: return; You should move this above the gtk initialization so the library isn't initialized twice.
http://codereview.chromium.org/6621045/diff/5003/chrome/browser/browser_main_... File chrome/browser/browser_main_gtk.cc (right): http://codereview.chromium.org/6621045/diff/5003/chrome/browser/browser_main_... chrome/browser/browser_main_gtk.cc:82: return; On 2011/03/07 23:40:16, Elliot Glaysher wrote: > You should move this above the gtk initialization so the library isn't > initialized twice. Done.
I think the logic of this function is backwards... Shouldn't it be: if --user_data_dir is given, don't exit. I'm seeing the opposite behavior.
Yes indeed! Sorry about that. We had noticed this before, and I thought a fix went in, but I think we overlooked it. Feel free to patch or i will do so in the morning. On Mon, Apr 25, 2011 at 5:22 PM, <antoniop@google.com> wrote: > I think the logic of this function is backwards... > > Shouldn't it be: if --user_data_dir is given, don't exit. I'm seeing the > opposite behavior. > > http://codereview.chromium.org/6621045/ >
Thanks for getting back to me! I don't know the first thing about your code tree. I'm sure it would be alot easier for you to zap that not operator. Thanks! --tony On Tue, Apr 26, 2011 at 12:29 AM, Mike Belshe <mbelshe@google.com> wrote: > Yes indeed! Sorry about that. We had noticed this before, and I thought a > fix went in, but I think we overlooked it. > Feel free to patch or i will do so in the morning. > > On Mon, Apr 25, 2011 at 5:22 PM, <antoniop@google.com> wrote: >> >> I think the logic of this function is backwards... >> >> Shouldn't it be: if --user_data_dir is given, don't exit. I'm seeing the >> opposite behavior. >> >> http://codereview.chromium.org/6621045/ > > -- Antonio Petruzziello AntonioP@Google.com Google Video Conference / NYC
Who is the genius that decided to not allow running chrome as root? On Mar 8, 11:34 am, mbel...@chromium.org wrote: > http://codereview.chromium.org/6621045/diff/5003/chrome/browser/brows... > File chrome/browser/browser_main_gtk.cc (right): > > http://codereview.chromium.org/6621045/diff/5003/chrome/browser/brows... > chrome/browser/browser_main_gtk.cc:82: return; > On 2011/03/07 23:40:16, Elliot Glaysher wrote: > > > You should move this above the gtk initialization so the library isn't > > initialized twice. > > Done. > > http://codereview.chromium.org/6621045/
Hello Mike, Did a fix ever make it in for this bug? Around what release could we expect to see this fix? Thanks! --tony On Mon, Apr 25, 2011 at 9:32 PM, Antonio Petruzziello <antoniop@google.com> wrote: > Thanks for getting back to me! > > I don't know the first thing about your code tree. I'm sure it would > be alot easier for you to zap that not operator. Thanks! > > --tony > > On Tue, Apr 26, 2011 at 12:29 AM, Mike Belshe <mbelshe@google.com> wrote: >> Yes indeed! Sorry about that. We had noticed this before, and I thought a >> fix went in, but I think we overlooked it. >> Feel free to patch or i will do so in the morning. >> >> On Mon, Apr 25, 2011 at 5:22 PM, <antoniop@google.com> wrote: >>> >>> I think the logic of this function is backwards... >>> >>> Shouldn't it be: if --user_data_dir is given, don't exit. I'm seeing the >>> opposite behavior. >>> >>> http://codereview.chromium.org/6621045/ >> >> > > > > -- > Antonio Petruzziello > AntonioP@Google.com > > Google Video Conference / NYC > -- Antonio Petruzziello AntonioP@Google.com Google Video Conference / NYC
fix was r83511 if you are running dev channel, we make releases every ~week, stable every ~6 weeks, beta in between -- Evan Stade On Wed, May 4, 2011 at 3:45 PM, Antonio Petruzziello <antoniop@google.com> wrote: > Hello Mike, > > Did a fix ever make it in for this bug? Around what release could we > expect to see this fix? > > Thanks! > --tony > > On Mon, Apr 25, 2011 at 9:32 PM, Antonio Petruzziello > <antoniop@google.com> wrote: >> Thanks for getting back to me! >> >> I don't know the first thing about your code tree. I'm sure it would >> be alot easier for you to zap that not operator. Thanks! >> >> --tony >> >> On Tue, Apr 26, 2011 at 12:29 AM, Mike Belshe <mbelshe@google.com> wrote: >>> Yes indeed! Sorry about that. We had noticed this before, and I thought a >>> fix went in, but I think we overlooked it. >>> Feel free to patch or i will do so in the morning. >>> >>> On Mon, Apr 25, 2011 at 5:22 PM, <antoniop@google.com> wrote: >>>> >>>> I think the logic of this function is backwards... >>>> >>>> Shouldn't it be: if --user_data_dir is given, don't exit. I'm seeing the >>>> opposite behavior. >>>> >>>> http://codereview.chromium.org/6621045/ >>> >>> >> >> >> >> -- >> Antonio Petruzziello >> AntonioP@Google.com >> >> Google Video Conference / NYC >> > > > > -- > Antonio Petruzziello > AntonioP@Google.com > > Google Video Conference / NYC >
great! thank you very much! On Wed, May 4, 2011 at 4:03 PM, Evan Stade <estade@chromium.org> wrote: > fix was r83511 > > if you are running dev channel, we make releases every ~week, stable > every ~6 weeks, beta in between > > > > -- Evan Stade > > > > On Wed, May 4, 2011 at 3:45 PM, Antonio Petruzziello > <antoniop@google.com> wrote: >> Hello Mike, >> >> Did a fix ever make it in for this bug? Around what release could we >> expect to see this fix? >> >> Thanks! >> --tony >> >> On Mon, Apr 25, 2011 at 9:32 PM, Antonio Petruzziello >> <antoniop@google.com> wrote: >>> Thanks for getting back to me! >>> >>> I don't know the first thing about your code tree. I'm sure it would >>> be alot easier for you to zap that not operator. Thanks! >>> >>> --tony >>> >>> On Tue, Apr 26, 2011 at 12:29 AM, Mike Belshe <mbelshe@google.com> wrote: >>>> Yes indeed! Sorry about that. We had noticed this before, and I thought a >>>> fix went in, but I think we overlooked it. >>>> Feel free to patch or i will do so in the morning. >>>> >>>> On Mon, Apr 25, 2011 at 5:22 PM, <antoniop@google.com> wrote: >>>>> >>>>> I think the logic of this function is backwards... >>>>> >>>>> Shouldn't it be: if --user_data_dir is given, don't exit. I'm seeing the >>>>> opposite behavior. >>>>> >>>>> http://codereview.chromium.org/6621045/ >>>> >>>> >>> >>> >>> >>> -- >>> Antonio Petruzziello >>> AntonioP@Google.com >>> >>> Google Video Conference / NYC >>> >> >> >> >> -- >> Antonio Petruzziello >> AntonioP@Google.com >> >> Google Video Conference / NYC >> > -- Antonio Petruzziello AntonioP@Google.com Google Video Conference / NYC
What version of chrome do I have to download in order for this to work? /opt/google/chrome/chrome --user-data-dir=/root/.config/chromium doesn't seem to be working. I am currently running only chrome as non- root, but that brings up unexpected issues with plugins and with the saving of files ( e.g saving files in 'desktop' saves them in /home/ pedro/Desktop instead of /root/Desktop ) I am aware that running a browser as root is not good for security, but since I am only an ordinary user in an ordinary world I would like to take my chances :) Regards, Pedro On 29 abr, 23:25, Kashif <a10n3.s7r1...@gmail.com> wrote: > Who is the genius that decided to not allow running chrome as root? > > On Mar 8, 11:34 am, mbel...@chromium.org wrote: > > > > > > > > >http://codereview.chromium.org/6621045/diff/5003/chrome/browser/brows... > > File chrome/browser/browser_main_gtk.cc (right): > > >http://codereview.chromium.org/6621045/diff/5003/chrome/browser/brows... > > chrome/browser/browser_main_gtk.cc:82: return; > > On 2011/03/07 23:40:16, Elliot Glaysher wrote: > > > > You should move this above the gtk initialization so the library isn't > > > initialized twice. > > > Done. > > >http://codereview.chromium.org/6621045/ |
