Add GSSAPI/Kerberos SSH authentication support#1382
Add GSSAPI/Kerberos SSH authentication support#1382liujunandzhou wants to merge 1 commit intogeneralaction:mainfrom
Conversation
Add a fourth SSH authentication method (GSSAPI/Kerberos) alongside password, SSH key, and SSH agent. Since the ssh2 library doesn't support GSSAPI natively, connections use the system's OpenSSH client with ControlMaster sockets for persistent, multiplexed sessions. Changes: - Add 'gssapi' to SshConfig.authType union across shared types, electron-api.d.ts, and all UI components - Add Kerberos card + setup info panel to AddRemoteProjectModal wizard and SshConnectionForm - Implement ControlMaster-based GSSAPI connections in SshService (connect, executeCommand, disconnect via system ssh) - Add GSSAPI test handler in sshIpc using system ssh with verbose output - Add command-based file operations (ls/cat/base64) for GSSAPI connections where SFTP is unavailable - Update RemotePtyService to spawn ssh -tt via node-pty for GSSAPI shell sessions instead of ssh2 client.shell() - Add 5 unit tests covering GSSAPI connect, auth failure, SFTP error, command execution, and connection info https://claude.ai/code/session_017956U4sRFiAYbRmqujqTEV
|
@claude is attempting to deploy a commit to the General Action Team on Vercel. A member of the Team first needs to authorize it. |
Greptile SummaryThis PR adds GSSAPI/Kerberos SSH authentication by delegating to the system's OpenSSH client (via Key issues found:
Confidence Score: 2/5
|
| Filename | Overview |
|---|---|
| src/main/services/ssh/SshService.ts | Core GSSAPI implementation using system ssh with ControlMaster; has issues with stale process tracking after ssh -f daemonizes and no connection establishment timeout. |
| src/main/ipc/sshIpc.ts | GSSAPI file operations via shell commands; symlink names are incorrectly parsed (including -> target), --time-style=+%s breaks on macOS/BSD remote servers, and binary file reads via cat will corrupt data. |
| src/main/services/RemotePtyService.ts | GSSAPI PTY support via node-pty + ControlMaster looks correct; shell allowlist and env var validation are properly reused; initialPrompt send via setTimeout is a minor fragility. |
| src/main/services/ssh/types.ts | Adds GssapiConnection interface — clean, well-typed definition with no issues. |
| src/main/services/ssh/tests/SshService.test.ts | Good test coverage for GSSAPI paths including success, auth failure, SFTP rejection, command execution, and connection info; mock setup is sound. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[User selects GSSAPI auth] --> B[SshService.connect]
B --> C{authType?}
C -- gssapi --> D[connectGssapi: spawn system ssh with ControlMaster flags]
C -- other --> E[createConnection via ssh2 library]
D --> F{ssh exits with code 0?}
F -- yes --> G[Store GssapiConnection + socket path]
F -- no --> H[Reject with stderr error message]
G --> I[Connection ready]
I --> J{Operation type?}
J -- executeCommand --> K[execFile ssh via ControlMaster socket]
J -- startPty --> L[node-pty spawn ssh -tt via ControlMaster socket]
J -- getSftp --> M[Throw: SFTP not supported for GSSAPI]
J -- listFiles --> N[execFile ls -la via executeCommand]
J -- readFile --> O[execFile cat via executeCommand]
J -- writeFile --> P[base64-encode content, pipe to file via executeCommand]
J -- disconnect --> Q[execFile ssh -O exit to close ControlMaster]
Q --> R[unlink socket file]
R --> S[Emit disconnected]
Comments Outside Diff (1)
-
src/main/ipc/sshIpc.ts, line 150-160 (link)Binary file content will be corrupted via
catexecFileby default decodes stdout as a UTF-8 string. Reading binary files throughcatwill corrupt any bytes that are not valid UTF-8 (replacement characters, truncated sequences, etc.). The SFTP path used by other auth types handles binary data correctly as aBuffer.This is an inherent limitation of the command-based approach, but callers and UI should be aware that GSSAPI
readFileis safe only for text files. A safer alternative for binary files would be to pipe throughbase64on the remote side (e.g.,cat file | base64) and decode on the Node side, similar to howwriteFilealready uses base64 encoding.
Last reviewed commit: ee18696
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const match = line.match( | ||
| /^([dlscp-])([rwxsStT-]{9})\s+\d+\s+\S+\s+\S+\s+(\d+)\s+(\d+)\s+(.+)$/ | ||
| ); | ||
| if (!match) continue; | ||
| const [, typeChar, perms, sizeStr, mtimeStr, name] = match; | ||
| if (name === '.' || name === '..') continue; |
There was a problem hiding this comment.
Symlink names include -> target path
When ls -la lists a symlink, the output format is lrwxrwxrwx ... name -> /path/to/target. The regex captures (.+) for the name field, which means symlink entries will have names like "somefile -> /etc/passwd" instead of just "somefile". This produces incorrect name and path values for every symlink in the directory.
A fix would be to strip the arrow and target from symlink names:
| const match = line.match( | |
| /^([dlscp-])([rwxsStT-]{9})\s+\d+\s+\S+\s+\S+\s+(\d+)\s+(\d+)\s+(.+)$/ | |
| ); | |
| if (!match) continue; | |
| const [, typeChar, perms, sizeStr, mtimeStr, name] = match; | |
| if (name === '.' || name === '..') continue; | |
| const [, typeChar, perms, sizeStr, mtimeStr, name] = match; | |
| const displayName = typeChar === 'l' ? name.replace(/ -> .+$/, '') : name; | |
| if (displayName === '.' || displayName === '..') continue; |
And use displayName in the entries.push() call instead of name:
entries.push({
path: `${path}/${displayName}`.replace(/\/+/g, '/'),
name: displayName,
...
| const result = await sshService.executeCommand( | ||
| connectionId, | ||
| `ls -la --time-style=+%s ${quoteShellArg(path)} 2>/dev/null` |
There was a problem hiding this comment.
--time-style is a GNU ls extension that breaks on macOS/BSD remote servers
--time-style=+%s is specific to GNU coreutils ls and is not available on macOS or BSD systems. If the remote host runs macOS, this command will exit non-zero with ls: illegal option -- - (the long option form is not supported), causing all GSSAPI directory listings to fail with a generic "Failed to list files" error.
Consider a more portable approach using stat for timestamps, or fall back gracefully. For example, using ls -la without --time-style and then separately fetching timestamps, or checking uname at connection time. At minimum, the error message could hint at the root cause when this fails.
| try { | ||
| await new Promise<void>((resolve) => { | ||
| execFile( | ||
| 'ssh', | ||
| ['-S', conn.controlSocketPath, '-O', 'exit', conn.config.host], | ||
| { timeout: 5000 }, | ||
| () => resolve() // Ignore errors, best-effort | ||
| ); | ||
| }); | ||
| } catch { | ||
| // Best-effort cleanup | ||
| } | ||
|
|
||
| // Clean up socket file | ||
| try { | ||
| await unlink(conn.controlSocketPath); | ||
| } catch { | ||
| // Ignore | ||
| } | ||
|
|
||
| // Kill any lingering process | ||
| const proc = this.gssapiProcesses.get(connectionId); | ||
| if (proc && !proc.killed) { | ||
| proc.kill(); | ||
| } | ||
|
|
||
| this.gssapiProcesses.delete(connectionId); | ||
| this.gssapiConnections.delete(connectionId); | ||
| this.emit('disconnected', connectionId); | ||
| } | ||
|
|
||
| /** | ||
| * Disconnects an existing SSH connection. | ||
| * @param connectionId - ID of the connection to close | ||
| */ | ||
| async disconnect(connectionId: string): Promise<void> { | ||
| // Handle GSSAPI connections | ||
| if (this.gssapiConnections.has(connectionId)) { | ||
| return this.disconnectGssapi(connectionId); | ||
| } | ||
|
|
||
| const connection = this.connections[connectionId]; |
There was a problem hiding this comment.
gssapiProcesses stores a reference to an already-exited process
ssh -f -N -M is designed to daemonize — after authentication succeeds, the foreground process forks into the background and exits with code 0. The proc reference captured from spawn() is the foreground parent, which has already exited by the time close fires with code 0. Storing it in gssapiProcesses and later calling proc.kill() in disconnectGssapi() means you're killing an already-dead process (which is harmless), but in a tight system, the PID could theoretically be reused by an unrelated process.
The real ControlMaster background process is correctly shut down via ssh -S ... -O exit .... The gssapiProcesses map and the proc.kill() call in disconnectGssapi do not actually target the ControlMaster — they are redundant at best and misleading at worst. Consider removing the gssapiProcesses tracking entirely and relying solely on the -O exit signal for cleanup.
| conn.config.username, | ||
| conn.config.host, | ||
| fullCommand, | ||
| ], | ||
| { timeout: 30000, maxBuffer: 10 * 1024 * 1024 }, | ||
| (err, stdout, stderr) => { | ||
| if (err && 'code' in err && typeof (err as any).code === 'number') { | ||
| // Command exited with non-zero code, still a valid result | ||
| resolve({ | ||
| stdout: (stdout || '').trim(), | ||
| stderr: (stderr || '').trim(), | ||
| exitCode: (err as any).code as number, | ||
| }); | ||
| } else if (err) { | ||
| reject(err); | ||
| } else { | ||
| resolve({ | ||
| stdout: (stdout || '').trim(), | ||
| stderr: (stderr || '').trim(), | ||
| exitCode: 0, | ||
| }); | ||
| } | ||
| } | ||
| ); | ||
| }); | ||
| } | ||
|
|
||
| /** | ||
| * Disconnects a GSSAPI connection by terminating the ControlMaster. | ||
| */ | ||
| private async disconnectGssapi(connectionId: string): Promise<void> { | ||
| const conn = this.gssapiConnections.get(connectionId); | ||
| if (!conn) return; | ||
|
|
||
| // Send exit command to ControlMaster | ||
| try { | ||
| await new Promise<void>((resolve) => { | ||
| execFile( | ||
| 'ssh', | ||
| ['-S', conn.controlSocketPath, '-O', 'exit', conn.config.host], | ||
| { timeout: 5000 }, | ||
| () => resolve() // Ignore errors, best-effort | ||
| ); | ||
| }); | ||
| } catch { | ||
| // Best-effort cleanup | ||
| } | ||
|
|
||
| // Clean up socket file | ||
| try { | ||
| await unlink(conn.controlSocketPath); | ||
| } catch { | ||
| // Ignore | ||
| } | ||
|
|
||
| // Kill any lingering process | ||
| const proc = this.gssapiProcesses.get(connectionId); | ||
| if (proc && !proc.killed) { | ||
| proc.kill(); | ||
| } | ||
|
|
||
| this.gssapiProcesses.delete(connectionId); | ||
| this.gssapiConnections.delete(connectionId); | ||
| this.emit('disconnected', connectionId); | ||
| } | ||
|
|
||
| /** | ||
| * Disconnects an existing SSH connection. | ||
| * @param connectionId - ID of the connection to close | ||
| */ | ||
| async disconnect(connectionId: string): Promise<void> { | ||
| // Handle GSSAPI connections | ||
| if (this.gssapiConnections.has(connectionId)) { | ||
| return this.disconnectGssapi(connectionId); | ||
| } | ||
|
|
||
| const connection = this.connections[connectionId]; | ||
| if (!connection) { |
There was a problem hiding this comment.
No timeout on GSSAPI connection establishment
connectGssapi() wraps the spawn call in a Promise but sets no Node.js-level timeout. If ssh -f stalls — for example, because the KDC is unreachable and the Kerberos library is waiting for a network timeout — the promise will never settle and the caller will hang indefinitely. The SSH ConnectTimeout=15 option only limits the TCP handshake, not the full Kerberos exchange.
Consider adding an explicit timeout (e.g., 30–60 s) that rejects the promise and kills the spawned process:
const timer = setTimeout(() => {
proc.kill();
reject(new Error('GSSAPI connection timed out'));
}, 30000);
proc.on('close', (code) => {
clearTimeout(timer);
// ... existing logic
});2 participants
Summary
This PR adds support for GSSAPI/Kerberos SSH authentication to the application. Since the
ssh2library doesn't natively support GSSAPI, the implementation uses the system's OpenSSH client with ControlMaster multiplexing to establish and reuse authenticated connections.Key Changes
Core SSH Service (
SshService.ts)GssapiConnectiontype to track GSSAPI connections separately from ssh2 connectionsconnectGssapi()method that spawns systemsshwith GSSAPI flags and ControlMaster modeexecuteCommandGssapi()to run commands via the ControlMaster socket usingexecFiledisconnectGssapi()for proper cleanup of ControlMaster processes and socket filesisGssapiConnection(),getGssapiConnection(),getGssapiSshArgs()isConnected(),listConnections(),getConnectionInfo(),disconnectAll()) to handle both ssh2 and GSSAPI connectionsIPC Layer (
sshIpc.ts)sshwith verbose output for debugginglistFiles()usesls -lawith timestamp parsingreadFile()usescatwriteFile()uses base64 encoding for safe transmission'gssapi'Remote PTY Service (
RemotePtyService.ts)startGssapiPty()that spawnsssh -ttwith ControlMaster socket for interactive terminal sessionsnode-ptyfor proper terminal emulationUI Components
AddRemoteProjectModal.tsxto include GSSAPI/Kerberos as an authentication option with Globe iconkinitbefore connecting)SshConnectionForm.tsxto support GSSAPI auth type selection'gssapi'auth typeType Definitions
GssapiConnectioninterface insrc/main/services/ssh/types.tsSshConfigand related types to include'gssapi'as valid auth typeTesting
Added comprehensive unit tests in
SshService.test.ts:Tests mock
spawnandexecFileto simulate system ssh behavior without requiring actual Kerberos setup.Type of change
Notes
kinitmust be run beforehand)https://claude.ai/code/session_017956U4sRFiAYbRmqujqTEV