feat(core): Add known hosts callback check for ssh connections (#217)

* feat(core): Add known hosts callback check for ssh connections

* fix code & add tests

go.mod

@@ -9,6 +9,7 @@ require (
 	github.com/hashicorp/terraform-plugin-log v0.7.0
 	github.com/hashicorp/terraform-plugin-sdk/v2 v2.24.1
 	github.com/pkg/sftp v1.13.5
+	github.com/skeema/knownhosts v1.1.0
 	github.com/stretchr/testify v1.8.1
 	golang.org/x/crypto v0.5.0
 )

go.sum

@@ -99,6 +99,8 @@ github.com/pkg/sftp v1.13.5 h1:a3RLUqkyjYRtBTZJZ1VRrKbN3zhuPLlUc3sphVz81go=
 github.com/pkg/sftp v1.13.5/go.mod h1:wHDZ0IZX6JcBYRK1TH9bcVq8G7TLpVHYIGJRFnmPfxg=
 github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
 github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
+github.com/skeema/knownhosts v1.1.0 h1:Wvr9V0MxhjRbl3f9nMnKnFfiWTJmtECJ9Njkea3ysW0=
+github.com/skeema/knownhosts v1.1.0/go.mod h1:sKFq3RD6/TKZkSWn8boUbDC7Qkgcv+8XXijpFO6roag=
 github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
 github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw=
 github.com/stretchr/objx v0.5.0/go.mod h1:Yh+to48EsGEfYuaHDzXPcE3xhTkx73EhmCGUpEOglKo=

proxmox/utils.go

@@ -5,12 +5,26 @@
 package proxmox
 
 import (
+	"context"
 	"fmt"
+	"io"
 	"math"
 	"strconv"
 	"strings"
+
+	"github.com/hashicorp/terraform-plugin-log/tflog"
 )
 
+func CloseOrLogError(ctx context.Context) func(io.Closer) {
+	return func(c io.Closer) {
+		if err := c.Close(); err != nil {
+			tflog.Error(ctx, "Failed to close", map[string]interface{}{
+				"error": err,
+			})
+		}
+	}
+}
+
 func ParseDiskSize(size *string) (int, error) {
 	if size == nil {
 		return 0, nil

proxmox/utils_test.go

@@ -1,7 +1,11 @@
 package proxmox
 
 import (
+	"context"
+	"fmt"
 	"testing"
+
+	"github.com/stretchr/testify/assert"
 )
 
 func TestParseDiskSize(t *testing.T) {
@@ -35,3 +39,33 @@ func TestParseDiskSize(t *testing.T) {
 		})
 	}
 }
+
+func TestCloseOrLogError(t *testing.T) {
+	t.Parallel()
+	f := CloseOrLogError(context.Background())
+
+	c := &testCloser{}
+	b := &badCloser{}
+	func() {
+		defer f(c)
+		defer f(b)
+		assert.Equal(t, false, c.isClosed)
+	}()
+
+	assert.Equal(t, true, c.isClosed)
+}
+
+type testCloser struct {
+	isClosed bool
+}
+
+func (t *testCloser) Close() error {
+	t.isClosed = true
+	return nil
+}
+
+type badCloser struct{}
+
+func (t *badCloser) Close() error {
+	return fmt.Errorf("bad")
+}

proxmox/virtual_environment_client.go

@@ -192,14 +192,7 @@ func (c *VirtualEnvironmentClient) DoRequest(
 		return fErr
 	}
 
-	defer func(Body io.ReadCloser) {
+	defer CloseOrLogError(ctx)(res.Body)
-		err := Body.Close()
-		if err != nil {
-			tflog.Error(ctx, "failed to close the response body", map[string]interface{}{
-				"error": err.Error(),
-			})
-		}
-	}(res.Body)
 
 	err = c.ValidateResponseCode(res)
 	if err != nil {

proxmox/virtual_environment_nodes.go

@@ -8,12 +8,15 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"net"
 	"net/url"
+	"os"
 	"sort"
 	"strings"
 	"time"
 
 	"github.com/hashicorp/terraform-plugin-log/tflog"
+	"github.com/skeema/knownhosts"
 
 	"golang.org/x/crypto/ssh"
 )
@@ -24,33 +27,19 @@ func (c *VirtualEnvironmentClient) ExecuteNodeCommands(
 	nodeName string,
 	commands []string,
 ) error {
+	closeOrLogError := CloseOrLogError(ctx)
+
 	sshClient, err := c.OpenNodeShell(ctx, nodeName)
 	if err != nil {
 		return err
 	}
-
+	defer closeOrLogError(sshClient)
-	defer func(sshClient *ssh.Client) {
-		err := sshClient.Close()
-		if err != nil {
-			tflog.Error(ctx, "Failed to close ssh client", map[string]interface{}{
-				"error": err,
-			})
-		}
-	}(sshClient)
 
 	sshSession, err := sshClient.NewSession()
 	if err != nil {
 		return err
 	}
-
+	defer closeOrLogError(sshSession)
-	defer func(sshSession *ssh.Session) {
-		err := sshSession.Close()
-		if err != nil {
-			tflog.Error(ctx, "Failed to close ssh session", map[string]interface{}{
-				"error": err,
-			})
-		}
-	}(sshSession)
 
 	output, err := sshSession.CombinedOutput(
 		fmt.Sprintf(
@@ -203,15 +192,49 @@ func (c *VirtualEnvironmentClient) OpenNodeShell(
 
 	ur := strings.Split(c.Username, "@")
 
+	sshHost := fmt.Sprintf("%s:22", *nodeAddress)
+	khPath := fmt.Sprintf("%s/.ssh/known_hosts", os.Getenv("HOME"))
+	kh, err := knownhosts.New(khPath)
+	if err != nil {
+		return nil, fmt.Errorf("failed to read %s: %w", khPath, err)
+	}
+
+	// Create a custom permissive hostkey callback which still errors on hosts
+	// with changed keys, but allows unknown hosts and adds them to known_hosts
+	cb := ssh.HostKeyCallback(func(hostname string, remote net.Addr, key ssh.PublicKey) error {
+		err := kh(hostname, remote, key)
+		if knownhosts.IsHostKeyChanged(err) {
+			return fmt.Errorf("REMOTE HOST IDENTIFICATION HAS CHANGED for host %s! This may indicate a MitM attack", hostname)
+		}
+
+		if knownhosts.IsHostUnknown(err) {
+			f, ferr := os.OpenFile(khPath, os.O_APPEND|os.O_WRONLY, 0o600)
+			if ferr == nil {
+				defer CloseOrLogError(ctx)(f)
+				ferr = knownhosts.WriteKnownHost(f, hostname, remote, key)
+			}
+			if ferr == nil {
+				tflog.Info(ctx, fmt.Sprintf("Added host %s to known_hosts", hostname))
+			} else {
+				tflog.Error(ctx, fmt.Sprintf("Failed to add host %s to known_hosts", hostname), map[string]interface{}{
+					"error": err,
+				})
+			}
+			return nil
+		}
+		return err
+	})
+
 	sshConfig := &ssh.ClientConfig{
 		User:              ur[0],
 		Auth:              []ssh.AuthMethod{ssh.Password(c.Password)},
-		HostKeyCallback: ssh.InsecureIgnoreHostKey(),
+		HostKeyCallback:   cb,
+		HostKeyAlgorithms: kh.HostKeyAlgorithms(sshHost),
 	}
 
-	sshClient, err := ssh.Dial("tcp", *nodeAddress+":22", sshConfig)
+	sshClient, err := ssh.Dial("tcp", sshHost, sshConfig)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("failed to dial %s: %w", sshHost, err)
 	}
 
 	return sshClient, nil

proxmoxtf/resource_virtual_environment_file.go

@@ -256,14 +256,7 @@ func resourceVirtualEnvironmentFileCreate(
 			if err != nil {
 				return diag.FromErr(err)
 			}
-			defer func(Body io.ReadCloser) {
+			defer proxmox.CloseOrLogError(ctx)(res.Body)
-				err := Body.Close()
-				if err != nil {
-					tflog.Error(ctx, "Failed to close body", map[string]interface{}{
-						"error": err,
-					})
-				}
-			}(res.Body)
 
 			tempDownloadedFile, err := os.CreateTemp("", "download")
 			if err != nil {
@@ -659,14 +652,7 @@ func readURL(
 		return
 	}
 
-	defer func(Body io.ReadCloser) {
+	defer proxmox.CloseOrLogError(ctx)(res.Body)
-		err := Body.Close()
-		if err != nil {
-			tflog.Error(ctx, "failed to close the response body", map[string]interface{}{
-				"error": err.Error(),
-			})
-		}
-	}(res.Body)
 
 	fileSize = res.ContentLength
 	httpLastModified := res.Header.Get("Last-Modified")