search

Foxboron / ssh-tpm-agent

:computer: :key: ssh-agent for TPMs
MIT License
354 stars 20 forks source link

agent: add --key-dir as a flag, and warn if key dir is a symlink. #14

Closed andersju closed 1 year ago

andersju commented 1 year ago

I noticed that ssh-tpm-agent won't load keys if $HOME/.ssh is a symlink (and won't complain either), because filepath.WalkDir doesn't follow symbolic links. Made me scratch my head for a minute :) I really don't know much about Go, but this seems like a simple fix. Thanks for a sweet tool!

Edit - changed to: add --key-dir as a flag, and warn if key dir is a symlink. (See below.)

Foxboron commented 1 year ago

Symlinks are generally a bit iffy so I need to think a little bit about scenarios here.

Usually you don't want to arbitrary eval these things. Thoughts @stigtsp?

Can run the test suite and probably iterate on it thought, thanks!

andersju commented 1 year ago

Yeah, I wasn't sure; maybe just warn if ~/.ssh is not a directory, or if no keys were loaded, and/or make the path configurable (this comment might be better suited for #3 I realize).

stigtsp commented 1 year ago

Imho, an argument to the agent specifying a key dir makes more sense, for cases where you want keys to be stored somewhere else.

Foxboron commented 1 year ago

Yeah, I wasn't sure; maybe just warn if ~/.ssh is not a directory, or if no keys were loaded, and/or make the path configurable (this comment might be better suited for https://github.com/Foxboron/ssh-tpm-agent/issues/3 I realize).

Lets implement --key-dir as a flag and rather warn if ~/.ssh is a symlink :)

Would you be interested doing the work? It shouldn't be very hard.

andersju commented 1 year ago

Yeah, I wasn't sure; maybe just warn if ~/.ssh is not a directory, or if no keys were loaded, and/or make the path configurable (this comment might be better suited for #3 I realize).

Lets implement --key-dir as a flag and rather warn if ~/.ssh is a symlink :)

Would you be interested doing the work? It shouldn't be very hard.

Sure! See latest commit. I rebased and added --key-dir and a warning. Moved GetSSHDir() to utils (couldn't think of anything better than misc at the moment, and it felt misc-y).

Foxboron commented 1 year ago

Looks great, thank you!

utils (couldn't think of anything better than misc at the moment, and it felt misc-y).

It could probably just stay inside utils.go frankly. No use having a single file for one function :)

andersju commented 1 year ago

It could probably just stay inside utils.go frankly. No use having a single file for one function :)

:+1: renamed it

Foxboron commented 1 year ago

Thanks!