hamzasood / pam_touchid

A PAM module for authentication with Touch ID
GNU General Public License v3.0
441 stars 32 forks source link

pam_touchid appears to break sudo over SSH #2

Open modest opened 7 years ago

modest commented 7 years ago

I haven't fully tested this scenario, but at first pass, it seems like pam_touchid prevents a remote SSH user from using sudo on a Mac with pam_touchid installed. The GUI prompt appears and never gives up without GUI user input.

Are there some possible mitigations here?

hamzasood commented 7 years ago

Interesting, I hadn't thought of that.

Does it work if you add something like:

if (getenv("SSH_TTY"))  
    return PAM_IGNORE;

to the very top of pam_sm_authenticate?

I can't test it at the moment, but hopefully that will make it fallback to the default authentication method when run from ssh (assuming you kept the opendirectory line intact in the ssh pam.d entry)

fraimondo commented 7 years ago

Just tested it. I can confirm it works.

cqexbesd commented 7 years ago

I think also that XPC_SERVICE_NAME will be set if running a shell locally - but that might depend on if you are using Terminal.app or an alternative. Neither way seem that great as a proxy for detecting if the user is local or not but I can't think of a better way.

barry-scott commented 7 years ago

@cqexbesd what if I have a GUI app that spawns a sudo command? Will I see the XPC_SERVICE_NAME then?

ITJesse commented 7 years ago

if (getenv("SSH_TTY"))
return PAM_IGNORE;

@hamzasood I can also confirm it works. Why don't you merge it into code?

caesar commented 7 years ago

Looks like this project may be abandoned, sadly... in the meantime, @BenKesselring has a fork with this bug fixed: https://github.com/BenKesselring/pam_touchid/

AdnanHodzic commented 7 years ago

@caesar didn't have a chance to check, did he include if (getenv("SSH_TTY")) return PAM_IGNORE; into his fork/master?

caesar commented 7 years ago

@AdnanHodzic yes. He also opened a PR (#4) to merge that fix into this project but @hamzasood unfortunately never merged it.