Skip to content

feat: validate ssh properties before launching workspaces #96

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

Merged
merged 1 commit into from
May 4, 2023
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
34 changes: 31 additions & 3 deletions src/remote.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import * as semver from "semver"
import * as vscode from "vscode"
import * as ws from "ws"
import { SSHConfig, SSHValues, defaultSSHConfigResponse, mergeSSHConfigValues } from "./sshConfig"
import { sshSupportsSetEnv } from "./sshSupport"
import { computeSSHProperties, sshSupportsSetEnv } from "./sshSupport"
import { Storage } from "./storage"

export class Remote {
Expand Down Expand Up @@ -411,7 +411,7 @@ export class Remote {
//
// If we didn't write to the SSH config file, connecting would fail with
// "Host not found".
await this.updateSSHConfig()
await this.updateSSHConfig(authorityParts[1])

this.findSSHProcessID().then((pid) => {
if (!pid) {
Expand Down Expand Up @@ -440,7 +440,7 @@ export class Remote {

// updateSSHConfig updates the SSH configuration with a wildcard that handles
// all Coder entries.
private async updateSSHConfig() {
private async updateSSHConfig(hostName: string) {
let deploymentSSHConfig = defaultSSHConfigResponse
try {
const deploymentConfig = await getDeploymentSSHConfig()
Expand Down Expand Up @@ -528,6 +528,34 @@ export class Remote {
}

await sshConfig.update(sshValues, sshConfigOverrides)

// A user can provide a "Host *" entry in their SSH config to add options
// to all hosts. We need to ensure that the options we set are not
// overridden by the user's config.
const computedProperties = computeSSHProperties(hostName, sshConfig.getRaw())
const keysToMatch: Array<keyof SSHValues> = ["ProxyCommand", "UserKnownHostsFile", "StrictHostKeyChecking"]
for (let i = 0; i < keysToMatch.length; i++) {
const key = keysToMatch[i]
if (computedProperties[key] === sshValues[key]) {
continue
}

const result = await this.vscodeProposed.window.showErrorMessage(
"Unexpected SSH Config Option",
{
useCustom: true,
modal: true,
detail: `Your SSH config is overriding the "${key}" property to "${computedProperties[key]}" when it expected "${sshValues[key]}" for the "${hostName}" host. Please fix this and try again!`,
},
"Reload Window",
)
if (result === "Reload Window") {
await this.reloadWindow()
}
await this.closeRemote()
}

return sshConfig.getRaw()
}

// showNetworkUpdates finds the SSH process ID that is being used by this
Expand Down
22 changes: 21 additions & 1 deletion src/sshSupport.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { it, expect } from "vitest"
import { sshSupportsSetEnv, sshVersionSupportsSetEnv } from "./sshSupport"
import { computeSSHProperties, sshSupportsSetEnv, sshVersionSupportsSetEnv } from "./sshSupport"

const supports = {
"OpenSSH_8.9p1 Ubuntu-3ubuntu0.1, OpenSSL 3.0.2 15 Mar 2022": true,
Expand All @@ -17,3 +17,23 @@ Object.entries(supports).forEach(([version, expected]) => {
it("current shell supports ssh", () => {
expect(sshSupportsSetEnv()).toBeTruthy()
})

it("computes the config for a host", () => {
const properties = computeSSHProperties(
"coder-vscode--testing",
`Host *
StrictHostKeyChecking yes

# --- START CODER VSCODE ---
Host coder-vscode--*
StrictHostKeyChecking no
Another=true
# --- END CODER VSCODE ---
`,
)

expect(properties).toEqual({
Another: "true",
StrictHostKeyChecking: "yes",
})
})
52 changes: 52 additions & 0 deletions src/sshSupport.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,3 +36,55 @@ export function sshVersionSupportsSetEnv(sshVersionString: string): boolean {
}
return false
}

// computeSSHProperties accepts an SSH config and a host name and returns
// the properties that should be set for that host.
export function computeSSHProperties(host: string, config: string): Record<string, string> {
let currentConfig:
| {
Host: string
properties: Record<string, string>
}
| undefined
const configs: Array<typeof currentConfig> = []
config.split("\n").forEach((line) => {
line = line.trim()
if (line === "") {
return
}
const [key, ...valueParts] = line.split(/\s+|=/)
if (key.startsWith("#")) {
// Ignore comments!
return
}
if (key === "Host") {
if (currentConfig) {
configs.push(currentConfig)
}
currentConfig = {
Host: valueParts.join(" "),
properties: {},
}
return
}
if (!currentConfig) {
return
}
currentConfig.properties[key] = valueParts.join(" ")
})
if (currentConfig) {
configs.push(currentConfig)
}

const merged: Record<string, string> = {}
configs.reverse().forEach((config) => {
if (!config) {
return
}
if (!new RegExp("^" + config?.Host.replace(/\*/g, ".*") + "$").test(host)) {
return
}
Object.assign(merged, config.properties)
})
return merged
}