Skip to content

feat: fetch SSH key from Coder if required #174

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.

Sign up for GitHub

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

Closed
wants to merge 4 commits into from
Closed

feat: fetch SSH key from Coder if required #174

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6d76aca
feat: fetch SSH key from Coder if required
johnstcn May 3, 2024
efd51b6
fix up after rebase
johnstcn May 3, 2024
b1ba280
update README
johnstcn May 3, 2024
19eed21
retry fetching agent ssh key
johnstcn May 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,24 @@ envbuilder will assume SSH authentication. You have the following options:
ghcr.io/coder/envbuilder
```

1. Fetch the SSH key from Coder: as long as `CODER_AGENT_URL` and
`CODER_AGENT_TOKEN` are set, then envbuilder will attempt to fetch the
corresponding Git SSH key directly from Coder. Example:

```terraform
resource "docker_container" "workspace" {
count = data.coder_workspace.me.start_count
image = "ghcr.io/coder/envbuilder:version"
name =
"coder-${data.coder_workspace.me.owner}-${lower(data.coder_workspace.me.name)}"

env = [
"CODER_AGENT_TOKEN=${coder_agent.dev.token}",
"CODER_AGENT_URL=${data.coder_workspace.me.access_url}",
...
]
```

1. Agent-based authentication: set `SSH_AUTH_SOCK` and mount in your agent socket, for example:

```bash
Expand Down
2 changes: 1 addition & 1 deletion envbuilder.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ func Run(ctx context.Context, options Options) error {
CABundle: caBundle,
}

cloneOpts.RepoAuth = SetupRepoAuth(&options)
cloneOpts.RepoAuth = SetupRepoAuth(ctx, &options)
if options.GitHTTPProxyURL != "" {
cloneOpts.ProxyOptions = transport.ProxyOptions{
URL: options.GitHTTPProxyURL,
Expand Down
94 changes: 90 additions & 4 deletions git.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,20 @@ package envbuilder

import (
"context"
"crypto/md5"
"errors"
"fmt"
"io"
"net"
"net/http"
"net/url"
"os"
"strings"
"time"

"github.com/cenkalti/backoff/v4"
"github.com/coder/coder/v2/codersdk"
"github.com/coder/coder/v2/codersdk/agentsdk"
"github.com/go-git/go-billy/v5"
"github.com/go-git/go-git/v5"
"github.com/go-git/go-git/v5/plumbing"
Expand All @@ -22,7 +27,6 @@ import (
gitssh "github.com/go-git/go-git/v5/plumbing/transport/ssh"
"github.com/go-git/go-git/v5/storage/filesystem"
"github.com/skeema/knownhosts"
"golang.org/x/crypto/ssh"
gossh "golang.org/x/crypto/ssh"
)

Expand Down Expand Up @@ -174,10 +178,17 @@ func LogHostKeyCallback(log LoggerFunc) gossh.HostKeyCallback {
// If SSH_PRIVATE_KEY_PATH is set, an SSH private key will be read from
// that path and the SSH auth method will be configured with that key.
//
// If no SSH_PRIVATE_KEY_PATH is set, but CODER_AGENT_URL and CODER_AGENT_TOKEN
// are both specified, envbuilder will attempt to fetch the corresponding
// Git SSH key for the user.
//
// Otherwise, SSH authentication will fall back to SSH_AUTH_SOCK, in which
// case SSH_AUTH_SOCK must be set to the path of a listening SSH agent socket.
//
// If SSH_KNOWN_HOSTS is not set, the SSH auth method will be configured
// to accept and log all host keys. Otherwise, host key checking will be
// performed as usual.
func SetupRepoAuth(options *Options) transport.AuthMethod {
func SetupRepoAuth(ctx context.Context, options *Options) transport.AuthMethod {
if options.GitURL == "" {
options.Logger(codersdk.LogLevelInfo, "#1: ❔ No Git URL supplied!")
return nil
Expand Down Expand Up @@ -207,14 +218,29 @@ func SetupRepoAuth(options *Options) transport.AuthMethod {
// Assume SSH auth for all other formats.
options.Logger(codersdk.LogLevelInfo, "#1: 🔑 Using SSH authentication!")

var signer ssh.Signer
var signer gossh.Signer
if options.GitSSHPrivateKeyPath != "" {
s, err := ReadPrivateKey(options.GitSSHPrivateKeyPath)
if err != nil {
options.Logger(codersdk.LogLevelError, "#1: ❌ Failed to read private key from %s: %s", options.GitSSHPrivateKeyPath, err.Error())
} else {
options.Logger(codersdk.LogLevelInfo, "#1: 🔑 Using %s key!", s.PublicKey().Type())
signer = s
options.Logger(codersdk.LogLevelInfo, "#1: 🔑 Using %s key %s!", s.PublicKey().Type(), keyFingerprint(signer)[:8])
}
}

// If we have no signer but we have a Coder URL and agent token, try to fetch
// an SSH key from Coder!
if signer == nil && options.CoderAgentURL != "" && options.CoderAgentToken != "" {
options.Logger(codersdk.LogLevelInfo, "#1: 🔑 Fetching key from %s!", options.CoderAgentURL)
fetchCtx, cancel := context.WithCancel(ctx)
defer cancel()
s, err := FetchCoderSSHKeyRetry(fetchCtx, options.Logger, options.CoderAgentURL, options.CoderAgentToken)
if err == nil {
signer = s
options.Logger(codersdk.LogLevelInfo, "#1: 🔑 Fetched %s key %s !", signer.PublicKey().Type(), keyFingerprint(signer)[:8])
} else {
options.Logger(codersdk.LogLevelInfo, "#1: ❌ Failed to fetch SSH key from %s: %w", options.CoderAgentURL, err)
}
Comment on lines +234 to 244
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deeply integrating the hot-path code with Coder feels incorrect to me.

I don't believe the envbuilder package should have any knowledge of the Coder agent or Coder URL. Prior to the CLI change, it didn't. The command-line simply logged to Coder as a sink if those vars were provided, and I think implementing this similarly abstractly would be an improvement.

We want people outside of the Coder ecosystem to use envbuilder. By integrating the codebase so heavily into Coder, we alienate that possibility.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My preference would be to allow passing SSH options to envbuilder, and also not provide Coder options directly to envbuilder.

Copy link
Member Author

@johnstcn johnstcn May 3, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about instead exposing some 'lifecycle hooks' inside envbuilder?
For example, we could expose a 'pre-clone' hook that executes a given script before cloning the git repository. For this use-case, that script could curl | jq to get the git private key. It would require modifying the envbuilder image, but I think this provides the decoupling and flexibility you're looking for.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adopting "lifecycle hooks" is a reasonable concept, we may argue where they should be scripts or just shell commands. Maybe we can implement them similarly to Git Hooks?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any thoughts as to what kind of hook, except pre-clone, we might need in envbuilder? Running scripts is generally already addressable by devcontainer.json.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I think you're right that another data source for the coder_user is good, but shouldn't block this effort.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mentioning that in coder/terraform-provider-coder#217

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we added a new data source to the coder provider, I believe we can address this similarly to what @kylecarbs suggested:

container {
  env = {
    GIT_SSH_KEY: basy64(data.coder_workspace.owner_ssh_key)
  }
}

👍 That's certainly a valid path forward.

Based on what was implemented in #170, there would be a few extra steps:

  • Create a secret containing the user's private key
  • Mount that secret into the container under a well-known path
  • Reference that path in the container's environment

Will open a separate PR for this.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@matifali it's funny you should mention that, it was my initial thought as well, but then I got lazy 😄. Perhaps we could move this to a separate issue in https://github.com/coder/terraform-provider-coder?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Already moved. See
coder/terraform-provider-coder#219

}

Expand Down Expand Up @@ -251,3 +277,63 @@ func SetupRepoAuth(options *Options) transport.AuthMethod {
}
return auth
}

// FetchCoderSSHKeyRetry wraps FetchCoderSSHKey in backoff.Retry.
// Retries are attempted if Coder responds with a 401 Unauthorized.
// This indicates that the workspace build has not yet completed.
// It will retry for up to 1 minute with exponential backoff.
// Any other error is considered a permanent failure.
func FetchCoderSSHKeyRetry(ctx context.Context, log LoggerFunc, coderURL, agentToken string) (gossh.Signer, error) {
ctx, cancel := context.WithCancel(ctx)
defer cancel()

signerChan := make(chan gossh.Signer, 1)
eb := backoff.NewExponentialBackOff()
eb.MaxElapsedTime = 0
eb.MaxInterval = time.Minute
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

review: should this maybe be a configurable knob?

bkoff := backoff.WithContext(eb, ctx)
err := backoff.Retry(func() error {
s, err := FetchCoderSSHKey(ctx, coderURL, agentToken)
if err != nil {
var sdkErr *codersdk.Error
if errors.As(err, &sdkErr) && sdkErr.StatusCode() == http.StatusUnauthorized {
// Retry, as this may just mean that the workspace build has not yet
// completed.
log(codersdk.LogLevelInfo, "#1: 🕐 Backing off as the workspace build has not yet completed...")
return err
}
close(signerChan)
Comment on lines +299 to +305
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

review: I'm open to adding more cases here, but keeping the retry scope small for now.

return backoff.Permanent(err)
}
signerChan <- s
return nil
}, bkoff)
return <-signerChan, err
}

// FetchCoderSSHKey fetches the user's Git SSH key from Coder using the supplied
// Coder URL and agent token.
func FetchCoderSSHKey(ctx context.Context, coderURL string, agentToken string) (gossh.Signer, error) {
u, err := url.Parse(coderURL)
if err != nil {
return nil, fmt.Errorf("invalid Coder URL: %w", err)
}
client := agentsdk.New(u)
client.SetSessionToken(agentToken)
key, err := client.GitSSHKey(ctx)
if err != nil {
return nil, fmt.Errorf("get coder ssh key: %w", err)
}
signer, err := gossh.ParsePrivateKey([]byte(key.PrivateKey))
if err != nil {
return nil, fmt.Errorf("parse coder ssh key: %w", err)
}
return signer, nil
}

// keyFingerprint returns the md5 checksum of the public key of signer.
func keyFingerprint(s gossh.Signer) string {
h := md5.New()
h.Write(s.PublicKey().Marshal())
return fmt.Sprintf("%x", h.Sum(nil))
}
Comment on lines +334 to +339
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an unrelated change, but I figured it would be nice to see

112 changes: 102 additions & 10 deletions git_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,23 +3,29 @@ package envbuilder_test
import (
"context"
"crypto/ed25519"
"encoding/json"
"fmt"
"io"
"net/http"
"net/http/httptest"
"net/url"
"os"
"path/filepath"
"regexp"
"sync/atomic"
"testing"

"github.com/coder/coder/v2/codersdk"
"github.com/coder/coder/v2/codersdk/agentsdk"
"github.com/coder/envbuilder"
"github.com/coder/envbuilder/testutil/gittest"
"github.com/go-git/go-billy/v5"
"github.com/go-git/go-billy/v5/memfs"
"github.com/go-git/go-billy/v5/osfs"
githttp "github.com/go-git/go-git/v5/plumbing/transport/http"
gitssh "github.com/go-git/go-git/v5/plumbing/transport/ssh"
"github.com/google/uuid"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
gossh "golang.org/x/crypto/ssh"
)
Expand Down Expand Up @@ -263,11 +269,12 @@ func TestCloneRepoSSH(t *testing.T) {
// nolint:paralleltest // t.Setenv for SSH_AUTH_SOCK
func TestSetupRepoAuth(t *testing.T) {
t.Setenv("SSH_AUTH_SOCK", "")
ctx := context.Background()
t.Run("Empty", func(t *testing.T) {
opts := &envbuilder.Options{
Logger: testLog(t),
}
auth := envbuilder.SetupRepoAuth(opts)
auth := envbuilder.SetupRepoAuth(ctx, opts)
require.Nil(t, auth)
})

Expand All @@ -276,7 +283,7 @@ func TestSetupRepoAuth(t *testing.T) {
GitURL: "http://host.tld/repo",
Logger: testLog(t),
}
auth := envbuilder.SetupRepoAuth(opts)
auth := envbuilder.SetupRepoAuth(ctx, opts)
require.Nil(t, auth)
})

Expand All @@ -287,7 +294,7 @@ func TestSetupRepoAuth(t *testing.T) {
GitPassword: "pass",
Logger: testLog(t),
}
auth := envbuilder.SetupRepoAuth(opts)
auth := envbuilder.SetupRepoAuth(ctx, opts)
ba, ok := auth.(*githttp.BasicAuth)
require.True(t, ok)
require.Equal(t, opts.GitUsername, ba.Username)
Expand All @@ -301,7 +308,7 @@ func TestSetupRepoAuth(t *testing.T) {
GitPassword: "pass",
Logger: testLog(t),
}
auth := envbuilder.SetupRepoAuth(opts)
auth := envbuilder.SetupRepoAuth(ctx, opts)
ba, ok := auth.(*githttp.BasicAuth)
require.True(t, ok)
require.Equal(t, opts.GitUsername, ba.Username)
Expand All @@ -315,7 +322,7 @@ func TestSetupRepoAuth(t *testing.T) {
GitSSHPrivateKeyPath: kPath,
Logger: testLog(t),
}
auth := envbuilder.SetupRepoAuth(opts)
auth := envbuilder.SetupRepoAuth(ctx, opts)
_, ok := auth.(*gitssh.PublicKeys)
require.True(t, ok)
})
Expand All @@ -327,7 +334,7 @@ func TestSetupRepoAuth(t *testing.T) {
GitSSHPrivateKeyPath: kPath,
Logger: testLog(t),
}
auth := envbuilder.SetupRepoAuth(opts)
auth := envbuilder.SetupRepoAuth(ctx, opts)
_, ok := auth.(*gitssh.PublicKeys)
require.True(t, ok)
})
Expand All @@ -340,7 +347,7 @@ func TestSetupRepoAuth(t *testing.T) {
GitSSHPrivateKeyPath: kPath,
Logger: testLog(t),
}
auth := envbuilder.SetupRepoAuth(opts)
auth := envbuilder.SetupRepoAuth(ctx, opts)
_, ok := auth.(*gitssh.PublicKeys)
require.True(t, ok)
})
Expand All @@ -353,7 +360,7 @@ func TestSetupRepoAuth(t *testing.T) {
GitUsername: "user",
Logger: testLog(t),
}
auth := envbuilder.SetupRepoAuth(opts)
auth := envbuilder.SetupRepoAuth(ctx, opts)
_, ok := auth.(*gitssh.PublicKeys)
require.True(t, ok)
})
Expand All @@ -365,7 +372,7 @@ func TestSetupRepoAuth(t *testing.T) {
GitSSHPrivateKeyPath: kPath,
Logger: testLog(t),
}
auth := envbuilder.SetupRepoAuth(opts)
auth := envbuilder.SetupRepoAuth(ctx, opts)
pk, ok := auth.(*gitssh.PublicKeys)
require.True(t, ok)
require.NotNil(t, pk.Signer)
Expand All @@ -379,9 +386,93 @@ func TestSetupRepoAuth(t *testing.T) {
GitURL: "ssh://[email protected]:repo/path",
Logger: testLog(t),
}
auth := envbuilder.SetupRepoAuth(opts)
auth := envbuilder.SetupRepoAuth(ctx, opts)
require.Nil(t, auth) // TODO: actually test SSH_AUTH_SOCK
})

t.Run("SSH/Coder", func(t *testing.T) {
token := uuid.NewString()
actualSigner, err := gossh.ParsePrivateKey([]byte(testKey))
require.NoError(t, err)
handler := func(w http.ResponseWriter, r *http.Request) {
hdr := r.Header.Get(codersdk.SessionTokenHeader)
if !assert.Equal(t, hdr, token) {
w.WriteHeader(http.StatusForbidden)
return
}
switch r.URL.Path {
case "/api/v2/workspaceagents/me/gitsshkey":
_ = json.NewEncoder(w).Encode(&agentsdk.GitSSHKey{
PublicKey: string(actualSigner.PublicKey().Marshal()),
PrivateKey: string(testKey),
})
default:
assert.Fail(t, "unknown path: %q", r.URL.Path)
}
}
srv := httptest.NewServer(http.HandlerFunc(handler))
opts := &envbuilder.Options{
CoderAgentURL: srv.URL,
CoderAgentToken: token,
GitURL: "ssh://[email protected]:repo/path",
Logger: testLog(t),
}
auth := envbuilder.SetupRepoAuth(ctx, opts)
pk, ok := auth.(*gitssh.PublicKeys)
require.True(t, ok)
require.NotNil(t, pk.Signer)
require.Equal(t, actualSigner, pk.Signer)
})

t.Run("SSH/CoderRetry", func(t *testing.T) {
token := uuid.NewString()
actualSigner, err := gossh.ParsePrivateKey([]byte(testKey))
require.NoError(t, err)
var count atomic.Int64
// Return 401 initially, but eventually 200.
handler := func(w http.ResponseWriter, r *http.Request) {
c := count.Add(1)
if c < 3 {
hdr := r.Header.Get(codersdk.SessionTokenHeader)
assert.Equal(t, hdr, token)
w.WriteHeader(http.StatusUnauthorized)
return
}
_ = json.NewEncoder(w).Encode(&agentsdk.GitSSHKey{
PublicKey: string(actualSigner.PublicKey().Marshal()),
PrivateKey: string(testKey),
})
}
srv := httptest.NewServer(http.HandlerFunc(handler))
opts := &envbuilder.Options{
CoderAgentURL: srv.URL,
CoderAgentToken: token,
GitURL: "ssh://[email protected]:repo/path",
Logger: testLog(t),
}
auth := envbuilder.SetupRepoAuth(ctx, opts)
pk, ok := auth.(*gitssh.PublicKeys)
require.True(t, ok)
require.NotNil(t, pk.Signer)
require.Equal(t, actualSigner, pk.Signer)
})

t.Run("SSH/NotCoder", func(t *testing.T) {
token := uuid.NewString()
handler := func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusTeapot)
_, _ = w.Write([]byte("I'm a teapot!"))
}
srv := httptest.NewServer(http.HandlerFunc(handler))
opts := &envbuilder.Options{
CoderAgentURL: srv.URL,
CoderAgentToken: token,
GitURL: "ssh://[email protected]:repo/path",
Logger: testLog(t),
}
auth := envbuilder.SetupRepoAuth(ctx, opts)
require.Nil(t, auth)
})
}

func mustRead(t *testing.T, fs billy.Filesystem, path string) string {
Expand All @@ -405,6 +496,7 @@ func randKeygen(t *testing.T) gossh.Signer {

func testLog(t *testing.T) envbuilder.LoggerFunc {
return func(_ codersdk.LogLevel, format string, args ...interface{}) {
t.Helper()
t.Logf(format, args...)
}
}
Expand Down
Loading