Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sftp: Ssh add key pem to config #4238

Closed
wants to merge 0 commits into from

Conversation

calisro
Copy link
Member

@calisro calisro commented May 15, 2020

What is the purpose of this change?

I hope this doesn't fly in the face of best practice. I have some platforms for which I do not allow password authentication at all. Only keys. I'd like to be able to store the key in the config (or environment variable) with the rest of the parameters rather than carrying it around in an external file to deploy along with the rclone config.

So i'd allow for a config parameter to store the actual key rather than only read a file system file. This would also enable tools that abstract the filesystem from the app running rclone to work correctly here (RCX for android for example).

command:
--sftp-key-pem (actual key)

config:
key_pem = (actual key)

I was thinking since the config file doesn't allow multi-line that I would force a sane delimiter (\r\n and \n).

Thoughts?

Was the change discussed in an issue or in the forum before?

no. It was a simple change so figured i'd just do a PR for review.

https://forum.rclone.org/t/add-ssh-key-to-config-file-rather-than-filesystem-pointer/16404

Checklist

  • I have read the contribution guidelines.
  • I have added tests for all changes in this PR if appropriate.
  • I have added documentation for the changes if appropriate.
  • All commit messages are in house style.
  • I'm done, this Pull Request is ready for review :-)

@calisro calisro force-pushed the ssh-add-key-to-config branch 4 times, most recently from 6dd2ee2 to 41be9fb Compare May 15, 2020 20:58
@calisro calisro changed the title Ssh add key pem to config Sftp: Ssh add key pem to config May 15, 2020
@calisro
Copy link
Member Author

calisro commented May 15, 2020

Perhaps the delimiter could be a simple character though like a ":". And do i don't think I actually need the replace statements besides the actual delimiter. I can adjust after feedback.

@ncw
Copy link
Member

ncw commented May 16, 2020

I think this is a perfectly reasonable thing to want to do!

I'd suggest a bit of experimentation first

  1. I have a feeling that the config file can have multiple line input if the lines are indented with space, eg
value = line1
 line2
 line3
  1. Did you try replacing all the cr/lf with spaces in the identity file - is that acceptable to ssh? It would be nice to be able to say to users, run sed 's/\n/ /g < keyin > keyout then paste keyout into your config. Whatever config scheme we come up with coming up with a little sed script will make peoples life easier!

@calisro
Copy link
Member Author

calisro commented May 16, 2020

  1. multi-line didn't work with spaces. Tried quotes and other things. I took a look at the config source and their documentation (unknwon/goconfig) and nothing came out either.

Failed to load config file "/home/xxxx/.rclone.conf": could not parse line: xxxxxxxxxxxxxx

Assuming multi-line is out for now:
2) spaces in PEM doesn't work either. Must be LF.

We could make an arbitrary delimiter and just use that. In the code I replace it with real LF anyway. or we can keep \n

awk '{printf "%s\n", $0}' < ~/.ssh/id_rsa

Thoughts? I can add all this to the docs.

@calisro
Copy link
Member Author

calisro commented May 16, 2020

oops. What did I just do. How do I fix this? I was trying to just remove the go.mod and go.sum files and apparently I removed them all.

@calisro calisro mentioned this pull request May 16, 2020
5 tasks
@ncw
Copy link
Member

ncw commented May 18, 2020

Good old git! I see you made a new PR :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants