networksetup prompts for admin password are broken #68

Closed
skivvies opened this Issue Jan 6, 2012 · 44 comments

Projects

None yet

3 participants

@skivvies

This is now occurring for me what seems to be every time the "Start Lantern" and "Stop Lantern" menu items' code paths are followed, i.e. also when I start Lantern via run.bash and when I quit it via the menu item. My ~/.lantern directory is accessible in all cases.

@myleshorton myleshorton was assigned Jan 6, 2012
@skivvies

Getting "** Error: Unable to commit changes to network database." repeatedly in the console output.

@skivvies

I no longer get this after unchecking "Require an administrator password to access system preferences with lock icons" in Security.prefpane (big ups to forkner for the tip).

Let's still figure out how to do something less horrible in the case that this is set. A UI should never display a dozen password prompts in a row, each of which is indifferent to whether you enter your password correctly, incorrectly, or just hit cancel.

@forkner

the repeating is related to the setting "Security and Privacy" / “Require an administrator password to access system preferences with lock" -- when this is checked each shell call to networksetup will ask for an administrative password.

Also triggerable in a single case by locking the preferences (clicking the big lock shut) this will cause popups until correct password is entered.

When the user refuses or is unable to provide the correct password, lantern is not really reacting, just silently failing and moving on to the next step.

@myleshorton
Lantern member

Huh, OK I thought we were all on the same page on this like 2 months ago, but I guess not. The "Require an administrator password to access system preferences with lock" is not the default setting on Lion or Snow Leopard, which is why we weren't necessarily considering this a blocker.

@skivvies

Sorry, didn't remember this came up before. Forgetting about that setting though this still comes up anytime just Network.prefpane is left locked, if I understand correctly? Which is maybe significantly more common enough we don't want to just assume it's unlocked?

@forkner

Yeah not the default. Don't remember hearing that we had pinned down the cause and it was not recorded here for posterity in any case.

@forkner

If the settings are locked you will get a popup until you input the correct password once. If you cancel lantern will fail silently. This is a problem, but not a huge blocker I think.

@myleshorton
Lantern member

Just added a sudo to the script that solves this for Snow Leopard at least. Does it work for you guys on Lion? You should now only get a single password prompt. I haven't added support for failure cases yet, however (when you enter the wrong password)

@skivvies

Now I don't get a password prompt at all. Blew away my ~/.lantern and tried first with just Network.prefpane locked and then with "Require an administrator password to access system preferences with lock" checked. Shouldn't I have gotten the prompt once?

@myleshorton
Lantern member

yeah -- I was curious about that myself =). i think it has to do with caching sudo in terminal -- had you sudoed at all from that terminal? I think there's actually some caching for Terminal.app outside of individual window processes as well. Maybe quit terminal and restart?

@forkner

Hmm. I'm not sure this is working so much as just prompting on the command line in a way that is hard to notice?
I randomly noticed this on the command line output when I was trying to quit but hitting the wrong keys:

Password:
Password:
Password:
Password:
136598 2012-01-19 13:26:24,912 INFO  [qtp78045353-49] lantern.Configurator.configureOsxScript (Configurator.java:206) - Result of script is: mkdir: /Users/ltucker/Library/Logs/Lantern: File exists
Sorry, try again.
Sorry, try again.
Sorry, try again.
sudo: 3 incorrect password attempts
@forkner

There are a few ways to run something and get graphical authorization, but I don't think any of them will retain the privs unless all the calls are made from the same script. Anyway, on the off chance they are helpful:

osascript can be used something like this

osascript -e "do shell script \"$bar\" with administrator privileges"

there is also cocoasudo http://www.performantdesign.com/2009/10/26/cocoasudo-a-graphical-cocoa-based-alternative-to-sudo/

@myleshorton
Lantern member

Hey ya'll-- so I just put in the osascript change and is working great. I think before I only got it working through elevating the privileges from the installer itself, but not sure how it was doing that internally.

Also moved all the proxy code from Configurator into a new "Proxifier" class since Configurator was pretty much all proxy code. Thanks for all the testing and suggestions!

@myleshorton myleshorton reopened this Jan 20, 2012
@myleshorton
Lantern member

Oh -- reopening for now to make sure it's working for you guys on Lion. Can you take it for a spin, likely manana?

@forkner

I get the prompts now, one each time proxy settings are changed. eg on startup and each time I switch modes -- not perfect, but mode switching is a fairly rare thing to do anyway.

A few notes though:

1) It would be better if we could check whether we need administrative privileges before we ask for them since in the normal case (default setup, unlocked network prefs) it doesn't require this and we could have no popups -- possibly checkable from applescript.

2) The popup says that "osascript" wants to make changes -- this is annoying since it isn't clear that it's lantern trying to do something. It seems to listen to it's own argv[0] though, so making a copy or hard link to the osascript binary to "Lantern automatic proxy configuration" will change the wording of the dialog.

3) I still don't get any indication of what it means when this fails or I cancel the dialog.

@forkner

oh it also asks on exit :/

@skivvies

oh it also asks on exit :/

I think it's that it asks on signout, which is entailed by exit if you're signed in

@myleshorton
Lantern member

Oh interesting -- so it pops up regardless of system prefs now, is that right forkner? Didn't realize that.

@forkner

yeah, same problem with sudo

@myleshorton
Lantern member

Ahh cool -- good catch. Will address that. The oascript naming was annoying me too -- will knock that out as well.

@forkner

looking much better. only possible inconsistency here will be that if the admin password setting is not on, but the network preferences are locked, the dialog will be from 'networksetup' not osascript, so another naming issue, possibly just use osascript in either case?

@forkner

maybe not actually, the networksetup behavior in the case where the admin password settings is off is preferable (unlocks setting for all further calls)

@myleshorton
Lantern member

I'm actually using osascript in either case, so we should be good. i just don't require admin privileges in one case and do in the other.

@skivvies skivvies pushed a commit that referenced this issue Jan 20, 2012
_pants Merge branch 'master' of github.com:getlantern/lantern
* 'master' of github.com:getlantern/lantern:
  use getlantern.org domain spreadsheet
  backend api work for contact form #22
  Fixed admin password issue #68
  Added AppleScript call to check admin password setting on osx #68
678a9af
@skivvies

Huge improvement, nice! Just noticed the prompt doesn't mention Lantern anywhere in it. Is there any way to make it explicit that it's Lantern that needs this info, in case the user can't figure out that "networksetup" (or "osascript", or whatever appears to be asking) is just a tool Lantern's using?

@forkner

yeah read back a bit

@skivvies

word, saw your hard link idea, should have just said +1
for posterity, here's a screenshot of the prompt i just got:
screenshot
user: "osascript? wtf? and what kind of changes does it want to make? maybe i should just hit cancel."

when i tried it the previous time i think it at least said something like "network setup is trying to modify your system settings", which gives you more of a hint. ideally would be something like "enter your password to use Lantern as your system proxy"?

also just want to document that when you hit cancel, Lantern still seems to proceed as though you typed a correct password (i see "getting access" and the lantern is lit). should we detect that the user hit cancel and then say something like "Lantern is running but was not set as your system proxy. Either configure your browser to use Lantern manually, or set it as your system proxy in Under the Hood." Ideally user could click to go to Under the Hood, where they'd find the system proxy setting unchecked; checking it would reinitiate the prompt. (does this make sense?)

also, i got the prompt again on quit, which i think is superfluous since i had canceled it on launch?

tested putting in a bad password which it is correctly rejecting now.

@myleshorton
Lantern member

Just added hard link generation and cancel handling. The cancel handling is still a little odd if the user really decides they want to cancel, but I think we'll have to punt on that for now.

@skivvies skivvies reopened this Jan 24, 2012
@skivvies

Just tried signing in during the welcome screens with Network.prefpane unlocked, did not get the prompt as expected, then locked Network.prefpane and chose Quit from the tray menu (while still in the welcome screens). I got the following dialog, which repeatedly ignored hitting cancel over and over again:
screenshot
So it basically forces you to enter a correct password. And it's back to saying "networksetup" instead of "Lantern".

@skivvies

I see this in the log output after hitting cancel probably 6 times, in case it's of interest:
511440 2012-01-24 01:54:01,022 INFO [Unset-Web-Proxy-Thread] lantern.Proxifier.proxyOsxViaScript (Proxifier.java:174) - Result of script is: ** Error: Unable to commit changes to network database.
** Error: Unable to commit changes to network database.
** Error: Unable to commit changes to network database.
** Error: Unable to commit changes to network database.
** Error: Unable to commit changes to network database.
** Error: Unable to commit changes to network database.

@skivvies

Just triggered the "Lantern"-titled prompt on quit, hit Cancel, and got this:

89260 2012-01-24 02:25:06,202 WARN [Unset-Web-Proxy-Thread] lantern.MacProxyManager.runScript (MacProxyManager.java:356) - Command failed!! -- -e
89261 2012-01-24 02:25:06,203 INFO [Unset-Web-Proxy-Thread] lantern.Proxifier.proxyOsxViaScript (Proxifier.java:176) - Result of script is: 0:113: execution error: User canceled. (-128)

89261 2012-01-24 02:25:06,203 INFO [Unset-Web-Proxy-Thread] lantern.Proxifier.proxyOsxViaScript (Proxifier.java:201) - Asking again
89262 2012-01-24 02:25:06,204 ERROR [Unset-Web-Proxy-Thread] lantern.LanternHttpProxyServer.uncaughtException (LanternHttpProxyServer.java:72) - Uncaught exception
org.eclipse.swt.SWTException: Device is disposed
at org.eclipse.swt.SWT.error(Unknown Source)
at org.eclipse.swt.SWT.error(Unknown Source)
at org.eclipse.swt.SWT.error(Unknown Source)
at org.eclipse.swt.widgets.Display.error(Unknown Source)
at org.eclipse.swt.widgets.Display.syncExec(Unknown Source)
at org.lantern.Dashboard.askQuestion(Dashboard.java:107)
at org.lantern.Proxifier.proxyOsxViaScript(Proxifier.java:202)
at org.lantern.Proxifier.unproxyOsxViaScript(Proxifier.java:320)
at org.lantern.Proxifier.unproxyOsx(Proxifier.java:316)
at org.lantern.Proxifier.stopProxying(Proxifier.java:124)
at org.lantern.Proxifier$1.run(Proxifier.java:82)
at java.lang.Thread.run(Thread.java:680)

Is Lantern quitting too early (before waiting to see what happens with the prompt)?

@skivvies skivvies pushed a commit that referenced this issue Jan 24, 2012
_pants lots of junk:
- make google.com a required whitelist entry (#22, #37)
- annotate Proxifier.java with a gillion questions (sorry @myleshorton) (#68)
- copy tweaks
- make signin UI more robust against edge cases (e.g. attempted signin as same user signed in already)
- make whitelist UI more robust to go with #116, add clientside hostname validation
- show .ng-invalid inputs with red outline + other tweaks
07524a7
@skivvies

@myleshorton took a peek at Proxifier.java and left some questions in 07524a7 (with apologies for anything I'm missing)

@myleshorton
Lantern member

Did you get the yes/no screen asking you if you really wanted to cancel, or did it just keep asking for your admin credentials?

@skivvies

@myleshorton you mean when i triggered the junk 4 comments back? If you just cancel the first prompt you get on Quit, can you reproduce?

@myleshorton
Lantern member

On the shutdown issue, we need to somehow have a blocking thread that checks the status of the SWT screens.

@myleshorton
Lantern member

Look at isProxying state on shutdown -- is it set correctly when native stuff fails?

@myleshorton
Lantern member

Check status of isSystemProxy in all cases.

@skivvies skivvies pushed a commit that referenced this issue Jan 25, 2012
_pants add setup step for system proxy setting (#68), more signin form impro…
…vements (#120), css tweaks
2bb6c0d
@skivvies

ok, there's an additional setup step in get mode now right before the setup complete screen for the system proxy setting as discussed today, so if there's going to be a password prompt it should happen when the user clicks "continue" on that screen. let me know how you'd like to be notified when the user clicks continue and i'll rig it in.

@forkner

I'm going to grab this one for a sec, seems likely it will make sense to roll together with #122 for autoproxy triggering

@forkner forkner was assigned Jan 25, 2012
@forkner

should now display error message in new configuration setup screen when user cancels or other failure to configure happens. also should get warning if you shut down via the quit menu and you cancel -- if you Ctrl-C, you will not get a warning not much you can do here I believe -- but you should still be able to put your password in.

We could possibly put the warnings into scripts so they always execute, but it seems to me this is only for devs anyway in the case we're worried about.

@skivvies

here's some prior art courtesy of Charles Proxy
screenshot

would be awesome if we could figure out how it's able to only prompt for your password once and modify your system proxy settings whenever it wants without prompting thereafter

@forkner

One way around is to just create a setuid binary in charge of only the proxy/unproxy junk during the install process I think -- possibly what this is doing.

@skivvies

nice. you guys think it's worth trying it for this release?

@myleshorton myleshorton was assigned Jan 27, 2012
@myleshorton
Lantern member

I don't think it's worth doing for this release. We should keep in mind it's only an issue at all for a minority of users, whereas I would guess the Charles Proxy prompt probably happens for all users. Do you know if that's true @pants?

@myleshorton
Lantern member

Closing this ticket -- should be working in all cases.

@skivvies skivvies pushed a commit that referenced this issue May 1, 2013
_pants Merge branch 'master' of github.com:getlantern/lantern
* 'master' of github.com:getlantern/lantern:
  use getlantern.org domain spreadsheet
  backend api work for contact form #22
  Fixed admin password issue #68
  Added AppleScript call to check admin password setting on osx #68
fd5a970
@skivvies skivvies pushed a commit that referenced this issue May 1, 2013
_pants lots of junk:
- make google.com a required whitelist entry (#22, #37)
- annotate Proxifier.java with a gillion questions (sorry @myleshorton) (#68)
- copy tweaks
- make signin UI more robust against edge cases (e.g. attempted signin as same user signed in already)
- make whitelist UI more robust to go with #116, add clientside hostname validation
- show .ng-invalid inputs with red outline + other tweaks
67d6c16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment