Skip to content

Add port configuration for SSH #139

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

Open
martialblog wants to merge 1 commit into
base: main
Choose a base branch
from
+6 −4
Open

Add port configuration for SSH #139

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
3 changes: 2 additions & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ Not supported at the moment is Kerberos.
```
Arguments:
-H, --host string Host name, IP Address of the remote host (default "127.0.0.1")
-p, --port int Port number WinRM
-p, --port int Port number WinRM (default: 5985 & 5986 for TLS)
-U, --user string Username of the remote host
-P, --password string Password of the user
-k, --insecure Don't verify the hostname on the returned certificate
Expand All @@ -37,6 +37,7 @@ Arguments:
--icingacmd string Executes commands of Icinga PowerShell Framework (e.g. Invoke-IcingaCheckCPU)
--auth string Authentication mechanism - NTLM | SSH (default "basic")
--sshhost string SSH Host (mandatory if --auth=SSH)
--sshport int SSH Port (default 22)
--sshuser string SSH Username (mandatory if --auth=SSH)
--sshpassword string SSH Password (mandatory if --auth=SSH)
-t, --timeout int Abort the check after n seconds (default 10)
Expand Down
7 changes: 4 additions & 3 deletions check.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ type Config struct {
SSHHost string
SSHUser string
SSHPassword string
SSHPort int
validated bool
}

Expand All @@ -52,7 +53,7 @@ func BuildConfigFlags(fs *pflag.FlagSet) (config *Config) {

fs.StringVarP(&config.Host, "host", "H", "127.0.0.1",
"Host name, IP Address of the remote host")
fs.IntVarP(&config.Port, "port", "p", 0, "Port number WinRM") // TODO: document default
fs.IntVarP(&config.Port, "port", "p", 0, "Port number WinRM (default: 5985 & 5986 for TLS)")

fs.StringVarP(&config.User, "user", "U", "", "Username of the remote host")
fs.StringVarP(&config.Password, "password", "P", "", "Password of the user")
Expand All @@ -72,6 +73,7 @@ func BuildConfigFlags(fs *pflag.FlagSet) (config *Config) {

// AuthSSH
fs.StringVar(&config.SSHHost, "sshhost", "", "SSH Host (mandatory if --auth=SSH)")
fs.IntVarP(&config.SSHPort, "sshport", "", 22, "SSH Port")
fs.StringVar(&config.SSHUser, "sshuser", "", "SSH Username (mandatory if --auth=SSH)")
fs.StringVar(&config.SSHPassword, "sshpassword", "", "SSH Password (mandatory if --auth=SSH)")

Expand Down Expand Up @@ -212,9 +214,8 @@ func (c *Config) Run(timeout time.Duration) (rc int, output string, err error) {
return &winrm.ClientAuthRequest{}
}
case AuthSSH:
// TODO: port configuration?
var sshClient *ssh.Client
sshClient, err = ssh.Dial("tcp", c.SSHHost+":22", &ssh.ClientConfig{
sshClient, err = ssh.Dial("tcp", fmt.Sprintf("%s:%d", c.SSHHost, c.SSHPort), &ssh.ClientConfig{
Copy link
Copy Markdown
Member

@RincewindsHat RincewindsHat May 4, 2026

Choose a reason for hiding this comment

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

should we verfify that this value is a valid port number (0..2**16)?

Copy link
Copy Markdown
Member Author

@martialblog martialblog May 4, 2026

Choose a reason for hiding this comment

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

We don't do that for the WinRM port and in any other repo, any reason we should start now?

Copy link
Copy Markdown
Member

@RincewindsHat RincewindsHat May 4, 2026

Choose a reason for hiding this comment

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

I tend to verify all input as harsh as possible as early as possible to provide proper feedback and not throw weird error message later in the process.

And yes, consequence would me that this should be added for the other ports too.

Copy link
Copy Markdown
Member Author

@martialblog martialblog May 4, 2026

Choose a reason for hiding this comment

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

The weird error would just be a "connect: connection refused" error, just like it is now

Copy link
Copy Markdown
Member

@RincewindsHat RincewindsHat May 4, 2026

Choose a reason for hiding this comment

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

./check_by_powershell -H localhost -p 324234234234234 --cmd 'echo foo' --user foo --password bla
[UNKNOWN] - execution failed: execution of remote cmd failed: unknown error Post "https://localhost:324234234234234/wsman": dial tcp: address 324234234234234: invalid port

the invalid port is probably the main message here and that could be the only message :-)

User: c.SSHUser,
Auth: []ssh.AuthMethod{ssh.Password(c.SSHPassword)},
HostKeyCallback: ssh.InsecureIgnoreHostKey(), //nolint:gosec // TODO: really?
Expand Down
Loading