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

chore(security): add context option to set StrictHostKeyChecking to yes for ssh based git operations #1612

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions cmd/agent/container/setup.go
Original file line number Diff line number Diff line change
Expand Up @@ -570,14 +570,14 @@ func configureSystemGitCredentials(ctx context.Context, cancel context.CancelFun
gitCredentials := fmt.Sprintf("!'%s' agent git-credentials --port %d", binaryPath, serverPort)
_ = os.Setenv("DEVPOD_GIT_HELPER_PORT", strconv.Itoa(serverPort))

err = git.CommandContext(ctx, "config", "--system", "--add", "credential.helper", gitCredentials).Run()
err = git.CommandContext(ctx, git.GitCommandOptions{}, "config", "--system", "--add", "credential.helper", gitCredentials).Run()
if err != nil {
return nil, fmt.Errorf("add git credential helper: %w", err)
}

cleanup := func() {
log.Debug("Unset setup system credential helper")
err = git.CommandContext(ctx, "config", "--system", "--unset", "credential.helper").Run()
err = git.CommandContext(ctx, git.GitCommandOptions{}, "config", "--system", "--unset", "credential.helper").Run()
if err != nil {
log.Errorf("unset system credential helper %v", err)
}
Expand Down
9 changes: 6 additions & 3 deletions cmd/agent/workspace/install_dotfiles.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,9 @@ import (
type InstallDotfilesCmd struct {
*flags.GlobalFlags

Repository string
InstallScript string
Repository string
InstallScript string
StrictHostKeyChecking bool
}

// NewInstallDotfilesCmd creates a new command
Expand All @@ -38,6 +39,7 @@ func NewInstallDotfilesCmd(flags *flags.GlobalFlags) *cobra.Command {
}
installDotfilesCmd.Flags().StringVar(&cmd.Repository, "repository", "", "The dotfiles repository")
installDotfilesCmd.Flags().StringVar(&cmd.InstallScript, "install-script", "", "The dotfiles install command to execute")
installDotfilesCmd.Flags().BoolVar(&cmd.StrictHostKeyChecking, "strict-host-key-checking", false, "Set to enable strict host key checking for git cloning via SSH")
return installDotfilesCmd
}

Expand All @@ -51,8 +53,9 @@ func (cmd *InstallDotfilesCmd) Run(ctx context.Context) error {
logger.Infof("Cloning dotfiles %s", cmd.Repository)

gitInfo := git.NormalizeRepositoryGitInfo(cmd.Repository)
gitOpts := git.GitCommandOptions{StrictHostKeyChecking: cmd.StrictHostKeyChecking}

if err := git.CloneRepository(ctx, gitInfo, targetDir, "", nil, logger); err != nil {
if err := git.CloneRepository(ctx, gitInfo, targetDir, gitOpts, "", nil, logger); err != nil {
return err
}
} else {
Expand Down
4 changes: 4 additions & 0 deletions cmd/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,10 @@ func NewBuildCmd(flags *flags.GlobalFlags) *cobra.Command {
}
}

if devPodConfig.ContextOption(config.ContextOptionSSHStrictHostKeyChecking) == "true" {
cmd.StrictHostKeyChecking = true
}

// create a temporary workspace
exists := workspace2.Exists(ctx, devPodConfig, args, "", log.Default)
sshConfigFile, err := os.CreateTemp("", "devpodssh.config")
Expand Down
18 changes: 14 additions & 4 deletions cmd/helper/get_workspace_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@ import (
"time"

"github.com/loft-sh/devpod/cmd/flags"
"github.com/loft-sh/devpod/pkg/config"
"github.com/loft-sh/devpod/pkg/devcontainer"
"github.com/loft-sh/devpod/pkg/git"
"github.com/loft-sh/log"
"github.com/sirupsen/logrus"
"github.com/spf13/cobra"
Expand All @@ -35,13 +37,18 @@ func NewGetWorkspaceConfigCommand(flags *flags.GlobalFlags) *cobra.Command {
shellCmd := &cobra.Command{
Use: "get-workspace-config",
Short: "Retrieves a workspace config",
RunE: func(_ *cobra.Command, args []string) error {
RunE: func(cobraCmd *cobra.Command, args []string) error {
devPodConfig, err := config.LoadConfig(cmd.Context, cmd.Provider)
if err != nil {
return err
}

if cmd.maxDepth < 0 {
log.Default.Debugf("--max-depth was %d, setting to 0", cmd.maxDepth)
cmd.maxDepth = 0
}

return cmd.Run(context.Background(), args)
return cmd.Run(cobraCmd.Context(), devPodConfig, args)
},
}

Expand All @@ -51,7 +58,7 @@ func NewGetWorkspaceConfigCommand(flags *flags.GlobalFlags) *cobra.Command {
return shellCmd
}

func (cmd *GetWorkspaceConfigCommand) Run(ctx context.Context, args []string) error {
func (cmd *GetWorkspaceConfigCommand) Run(ctx context.Context, devPodConfig *config.Config, args []string) error {
if len(args) != 1 {
return fmt.Errorf("workspace source is missing")
}
Expand All @@ -78,7 +85,10 @@ func (cmd *GetWorkspaceConfigCommand) Run(ctx context.Context, args []string) er
_ = os.RemoveAll(tmpDir)
}()
go func() {
result, err := devcontainer.FindDevcontainerFiles(ctx, rawSource, tmpDir, cmd.maxDepth, logger)
gitOpts := git.GitCommandOptions{
StrictHostKeyChecking: devPodConfig.ContextOption(config.ContextOptionSSHStrictHostKeyChecking) == "true",
}
result, err := devcontainer.FindDevcontainerFiles(ctx, rawSource, tmpDir, cmd.maxDepth, gitOpts, logger)
if err != nil {
errChan <- err
return
Expand Down
16 changes: 12 additions & 4 deletions cmd/up.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,10 @@ func NewUpCmd(f *flags.GlobalFlags) *cobra.Command {
return err
}

if devPodConfig.ContextOption(config.ContextOptionSSHStrictHostKeyChecking) == "true" {
cmd.StrictHostKeyChecking = true
}

ctx := cobraCmd.Context()
client, logger, err := cmd.prepareClient(ctx, devPodConfig, args)
if err != nil {
Expand Down Expand Up @@ -1104,7 +1108,7 @@ func setupDotfiles(
log.Infof("Dotfiles git repository %s specified", dotfilesRepo)
log.Debug("Cloning dotfiles into the devcontainer...")

dotCmd, err := buildDotCmd(dotfilesRepo, dotfilesScript, envFiles, envKeyValuePairs, devPodConfig, client, log)
dotCmd, err := buildDotCmd(devPodConfig, dotfilesRepo, dotfilesScript, envFiles, envKeyValuePairs, client, log)
if err != nil {
return err
}
Expand All @@ -1129,7 +1133,7 @@ func setupDotfiles(
return nil
}

func buildDotCmdAgentArguments(dotfilesRepo, dotfilesScript string, log log.Logger) []string {
func buildDotCmdAgentArguments(devPodConfig *config.Config, dotfilesRepo, dotfilesScript string, log log.Logger) []string {
agentArguments := []string{
"agent",
"workspace",
Expand All @@ -1138,6 +1142,10 @@ func buildDotCmdAgentArguments(dotfilesRepo, dotfilesScript string, log log.Logg
dotfilesRepo,
}

if devPodConfig.ContextOption(config.ContextOptionSSHStrictHostKeyChecking) == "true" {
agentArguments = append(agentArguments, "--strict-host-key-checking")
}

if log.GetLevel() == logrus.DebugLevel {
agentArguments = append(agentArguments, "--debug")
}
Expand All @@ -1150,7 +1158,7 @@ func buildDotCmdAgentArguments(dotfilesRepo, dotfilesScript string, log log.Logg
return agentArguments
}

func buildDotCmd(dotfilesRepo, dotfilesScript string, envFiles, envKeyValuePairs []string, devPodConfig *config.Config, client client2.BaseWorkspaceClient, log log.Logger) (*exec.Cmd, error) {
func buildDotCmd(devPodConfig *config.Config, dotfilesRepo, dotfilesScript string, envFiles, envKeyValuePairs []string, client client2.BaseWorkspaceClient, log log.Logger) (*exec.Cmd, error) {
sshCmd := []string{
"ssh",
"--agent-forwarding=true",
Expand All @@ -1175,7 +1183,7 @@ func buildDotCmd(dotfilesRepo, dotfilesScript string, envFiles, envKeyValuePairs
remoteUser = "root"
}

agentArguments := buildDotCmdAgentArguments(dotfilesRepo, dotfilesScript, log)
agentArguments := buildDotCmdAgentArguments(devPodConfig, dotfilesRepo, dotfilesScript, log)
sshCmd = append(sshCmd,
"--user",
remoteUser,
Expand Down
1 change: 0 additions & 1 deletion pkg/agent/tunnel/tunnel.proto
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ service Tunnel {
rpc ForwardPort(ForwardPortRequest) returns (ForwardPortResponse) {}
rpc StopForwardPort(StopForwardPortRequest) returns (StopForwardPortResponse) {}

rpc StreamGitClone(Empty) returns (stream Chunk) {}
rpc StreamWorkspace(Empty) returns (stream Chunk) {}
rpc StreamMount(StreamMountRequest) returns (stream Chunk) {}
}
Expand Down
17 changes: 0 additions & 17 deletions pkg/agent/tunnelserver/proxyserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,23 +134,6 @@ func (t *proxyServer) Log(ctx context.Context, message *tunnel.LogMessage) (*tun
return t.client.Log(ctx, message)
}

func (t *proxyServer) StreamGitClone(message *tunnel.Empty, stream tunnel.Tunnel_StreamGitCloneServer) error {
t.log.Debug("Cloning and reading workspace")
client, err := t.client.StreamGitClone(context.TODO(), &tunnel.Empty{})
if err != nil {
return err
}

buf := bufio.NewWriterSize(NewStreamWriter(stream, t.log), 10*1024)
_, err = io.Copy(buf, NewStreamReader(client, t.log))
if err != nil {
return err
}

// make sure buffer is flushed
return buf.Flush()
}

func (t *proxyServer) StreamWorkspace(message *tunnel.Empty, stream tunnel.Tunnel_StreamWorkspaceServer) error {
t.log.Debug("Start reading workspace")

Expand Down
70 changes: 0 additions & 70 deletions pkg/agent/tunnelserver/tunnelserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import (
"github.com/loft-sh/devpod/pkg/devcontainer/config"
"github.com/loft-sh/devpod/pkg/dockercredentials"
"github.com/loft-sh/devpod/pkg/extract"
"github.com/loft-sh/devpod/pkg/git"
"github.com/loft-sh/devpod/pkg/gitcredentials"
"github.com/loft-sh/devpod/pkg/gitsshsigning"
"github.com/loft-sh/devpod/pkg/gpg"
Expand Down Expand Up @@ -335,75 +334,6 @@ func (t *tunnelServer) Log(ctx context.Context, message *tunnel.LogMessage) (*tu
return &tunnel.Empty{}, nil
}

func (t *tunnelServer) StreamGitClone(message *tunnel.Empty, stream tunnel.Tunnel_StreamGitCloneServer) error {
if t.workspace == nil {
return fmt.Errorf("workspace is nil")
} else if t.workspace.Source.GitRepository == "" {
return fmt.Errorf("invalid repository")
}

// clone here
tempDir, err := os.MkdirTemp("", "devpod-git-clone-*")
if err != nil {
return fmt.Errorf("create temp dir: %w", err)
}
defer os.RemoveAll(tempDir)

// clone repository
cloneArgs := []string{"clone", t.workspace.Source.GitRepository, tempDir}
if t.workspace.Source.GitBranch != "" {
cloneArgs = append(cloneArgs, "--branch", t.workspace.Source.GitBranch)
}

// run command
err = git.CommandContext(context.Background(), cloneArgs...).Run()
if err != nil {
return err
}

if t.workspace.Source.GitPRReference != "" {
prBranch := git.GetBranchNameForPR(t.workspace.Source.GitPRReference)

// git fetch origin pull/996/head:PR996
fetchArgs := []string{"fetch", "origin", t.workspace.Source.GitPRReference + ":" + prBranch}
fetchCmd := git.CommandContext(context.Background(), fetchArgs...)
fetchCmd.Dir = tempDir
err = fetchCmd.Run()
if err != nil {
return err
}

// git switch PR996
switchArgs := []string{"switch", prBranch}
switchCmd := git.CommandContext(context.Background(), switchArgs...)
switchCmd.Dir = tempDir
err = switchCmd.Run()
if err != nil {
return err
}
} else if t.workspace.Source.GitCommit != "" {
// reset here
// git reset --hard $COMMIT_SHA
resetArgs := []string{"reset", "--hard", t.workspace.Source.GitCommit}
resetCmd := git.CommandContext(context.Background(), resetArgs...)
resetCmd.Dir = tempDir

err = resetCmd.Run()
if err != nil {
return err
}
}

buf := bufio.NewWriterSize(NewStreamWriter(stream, t.log), 10*1024)
err = extract.WriteTar(buf, tempDir, false)
if err != nil {
return err
}

// make sure buffer is flushed
return buf.Flush()
}

func (t *tunnelServer) StreamWorkspace(message *tunnel.Empty, stream tunnel.Tunnel_StreamWorkspaceServer) error {
if t.workspace == nil {
return fmt.Errorf("workspace is nil")
Expand Down
3 changes: 2 additions & 1 deletion pkg/agent/workspace.go
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,8 @@ func CloneRepositoryForWorkspace(
// run git command
cloner := git.NewClonerWithOpts(getGitOptions(options)...)
gitInfo := git.NewGitInfo(source.GitRepository, source.GitBranch, source.GitCommit, source.GitPRReference, source.GitSubPath)
err := git.CloneRepositoryWithEnv(ctx, gitInfo, extraEnv, workspaceDir, helper, cloner, log)
gitOpts := git.GitCommandOptions{StrictHostKeyChecking: options.StrictHostKeyChecking}
err := git.CloneRepositoryWithEnv(ctx, gitInfo, extraEnv, workspaceDir, gitOpts, helper, cloner, log)
if err != nil {
// cleanup workspace dir if clone failed, otherwise we won't try to clone again when rebuilding this workspace
if cleanupErr := cleanupWorkspaceDir(workspaceDir); cleanupErr != nil {
Expand Down
7 changes: 7 additions & 0 deletions pkg/config/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ const (
ContextOptionSSHConfigPath = "SSH_CONFIG_PATH"
ContextOptionAgentInjectTimeout = "AGENT_INJECT_TIMEOUT"
ContextOptionRegistryCache = "REGISTRY_CACHE"
ContextOptionSSHStrictHostKeyChecking = "SSH_STRICT_HOST_KEY_CHECKING"
)

var ContextOptions = []ContextOption{
Expand Down Expand Up @@ -98,6 +99,12 @@ var ContextOptions = []ContextOption{
Description: "Specifies the registry to use as a build cache, e.g. gcr.io/my-project/my-dev-env",
Default: "",
},
{
Name: ContextOptionSSHStrictHostKeyChecking,
Description: "Enables strict ssh host key checking for all operations",
Default: "false",
Enum: []string{"true", "false"},
},
}

func MergeContextOptions(contextConfig *ContextConfig, environ []string) {
Expand Down
10 changes: 5 additions & 5 deletions pkg/devcontainer/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ type GetWorkspaceConfigResult struct {
ConfigPaths []string `json:"configPaths"`
}

func FindDevcontainerFiles(ctx context.Context, rawSource, tmpDirPath string, maxDepth int, log log.Logger) (*GetWorkspaceConfigResult, error) {
func FindDevcontainerFiles(ctx context.Context, rawSource, tmpDirPath string, maxDepth int, gitOpts git.GitCommandOptions, log log.Logger) (*GetWorkspaceConfigResult, error) {
// local path
isLocalPath, _ := file.IsLocalDir(rawSource)
if isLocalPath {
Expand All @@ -31,7 +31,7 @@ func FindDevcontainerFiles(ctx context.Context, rawSource, tmpDirPath string, ma
gitRepository, gitPRReference, gitBranch, gitCommit, gitSubDir := git.NormalizeRepository(rawSource)
if strings.HasSuffix(rawSource, ".git") || git.PingRepository(gitRepository) {
log.Debug("Git repository detected")
return FindFilesInGitRepo(ctx, gitRepository, gitPRReference, gitBranch, gitCommit, gitSubDir, tmpDirPath, maxDepth, log)
return FindFilesInGitRepo(ctx, gitRepository, gitPRReference, gitBranch, gitCommit, gitSubDir, tmpDirPath, gitOpts, maxDepth, log)
}

result := &GetWorkspaceConfigResult{ConfigPaths: []string{}}
Expand Down Expand Up @@ -81,15 +81,15 @@ func FindFilesInLocalDir(rawSource string, maxDepth int, log log.Logger) (*GetWo
return result, nil
}

func FindFilesInGitRepo(ctx context.Context, gitRepository, gitPRReference, gitBranch, gitCommit, gitSubDir, tmpDirPath string, maxDepth int, log log.Logger) (*GetWorkspaceConfigResult, error) {
func FindFilesInGitRepo(ctx context.Context, gitRepository, gitPRReference, gitBranch, gitCommit, gitSubDir, tmpDirPath string, gitOpts git.GitCommandOptions, maxDepth int, log log.Logger) (*GetWorkspaceConfigResult, error) {
result := &GetWorkspaceConfigResult{
ConfigPaths: []string{},
IsGitRepository: true,
}

gitInfo := git.NewGitInfo(gitRepository, gitBranch, gitCommit, gitPRReference, gitSubDir)
log.Debugf("Cloning git repository into %s", tmpDirPath)
err := git.CloneRepository(ctx, gitInfo, tmpDirPath, "", git.NewCloner(git.BareCloneStrategy), log)
err := git.CloneRepository(ctx, gitInfo, tmpDirPath, gitOpts, "", git.NewCloner(git.BareCloneStrategy), log)
if err != nil {
return nil, err
}
Expand All @@ -102,7 +102,7 @@ func FindFilesInGitRepo(ctx context.Context, gitRepository, gitPRReference, gitB
}
// git ls-tree -r --full-name --name-only $REF
lsArgs := []string{"ls-tree", "-r", "--full-name", "--name-only", ref}
lsCmd := git.CommandContext(ctx, lsArgs...)
lsCmd := git.CommandContext(ctx, gitOpts, lsArgs...)
lsCmd.Dir = tmpDirPath
stdout, err := lsCmd.StdoutPipe()
if err != nil {
Expand Down
10 changes: 5 additions & 5 deletions pkg/git/clone.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ const (
)

type Cloner interface {
Clone(ctx context.Context, repository string, targetDir string, extraArgs, extraEnv []string, log log.Logger) error
Clone(ctx context.Context, repository string, targetDir string, gitOpts GitCommandOptions, extraArgs, extraEnv []string, log log.Logger) error
}

type Option func(*cloner)
Expand Down Expand Up @@ -100,20 +100,20 @@ func (c *cloner) initialArgs() []string {
return []string{"clone"}
}

func (c *cloner) Clone(ctx context.Context, repository string, targetDir string, extraArgs, extraEnv []string, log log.Logger) error {
func (c *cloner) Clone(ctx context.Context, repository string, targetDir string, gitOpts GitCommandOptions, extraArgs, extraEnv []string, log log.Logger) error {
args := c.initialArgs()
args = append(args, extraArgs...)
args = append(args, c.extraArgs...)
args = append(args, repository, targetDir)
return run(ctx, args, extraEnv, log)
return run(ctx, args, gitOpts, extraEnv, log)
}

func run(ctx context.Context, args []string, extraEnv []string, log log.Logger) error {
func run(ctx context.Context, args []string, gitOpts GitCommandOptions, extraEnv []string, log log.Logger) error {
var buf bytes.Buffer

args = append(args, "--progress")

gitCommand := CommandContext(ctx, args...)
gitCommand := CommandContext(ctx, gitOpts, args...)
gitCommand.Stdout = &buf
gitCommand.Stderr = &buf
gitCommand.Env = append(gitCommand.Env, extraEnv...)
Expand Down
Loading
Loading