ssh config files and/or private keys

[vieux]

close #24 #22

[vieux]

@cyli @stevvooe what's wrong with this version ?

/cc @mattzuba @davidwilliamson

[stevvooe]

Looks reasonable. Using a mount to get the keys in!

[marcelo-ochoa]

It works perfect, thanks for the contribution.
When this patch will be merged with master branch??
Best regards, Marcelo.

[vieux]

@marcelo-ochoa I'll merge today, push the v1.3 image today and if everything go as planned, I'll update latest -> 1.3 next week.

[marcelo-ochoa]

Great!!! Thanks a lot. In the meantime I'll configure my volumes using vieux/sshfs:next compile from sources.

[mattzuba]

The only question I'd have is why not mount it to /root/.ssh, instead of /.ssh? /root/.ssh would allow more flexibility with a ssh config file rather than having to specify all of the -o options in the volume create command.

My use-case is probably somewhat unique, but I basically have an in-house ci/cd system modeled after Bitbucket's Pipelines with an added extension that allows me to specify mounts in the docker containers to push builds to for qa and production deployments (see example below). The simpler I can keep this, the better off we are. With the yaml config below, along with the simple change I mentioned in my issue, I can do all of the ssh settings in a ssh_config file which is mounted in /root/.ssh of this plugin along with the ssh keys. With the proposed PR, I'd likely have to add an 'options' array to the mount definition and pass those as a bunch of -o options to the volume create command.

If /root/.ssh is a security risk, I completely understand and I can work around it (maybe using -F /.ssh/config in volume create?)

image: deploy-image

pipelines:
  branches:
    stable:
      - step:
          mounts:
            - linux01
          script:
            - deploy -d /mnt/linux01/srv/www/myapp

definitions:
  mounts:
   linux01:
      type: linux
      host: linux01.mydomain.com
      path: /

[vieux]

@mattzuba how about like this ?

[mattzuba]

Yup, I think that would be perfect! I already left work for the day so I can't test it out right now, but I don't see why it wouldn't work!

[mattzuba]

Should this be adjusted to be /root/.ssh instead of just .ssh?

[vieux]

I'll take a look, I'm not sure it's needed since the tests are passing right now

[marcelo-ochoa]

I tested latest commit and it works without passing the argument:
-o IdentityFile=/.ssh/id_rsa
but it still requires the plugin setting:
docker plugin set vieux/sshfs:next sshkey.source=/root/.ssh/
to works with keys stored and named using ssh default settings.
Its there a possibility to avoid above setting having a default value when the plugin is installed/enabled?
Best regards, Marcelo.
PD: After installing plugin mounts look like:

            "Mounts": [
                {
                    "Description": "",
                    "Destination": "/mnt/state",
                    "Name": "state",
                    "Options": [
                        "rbind"
                    ],
                    "Settable": [
                        "source"
                    ],
                    "Source": "/var/lib/docker/plugins/",
                    "Type": "bind"
                },
                {
                    "Description": "",
                    "Destination": "/root/.ssh",
                    "Name": "sshkey",
                    "Options": [
                        "rbind"
                    ],
                    "Settable": [
                        "source"
                    ],
                    "Source": "",
                    "Type": "bind"
                }
            ]

note the property "Source": "", after calling plugin set it is defined "Source": "/root/.ssh/"

[marcelo-ochoa]

well this simple patch makes Source property with a default value :)

diff --git a/config.json b/config.json
index e396ffa..6852b4b 100644
--- a/config.json
+++ b/config.json
@@ -52,6 +52,7 @@
       "settable": [
         "source"
       ],
+      "Source": "/root/.ssh/",
       "type": "bind"
     }
   ],

[vieux]

I'm not really fond of this for 2 reasons : I prefer users to be explicit when mounting ssh keys into the plugin, I think it's safer. It won't work when users other than root will use the plugin. On Sep 2, 2017 2:12 PM, "marcelo-ochoa" <notifications@github.com> wrote: well this simple patch makes Source property with a default value :) diff --git a/config.json b/config.json index e396ffa..6852b4b 100644 --- a/config.json +++ b/config.json @@ -52,6 +52,7 @@ "settable": [ "source" ], + "Source": "/root/.ssh/", "type": "bind" } ], — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#33 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AA_BR_jS4AEaExzgIi7sbwHZK4ocexHTks5secSmgaJpZM4PDLlN> .

[mattzuba]

I agree on the safety aspect, forcing plugin installers to be explicit in where the keys and config is located is definitely the better option, and it's documented on how to specify it when enabling the plugin as well.

…

On Sat, Sep 2, 2017, 2:46 PM Victor Vieux ***@***.***> wrote: I'm not really fond of this for 2 reasons : I prefer users to be explicit when mounting ssh keys into the plugin, I think it's safer. It won't work when users other than root will use the plugin. On Sep 2, 2017 2:12 PM, "marcelo-ochoa" ***@***.***> wrote: well this simple patch makes Source property with a default value :) diff --git a/config.json b/config.json index e396ffa..6852b4b 100644 --- a/config.json +++ b/config.json @@ -52,6 +52,7 @@ "settable": [ "source" ], + "Source": "/root/.ssh/", "type": "bind" } ], — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub < #33 (comment) >, or mute the thread < https://github.com/notifications/unsubscribe-auth/AA_BR_jS4AEaExzgIi7sbwHZK4ocexHTks5secSmgaJpZM4PDLlN > . — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#33 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABbOua63-MS-QlmaYTZmpAhUR2bsLd84ks5secy7gaJpZM4PDLlN> .

[marcelo-ochoa]

OK, my point is to provide two step installing.
IMO plugin installer should be root for the bare metal server which for sure have access to /root/.ssh directory, if plugin installer are other user rather than root what happen if they call to
docker plugin set vieux/sshfs:next sshkey.source=/root/.ssh/
to gain access to root directory?
Best regards, Marcelo.

[marcelo-ochoa]

Forget the diff about config.json file, I found that if Source property is set in that file then is not possible to change at run-time with:
docker plugin set vieux/sshfs:next sshkey.source=/root/.ssh
on the other hand if a regular user is included into Unix docker group, it will be able to enable/disable the plugin and obviously setting above property which could open a security hole in docker plugin run-time by changing the directory to other which have different private keys.
But if the server's sysadmin grants another user to the group docker for sure trust in that user ;)