Conversation
When -s or --ssh-copy-id flags are used, initially id_rsa is copied to host.
Staphylo
left a comment
Staphylo
left a comment
There was a problem hiding this comment.
Thanks for contributing to this tool.
ssh-copy-id is a pretty good enhancement to have.
|
|
||
| def copy_ssh_id(self): | ||
| self.create_ssh_id_if_not_exists() | ||
| cmd = 'ssh-copy-id -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null -i ~/.ssh/id_rsa.pub -f '+self.host |
There was a problem hiding this comment.
Please use the array format for subprocess as it's done above.
Also do not use shell= because it's unsafe and definetly not needed here.
Last note is to use f-strings instead of string cocatenation.
| if self.ssh_copy_id: | ||
| self.copy_ssh_id() |
There was a problem hiding this comment.
I would prefer avoiding operations in the constructor of the Remote object this is bad style.
Could you move this logic in the main function before if args.init line 411
This way there is no need to add a new attribute in the class that doesn't matter after init.
| self.create_ssh_id_if_not_exists() | ||
| cmd = 'ssh-copy-id -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null -i ~/.ssh/id_rsa.pub -f '+self.host | ||
| subprocess.run(cmd, shell=True) |
There was a problem hiding this comment.
I prefer if the tool gracefully exits if there is no key rather than trying to create one.
It is not the place for this tool to create a ssh key in behalf of the user, especially with the risk of overriding an existing one.
Also the defaults that you set for the key are extremely low.
I for one do not believe in rsa anymore especially not below 8192 bits.
2 participants
When -s or --ssh-copy-id flags are used, initially id_rsa is copied to host.