Skip to content

Commit

Permalink
fix: update argument handling based on ansible execution tracing
Browse files Browse the repository at this point in the history 
Add a bunch of test cases created by sniffing the execution of ssh
commands on an openssh client and server.
  • Loading branch information
@smlx
smlx committed Oct 22, 2024
1 parent 1c1557f commit 9204b74
Show file tree
Hide file tree
Showing 4 changed files with 69 additions and 11 deletions.
1 change: 0 additions & 1 deletion go.mod
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ module github.com/uselagoon/ssh-portal
go 1.22.2

require (
al.essio.dev/pkg/shellescape v1.5.1
github.com/DATA-DOG/go-sqlmock v1.5.2
github.com/MicahParks/keyfunc/v2 v2.1.0
github.com/alecthomas/assert/v2 v2.11.0
Expand Down
4 changes: 0 additions & 4 deletions go.sum
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
al.essio.dev/pkg/shellescape v1.5.1 h1:86HrALUujYS/h+GtqoB26SBEdkWfmMI6FubjXlsXyho=
al.essio.dev/pkg/shellescape v1.5.1/go.mod h1:6sIqp7X2P6mThCQ7twERpZTuigpr6KbZWtls1U8I890=
filippo.io/edwards25519 v1.1.0 h1:FNf4tywRC1HmFuKW5xopWpigGjJKiJSV0Cqo0cJWDaA=
filippo.io/edwards25519 v1.1.0/go.mod h1:BxyFTGdWcka3PhytdK4V28tE5sGfRvvvRV7EaN4VDT4=
github.com/DATA-DOG/go-sqlmock v1.5.2 h1:OcvFkGmslmlZibjAjaHm3L//6LiuBgolP7OputlJIzU=
Expand Down Expand Up @@ -69,8 +67,6 @@ github.com/google/gofuzz v1.2.0 h1:xRy4A+RhZaiKjJ1bPfwQ8sedCA+YS2YcCHW6ec7JMi0=
github.com/google/gofuzz v1.2.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg=
github.com/google/pprof v0.0.0-20240525223248-4bfdf5a9a2af h1:kmjWCqn2qkEml422C2Rrd27c3VGxi6a/6HNq8QmHRKM=
github.com/google/pprof v0.0.0-20240525223248-4bfdf5a9a2af/go.mod h1:K1liHPHnj73Fdn/EKuT8nrFqBihUSKXoLYU0BuatOYo=
github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 h1:El6M4kTTCOh6aBiKaUGG7oYTSPP8MxqL4YI3kZKwcP4=
github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510/go.mod h1:pupxD2MaaD3pAXIBCelhxNneeOaAeabZDe5s4K6zSpQ=
github.com/google/uuid v1.6.1-0.20240806143717-0e97ed3b5379 h1:9pvPp/2VCtCB2xdSUCaKe1VKCzVHMR+GGgIAVLfQxIs=
github.com/google/uuid v1.6.1-0.20240806143717-0e97ed3b5379/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
github.com/gorilla/securecookie v1.1.2 h1:YCIWL56dvtr73r6715mJs5ZvhtnY73hBvEF8kXD8ePA=
Expand Down
4 changes: 2 additions & 2 deletions internal/sshserver/sessionhandler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@ import (
"fmt"
"io"
"log/slog"
"strings"
"time"

"al.essio.dev/pkg/shellescape"
"github.com/gliderlabs/ssh"
"github.com/prometheus/client_golang/prometheus"
"github.com/prometheus/client_golang/prometheus/promauto"
Expand Down Expand Up @@ -86,7 +86,7 @@ func getSSHIntent(sftp bool, cmd []string) []string {
// if there is a command, wrap it in a shell the way openssh does
// https://github.com/openssh/openssh-portable/blob/
// 73dcca12115aa12ed0d123b914d473c384e52651/session.c#L1705-L1713
return []string{"sh", "-c", shellescape.QuoteCommand(cmd)}
return []string{"sh", "-c", strings.Join(cmd, " ")}
}

// sessionHandler returns a ssh.Handler which connects the ssh session to the
Expand Down
71 changes: 67 additions & 4 deletions internal/sshserver/sessionhandler_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ func TestExec(t *testing.T) {
user: "project-test",
deployment: "cli",
rawCommand: []string{"/bin/sh", "-c", "( echo foo; echo bar; echo baz ) | tail -n2"},
command: []string{"sh", "-c", "/bin/sh -c '( echo foo; echo bar; echo baz ) | tail -n2'"},
command: []string{"sh", "-c", "/bin/sh -c ( echo foo; echo bar; echo baz ) | tail -n2"},
sftp: false,
logAccessEnabled: false,
pty: false,
Expand All @@ -52,7 +52,7 @@ func TestExec(t *testing.T) {
user: "project-test",
deployment: "cli",
rawCommand: []string{"/bin/sh", "-c", "sleep 3 & echo $(pgrep sleep)"},
command: []string{"sh", "-c", "/bin/sh -c 'sleep 3 & echo $(pgrep sleep)'"},
command: []string{"sh", "-c", "/bin/sh -c sleep 3 & echo $(pgrep sleep)"},
sftp: false,
logAccessEnabled: false,
pty: false,
Expand All @@ -61,7 +61,7 @@ func TestExec(t *testing.T) {
user: "project-test",
deployment: "cli",
rawCommand: []string{"/bin/sh", "-c", "sleep 3 & echo $( pgrep sleep )"},
command: []string{"sh", "-c", "/bin/sh -c 'sleep 3 & echo $( pgrep sleep )'"},
command: []string{"sh", "-c", "/bin/sh -c sleep 3 & echo $( pgrep sleep )"},
sftp: false,
logAccessEnabled: false,
pty: false,
Expand All @@ -70,7 +70,70 @@ func TestExec(t *testing.T) {
user: "project-test",
deployment: "cli",
rawCommand: []string{"/bin/sh", "-c", "echo $$ $USER"},
command: []string{"sh", "-c", "/bin/sh -c 'echo $$ $USER'"},
command: []string{"sh", "-c", "/bin/sh -c echo $$ $USER"},
sftp: false,
logAccessEnabled: false,
pty: false,
},
"shell variables 2": {
user: "project-test",
deployment: "cli",
rawCommand: []string{"echo", "$(( $$ + 1 ))"},
command: []string{"sh", "-c", "echo $(( $$ + 1 ))"},
sftp: false,
logAccessEnabled: false,
pty: false,
},
"ansible 1": {
user: "project-test",
deployment: "cli",
rawCommand: []string{"/bin/sh -c '( umask 77 && mkdir -p \"` echo /tmp `\"&& mkdir \"` echo /tmp/ansible-tmp-1729564333.3484864-620266-10397749948780 `\" && echo ansible-tmp-1729564333.3484864-620266-10397749948780=\"` echo /tmp/ansible-tmp-1729564333.3484864-620266-10397749948780 `\" ) && sleep 0'"},
command: []string{"sh", "-c", "/bin/sh -c '( umask 77 && mkdir -p \"` echo /tmp `\"&& mkdir \"` echo /tmp/ansible-tmp-1729564333.3484864-620266-10397749948780 `\" && echo ansible-tmp-1729564333.3484864-620266-10397749948780=\"` echo /tmp/ansible-tmp-1729564333.3484864-620266-10397749948780 `\" ) && sleep 0'"},
sftp: false,
logAccessEnabled: false,
pty: false,
},
"ansible 2": {
user: "project-test",
deployment: "cli",
rawCommand: []string{"/bin/sh -c 'echo PLATFORM; uname; echo FOUND; command -v '\"'\"'python3.12'\"'\"'; command -v '\"'\"'python3.11'\"'\"'; command -v '\"'\"'python3.10'\"'\"'; command -v '\"'\"'python3.9'\"'\"'; command -v '\"'\"'python3.8'\"'\"'; command -v '\"'\"'python3.7'\"'\"'; command -v '\"'\"'/usr/bin/python3'\"'\"'; command -v '\"'\"'python3'\"'\"'; echo ENDFOUND && sleep 0'"},
command: []string{"sh", "-c", "/bin/sh -c 'echo PLATFORM; uname; echo FOUND; command -v '\"'\"'python3.12'\"'\"'; command -v '\"'\"'python3.11'\"'\"'; command -v '\"'\"'python3.10'\"'\"'; command -v '\"'\"'python3.9'\"'\"'; command -v '\"'\"'python3.8'\"'\"'; command -v '\"'\"'python3.7'\"'\"'; command -v '\"'\"'/usr/bin/python3'\"'\"'; command -v '\"'\"'python3'\"'\"'; echo ENDFOUND && sleep 0'"},
sftp: false,
logAccessEnabled: false,
pty: false,
},
"ansible 3": {
user: "project-test",
deployment: "cli",
rawCommand: []string{"/bin/sh -c '/usr/bin/python3.9 && sleep 0'"},
command: []string{"sh", "-c", "/bin/sh -c '/usr/bin/python3.9 && sleep 0'"},
sftp: false,
logAccessEnabled: false,
pty: false,
},
"ansible 4": {
user: "project-test",
deployment: "cli",
rawCommand: []string{"/bin/sh -c 'chmod u+x /tmp/ansible-tmp-1729564333.3484864-620266-10397749948780/ /tmp/ansible-tmp-1729564333.3484864-620266-10397749948780/AnsiballZ_command.py && sleep 0'"},
command: []string{"sh", "-c", "/bin/sh -c 'chmod u+x /tmp/ansible-tmp-1729564333.3484864-620266-10397749948780/ /tmp/ansible-tmp-1729564333.3484864-620266-10397749948780/AnsiballZ_command.py && sleep 0'"},
sftp: false,
logAccessEnabled: false,
pty: false,
},
"ansible 5": {
user: "project-test",
deployment: "cli",
rawCommand: []string{"/bin/sh -c '/usr/bin/python3.9 /tmp/ansible-tmp-1729564333.3484864-620266-10397749948780/AnsiballZ_command.py && sleep 0'"},
command: []string{"sh", "-c", "/bin/sh -c '/usr/bin/python3.9 /tmp/ansible-tmp-1729564333.3484864-620266-10397749948780/AnsiballZ_command.py && sleep 0'"},
sftp: false,
logAccessEnabled: false,
pty: false,
},
"ansible 6": {
user: "project-test",
deployment: "cli",
rawCommand: []string{"/bin/sh -c 'rm -f -r /tmp/ansible-tmp-1729564333.3484864-620266-10397749948780/ > /dev/null 2>&1 && sleep 0'"},
command: []string{"sh", "-c", "/bin/sh -c 'rm -f -r /tmp/ansible-tmp-1729564333.3484864-620266-10397749948780/ > /dev/null 2>&1 && sleep 0'"},
sftp: false,
logAccessEnabled: false,
pty: false,
Expand Down

0 comments on commit 9204b74

Please sign in to comment.