Support SSH key credentials directly in external service configs

[tsenart]

Today, users have to jump through hoops to make Sourcegraph talk SSH when cloning repositories.

Similarly to what we do with client certificates, we should support configuring SSH credentials directly in external service configuration.

This would allow us to make GitoliteSource in repo updater talk directly to Gitolite instead of proxying requests to gitserver.

/cc @sourcegraph/source

[keegancsmith]

@tsenart Did you mean to add this to the 3.4 milestone?

[tsenart]

@keegancsmith: Nope.

[keegancsmith]

We are working on TLS communication this milestone, and the idea is to include SSH as part of that work. So we hope to include this in 3.13.

This would allow us to make GitoliteSource in repo updater talk directly to Gitolite instead of proxying requests to gitserver.

We probably want to keep this behaviour, since it would be unreasonable for us to support all the different ways people setup SSH auth (eg specific settings in your ssh config like tunneling / etc). What are the downsides of gitolitesource going via gitserver?

So to clarify this task is about simplifying the most common thing a user needs to do to get cloning via SSH working. But won't necessarily cover all the things an admin may need to do for less common setups.

[keegancsmith]

Initially I thought we could use a global configuration where SSH will just try a list of private keys. However, if you have multiple GitHub keys github will accept the first one that is valid, and then use that as the user. This means we would 404 on repos which are only accessible to the 2nd or on private key. As such the correct solution seems to be to specify the private key like we specify the clone URL. We can then specify the private key via GIT_SSH_COMMAND="ssh -i path/to/key".

I am thinking instead of co-ordinating some key management / writing the keys to temp we just write the key to use to .git/id_rsa before running a remote git command. WDYT?

[tsenart]

I am thinking instead of co-ordinating some key management / writing the keys to temp we just write the key to use to .git/id_rsa before running a remote git command. WDYT?

If that's not too much I/O churn, :+1:

[unknwon]

We can then specify the private key via GIT_SSH_COMMAND="ssh -i path/to/key".

I am thinking instead of co-ordinating some key management / writing the keys to temp we just write the key to use to .git/id_rsa before running a remote git command. WDYT?

Since "path/to/key" could be any path, could we only keep one copy on disk for each key in a central location (and naming them like id_rsa_<external_id>), pointing to the appropriate one at clone time instead of writing it to each repo? So we don't actually write anything but just changing the envs arg.

[keegancsmith]

We can then specify the private key via GIT_SSH_COMMAND="ssh -i path/to/key". I am thinking instead of co-ordinating some key management / writing the keys to temp we just write the key to use to .git/id_rsa before running a remote git command. WDYT?

Since "path/to/key" could be any path, could we only keep one copy on disk for each key in a central location (and naming them like id_rsa_<external_id>), pointing to the appropriate one at clone time instead of writing it to each repo? So we don't actually write anything but just changing the envs arg.

gitserver is not aware of external services. repo-updater will tell gitserver to clone a repo with certain credentials. As I mentioned above I don't really want to introduce a central location because then concerns around deletion, atomic updates, etc come into play. We can safely isolate operations to a single repo since we already have concurrency controls in place for remote operations on single repositories.

[keegancsmith]

We still have SSHConfig in the gitserver exec protocol :D. I did some digging and found how we used to handle SSHConfig in a private archive of old sourcegraph code which blames to Feb 2016 in a file called helper.go :)

package gitserver

import (
    "io/ioutil"
    "log"
    "os"
    "os/exec"
    "path/filepath"
    "runtime"

    "sourcegraph.com/sourcegraph/sourcegraph/pkg/vcs"
    "sourcegraph.com/sourcegraph/sourcegraph/pkg/vcs/util"
)

// InsecureSkipCheckVerifySSH controls whether the client verifies the
// SSH server's certificate or host key. If InsecureSkipCheckVerifySSH
// is true, the program is susceptible to a man-in-the-middle
// attack. This should only be used for testing.
var InsecureSkipCheckVerifySSH bool

func runWithRemoteOpts(cmd *exec.Cmd, opt *vcs.RemoteOpts) error {
    if opt != nil && opt.SSH != nil {
        gitSSHWrapper, gitSSHWrapperDir, keyFile, err := makeGitSSHWrapper(opt.SSH.PrivateKey)
        defer func() {
            if keyFile != "" {
                if err := os.Remove(keyFile); err != nil {
                    log.Fatalf("Error removing SSH key file %s: %s.", keyFile, err)
                }
            }
        }()
        if err != nil {
            return err
        }
        defer os.Remove(gitSSHWrapper)
        if gitSSHWrapperDir != "" {
            defer os.RemoveAll(gitSSHWrapperDir)
        }
        cmd.Env = append(cmd.Env, "GIT_SSH="+gitSSHWrapper)
    }

    if opt != nil && opt.HTTPS != nil {
        gitPassHelper, gitPassHelperDir, err := makeGitPassHelper(opt.HTTPS.Pass)
        if err != nil {
            return err
        }
        defer os.Remove(gitPassHelper)
        if gitPassHelperDir != "" {
            defer os.RemoveAll(gitPassHelperDir)
        }
        cmd.Env = append(cmd.Env, "GIT_ASKPASS="+gitPassHelper)
    }

    return cmd.Run()
}

// makeGitSSHWrapper writes a GIT_SSH wrapper that runs ssh with the
// private key. You should remove the sshWrapper, sshWrapperDir and
// the keyFile after using them.
func makeGitSSHWrapper(privKey []byte) (sshWrapper, sshWrapperDir, keyFile string, err error) {
    var otherOpt string
    if InsecureSkipCheckVerifySSH {
        otherOpt = "-o StrictHostKeyChecking=no"
    }

    kf, err := ioutil.TempFile("", "go-vcs-gitcmd-key")
    if err != nil {
        return "", "", "", err
    }
    keyFile = kf.Name()
    err = util.WriteFileWithPermissions(keyFile, privKey, 0600)
    if err != nil {
        return "", "", keyFile, err
    }

    tmpFile, tmpFileDir, err := gitSSHWrapper(keyFile, otherOpt)
    return tmpFile, tmpFileDir, keyFile, err
}

// Makes system-dependent SSH wrapper
func gitSSHWrapper(keyFile string, otherOpt string) (sshWrapperFile string, tempDir string, err error) {
    // TODO(sqs): encrypt and store the key in the env so that
    // attackers can't decrypt if they have disk access after our
    // process dies

    var script string

    if runtime.GOOS == "windows" {
        script = `
    @echo off
    ssh -o ControlMaster=no -o ControlPath=none ` + otherOpt + ` -i ` + filepath.ToSlash(keyFile) + ` "%@"
`
    } else {
        script = `
    #!/bin/sh
    exec /usr/bin/ssh -o ControlMaster=no -o ControlPath=none ` + otherOpt + ` -i ` + filepath.ToSlash(keyFile) + ` "$@"
`
    }

    sshWrapperName, tempDir, err := util.ScriptFile("go-vcs-gitcmd")
    if err != nil {
        return sshWrapperName, tempDir, err
    }

    err = util.WriteFileWithPermissions(sshWrapperName, []byte(script), 0500)
    return sshWrapperName, tempDir, err
}

// makeGitPassHelper writes a GIT_ASKPASS helper that supplies password over stdout.
// You should remove the passHelper (and tempDir if any) after using it.
func makeGitPassHelper(pass string) (passHelper string, tempDir string, err error) {
    tmpFile, dir, err := util.ScriptFile("go-vcs-gitcmd-ask")
    if err != nil {
        return tmpFile, dir, err
    }

    passPath := filepath.Join(dir, "password")
    err = util.WriteFileWithPermissions(passPath, []byte(pass), 0600)
    if err != nil {
        return tmpFile, dir, err
    }

    var script string

    // We assume passPath can be escaped with a simple wrapping of single
    // quotes. The path is not user controlled so this assumption should
    // not be violated.
    if runtime.GOOS == "windows" {
        script = "@echo off\ntype " + passPath + "\n"
    } else {
        script = "#!/bin/sh\ncat '" + passPath + "'\n"
    }

    err = util.WriteFileWithPermissions(tmpFile, []byte(script), 0500)
    return tmpFile, dir, err
}

// repoExists checks if dir is a valid GIT_DIR.
func repoExists(dir string) bool {
    _, err := os.Stat(filepath.Join(dir, "HEAD"))
    return !os.IsNotExist(err)
}

func recoverAndLog() {
    if err := recover(); err != nil {
        log.Print(err)
    }
}

[unknwon]

We can then specify the private key via GIT_SSH_COMMAND="ssh -i path/to/key". I am thinking instead of co-ordinating some key management / writing the keys to temp we just write the key to use to .git/id_rsa before running a remote git command. WDYT?

Since "path/to/key" could be any path, could we only keep one copy on disk for each key in a central location (and naming them like id_rsa_<external_id>), pointing to the appropriate one at clone time instead of writing it to each repo? So we don't actually write anything but just changing the envs arg.

gitserver is not aware of external services. repo-updater will tell gitserver to clone a repo with certain credentials. As I mentioned above I don't really want to introduce a central location because then concerns around deletion, atomic updates, etc come into play. We can safely isolate operations to a single repo since we already have concurrency controls in place for remote operations on single repositories.

You're right, agree with you.

[uwedeportivo]

Dear all,

This is your release captain speaking. 🚂🚂🚂

Branch cut for the 3.13 release is scheduled for tomorrow.

Is this issue / PR going to make it in time? Please change the milestone accordingly. When in doubt, reach out!

Thank you

[keegancsmith]

I'm backlogging. I focussed on more pressing customer issues / the impromptu London Core Services meetup took up my time.

[keegancsmith]

Won't get this in before branch cut, but hope to work on it this week.