Skip to content

Commit

Permalink
refactor(client): review changes
Browse files Browse the repository at this point in the history 
See <francoismichel#45>.
  • Loading branch information
@TheoTechnicguy
TheoTechnicguy committed Dec 18, 2023
1 parent 1c216c7 commit 274c10b
Showing 1 changed file with 25 additions and 38 deletions.
63 changes: 25 additions & 38 deletions cli/client/main.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ func homedir() string {
}
}


func forwardAgent(parent context.Context, channel ssh3.Channel) error {
sockPath := os.Getenv("SSH_AUTH_SOCK")
if sockPath == "" {
Expand Down Expand Up @@ -393,7 +392,6 @@ func mainWithStatusCode() int {
keyLog = f
}


parsedUrl, err := url.Parse(urlFromParam)
if err != nil {
log.Fatal().Msgf("%s", err)
Expand All @@ -413,32 +411,25 @@ func mainWithStatusCode() int {
}

hostnameIsAnIP := net.ParseIP(hostname) != nil
// *** Port selection ***
// There are six scenarios:
//
// | CLI port | parse | config port | program behaviour |
// | -------- | ----- | ----------- | -------------------- |
// | yes | nok | yes | throw error and exit |
// | yes | nok | no | throw error and exit |
// | yes | ok | yes | use CLI port |
// | yes | ok | no | use CLI port |
// | no | --- | yes | use config port |
// | no | --- | no | use default port |

var port int
if urlPort != "" {
if parsedPort, err := strconv.Atoi(urlPort); err != nil || parsedPort > 0xffff {
if parsedPort, err := strconv.Atoi(urlPort); err == nil && parsedPort < 0xffff {
// There is a port in the CLI and the port is valid. Use the CLI port.
port = parsedPort
} else {
// There is a port in the CLI but it is not valid.
// use WithLevel(zerolog.FatalLevel) to log a fatal level, but let us handle
// program termination. log.Fatal() exits with os.Exit(1).
log.WithLevel(zerolog.FatalLevel).Str("Port", urlPort).Err(err).Msg("cli contains an invalid port")
fmt.Fprintf(os.Stderr, "Bad port '%s'\n", urlPort)
return -1
} else {
port = parsedPort
}
} else if configPort != -1 {
// There is no port in the CLI, but one in a config file. Use the config port.
port = configPort
} else {
// There is no port specified, neither in the CLI, nor in the configuration.
port = 443
}

Expand Down Expand Up @@ -467,13 +458,11 @@ func mainWithStatusCode() int {
parsedUrl.RawQuery = urlQuery.Encode()
requestUrl := parsedUrl.String()


pool, err := x509.SystemCertPool()
if err != nil {
log.Fatal().Msgf("%s", err)
}


tlsConf := &tls.Config{
RootCAs: pool,
InsecureSkipVerify: *insecure,
Expand Down Expand Up @@ -517,7 +506,6 @@ func mainWithStatusCode() int {

defer roundTripper.Close()


// connect to SSH agent if it exists
var agentClient agent.ExtendedAgent
var agentKeys []ssh.PublicKey
Expand All @@ -540,7 +528,6 @@ func mainWithStatusCode() int {
}
}


log.Debug().Msgf("dialing QUIC host at %s", fmt.Sprintf("%s:%d", hostname, port))

if hostnameIsAnIP {
Expand All @@ -556,16 +543,16 @@ func mainWithStatusCode() int {
tlsConf,
&qconf)
if err != nil {
if transportErr, ok := err.(*quic.TransportError); ok {
if transportErr, ok := err.(*quic.TransportError); ok {
if transportErr.ErrorCode.IsCryptoError() {
if tty == nil {
log.Error().Msgf("insecure server cert in non-terminal session, aborting")
return -1
}
if _, ok = knownHosts[hostname]; ok {
log.Error().Msgf("The server certificate cannot be verified using the one installed in %s. " +
"If you did not change the server certificate, it could be a machine-in-the-middle attack. "+
"TLS error: %s", knownHostsPath, err)
log.Error().Msgf("The server certificate cannot be verified using the one installed in %s. "+
"If you did not change the server certificate, it could be a machine-in-the-middle attack. "+
"TLS error: %s", knownHostsPath, err)
log.Error().Msgf("Aborting.")
return -1
}
Expand All @@ -579,9 +566,9 @@ func mainWithStatusCode() int {
}

_, err := quic.DialAddrEarly(ctx,
fmt.Sprintf("%s:%d", hostname, port),
tlsConf,
&qconf)
fmt.Sprintf("%s:%d", hostname, port),
tlsConf,
&qconf)
if !errors.Is(err, certError) {
log.Error().Msgf("could not create client QUIC connection: %s", err)
return -1
Expand All @@ -594,22 +581,22 @@ func mainWithStatusCode() int {
// first, carriage return
_, _ = tty.WriteString("\r")
_, err = tty.WriteString("Received an unknown self-signed certificate from the server.\n\r" +
"We recommend not using self-signed certificates.\n\r" +
"This session is vulnerable a machine-in-the-middle attack.\n\r" +
"Certificate fingerprint: " +
"SHA256 " + util.Sha256Fingerprint(peerCertificate.Raw) + "\n\r" +
"Do you want to add this certificate to ~/.ssh3/known_hosts (yes/no)? ")
"We recommend not using self-signed certificates.\n\r" +
"This session is vulnerable a machine-in-the-middle attack.\n\r" +
"Certificate fingerprint: " +
"SHA256 " + util.Sha256Fingerprint(peerCertificate.Raw) + "\n\r" +
"Do you want to add this certificate to ~/.ssh3/known_hosts (yes/no)? ")
if err != nil {
log.Error().Msgf("cound not write on /dev/tty: %s", err)
return -1
}

answer := ""
reader := bufio.NewReader(tty)
for {
answer, _ = reader.ReadString('\n')
answer = strings.TrimSpace(answer)
_, _ = tty.WriteString("\r") // always ensure a carriage return
_, _ = tty.WriteString("\r") // always ensure a carriage return
if answer == "yes" || answer == "no" {
break
}
Expand Down Expand Up @@ -665,7 +652,7 @@ func mainWithStatusCode() int {
if *privKeyFile != "" {
authMethods = append(authMethods, ssh3.NewPrivkeyFileAuthMethod(*privKeyFile))
}

if *pubkeyForAgent != "" {
if agentClient == nil {
log.Warn().Msgf("specified a public key (%s) but no agent is running", *pubkeyForAgent)
Expand All @@ -683,7 +670,7 @@ func mainWithStatusCode() int {
return -1
}
}

for _, candidateKey := range agentKeys {
if pubkey == nil || bytes.Equal(candidateKey.Marshal(), pubkey.Marshal()) {
log.Debug().Msgf("found key in agent: %s", candidateKey)
Expand All @@ -692,11 +679,11 @@ func mainWithStatusCode() int {
}
}
}

if *passwordAuthentication {
authMethods = append(authMethods, ssh3.NewPasswordAuthMethod())
}

} else {
// for now, only perform OIDC if it was explicitly asked by the user
if *issuerUrl != "" {
Expand Down

0 comments on commit 274c10b

Please sign in to comment.