Continue work on TLS remote support

* Moved cert bundle read to separate package * Removed duplication for non-tcp tls flag check * Added tls info to `system connection list` * Removed TCP warning if TLS is enabled * Fixed not using TLS when using ABI instead of remote * Added central check for cert without key or vice-versa Signed-off-by: Andrew Melnick <[email protected]>
containers · Nov 30, 2024 · 0748cfe · 0748cfe
1 parent 0259e04
commit 0748cfe
Show file tree

Hide file tree

Showing 8 changed files with 32 additions and 32 deletions.
diff --git a/cmd/podman/root.go b/cmd/podman/root.go
@@ -518,15 +518,15 @@ func rootFlags(cmd *cobra.Command, podmanConfig *entities.PodmanConfig) {
 	_ = cmd.RegisterFlagCompletionFunc(identityFlagName, completion.AutocompleteDefault)
 
 	tlsCertFileFlagName := "tls-cert"
-	lFlags.StringVar(&podmanConfig.TLSCertFile, tlsCertFileFlagName, "", "path to TLS client certificate PEM file for remote, (CONTAINER_TLS_CERT)")
+	lFlags.StringVar(&podmanConfig.TLSCertFile, tlsCertFileFlagName, podmanConfig.TLSCertFile, "path to TLS client certificate PEM file for remote, (CONTAINER_TLS_CERT)")
 	_ = cmd.RegisterFlagCompletionFunc(tlsCertFileFlagName, completion.AutocompleteDefault)
 
 	tlsKeyFileFlagName := "tls-key"
-	lFlags.StringVar(&podmanConfig.TLSKeyFile, tlsKeyFileFlagName, "", "path to TLS client certificate private key PEM file for remote, (CONTAINER_TLS_KEY)")
+	lFlags.StringVar(&podmanConfig.TLSKeyFile, tlsKeyFileFlagName, podmanConfig.TLSKeyFile, "path to TLS client certificate private key PEM file for remote, (CONTAINER_TLS_KEY)")
 	_ = cmd.RegisterFlagCompletionFunc(tlsKeyFileFlagName, completion.AutocompleteDefault)
 
 	tlsCAFileFlagName := "tls-ca"
-	lFlags.StringVar(&podmanConfig.TLSCAFile, tlsCAFileFlagName, "", "path to TLS certificate Authority PEM file for remote, (CONTAINER_TLS_CA)")
+	lFlags.StringVar(&podmanConfig.TLSCAFile, tlsCAFileFlagName, podmanConfig.TLSCAFile, "path to TLS certificate Authority PEM file for remote, (CONTAINER_TLS_CA)")
 	_ = cmd.RegisterFlagCompletionFunc(tlsCAFileFlagName, completion.AutocompleteDefault)
 
 	// Flags that control or influence any kind of output.

diff --git a/cmd/podman/system/connection/add.go b/cmd/podman/system/connection/add.go
@@ -155,32 +155,24 @@ func add(cmd *cobra.Command, args []string) error {
 		return fmt.Errorf("invalid ssh mode")
 	}
 
-	switch uri.Scheme {
-	case "ssh":
+	if uri.Scheme != "tcp" {
 		if cmd.Flags().Changed("tls-cert") {
-			return errors.New("--tls-cert option not supported for ssh scheme")
+			return fmt.Errorf("--tls-cert option not supported for %s scheme", uri.Scheme)
 		}
 		if cmd.Flags().Changed("tls-key") {
-			return errors.New("--tls-key option not supported for ssh scheme")
+			return fmt.Errorf("--tls-key option not supported for %s scheme", uri.Scheme)
 		}
 		if cmd.Flags().Changed("tls-ca") {
-			return errors.New("--tls-ca option not supported for ssh scheme")
+			return fmt.Errorf("--tls-ca option not supported for %s scheme", uri.Scheme)
 		}
+	}
+	switch uri.Scheme {
+	case "ssh":
 		return ssh.Create(entities, sshMode)
 	case "unix":
 		if cmd.Flags().Changed("identity") {
 			return errors.New("--identity option not supported for unix scheme")
 		}
-		if cmd.Flags().Changed("tls-cert") {
-			return errors.New("--tls-cert option not supported for unix scheme")
-		}
-		if cmd.Flags().Changed("tls-key") {
-			return errors.New("--tls-key option not supported for unix scheme")
-		}
-		if cmd.Flags().Changed("tls-ca") {
-			return errors.New("--tls-ca option not supported for unix scheme")
-		}
-
 		if cmd.Flags().Changed("socket-path") {
 			uri.Path = cmd.Flag("socket-path").Value.String()
 		}

diff --git a/cmd/podman/system/connection/list.go b/cmd/podman/system/connection/list.go
@@ -118,19 +118,22 @@ func inspect(cmd *cobra.Command, args []string) error {
 		rpt, err = rpt.Parse(report.OriginUser, format)
 	} else {
 		rpt, err = rpt.Parse(report.OriginPodman,
-			"{{range .}}{{.Name}}\t{{.URI}}\t{{.Identity}}\t{{.Default}}\t{{.ReadWrite}}\n{{end -}}")
+			"{{range .}}{{.Name}}\t{{.URI}}\t{{.Identity}}\t{{.TLSCAFile}}\t{{.TLSCertFile}}\t{{.TLSKeyFile}}\t{{.Default}}\t{{.ReadWrite}}\n{{end -}}")
 	}
 	if err != nil {
 		return err
 	}
 
 	if rpt.RenderHeaders {
 		err = rpt.Execute([]map[string]string{{
-			"Default":   "Default",
-			"Identity":  "Identity",
-			"Name":      "Name",
-			"URI":       "URI",
-			"ReadWrite": "ReadWrite",
+			"Default":     "Default",
+			"Identity":    "Identity",
+			"TLSCAFile":   "TLSCAFile",
+			"TLSCertFile": "TLSCertFile",
+			"TLSKeyFile":  "TLSKeyFile",
+			"Name":        "Name",
+			"URI":         "URI",
+			"ReadWrite":   "ReadWrite",
 		}})
 		if err != nil {
 			return err

diff --git a/cmd/podman/system/service_abi.go b/cmd/podman/system/service_abi.go
@@ -76,11 +76,13 @@ func restService(flags *pflag.FlagSet, cfg *entities.PodmanConfig, opts entities
 				}
 			}
 		case "tcp":
-			// We want to check if the user is requesting a TCP address.
+			// We want to check if the user is requesting a TCP address if TLS is not active.
 			// If so, warn that this is insecure.
 			// Ignore errors here, the actual backend code will handle them
 			// better than we can here.
-			logrus.Warnf("Using the Podman API service with TCP sockets is not recommended, please see `podman system service` manpage for details")
+			if opts.TLSKeyFile == "" || opts.TLSCertFile == "" {
+				logrus.Warnf("Using the Podman API service with TCP sockets without TLS is not recommended, please see `podman system service` manpage for details")
+			}
 
 			host := uri.Host
 			if host == "" {

diff --git a/internal/domain/infra/runtime_abi.go b/internal/domain/infra/runtime_abi.go
@@ -19,7 +19,7 @@ func NewTestingEngine(facts *entities.PodmanConfig) (ientities.TestingEngine, er
 		r, err := NewLibpodTestingRuntime(facts.FlagSet, facts)
 		return r, err
 	case entities.TunnelMode:
-		ctx, err := bindings.NewConnectionWithIdentity(context.Background(), facts.URI, facts.Identity, facts.MachineMode)
+		ctx, err := bindings.NewConnectionWithIdentityOrTLS(context.Background(), facts.URI, facts.Identity, facts.TLSCertFile, facts.TLSKeyFile, facts.TLSCAFile, facts.MachineMode)
 		return &tunnel.TestingEngine{ClientCtx: ctx}, err
 	}
 	return nil, fmt.Errorf("runtime mode '%v' is not supported", facts.EngineMode)

diff --git a/pkg/api/server/server.go b/pkg/api/server/server.go
@@ -23,7 +23,7 @@ import (
 	"github.com/containers/podman/v5/pkg/api/server/idle"
 	"github.com/containers/podman/v5/pkg/api/types"
 	"github.com/containers/podman/v5/pkg/domain/entities"
-	"github.com/containers/podman/v5/pkg/util"
+	"github.com/containers/podman/v5/pkg/util/tlsutil"
 	"github.com/coreos/go-systemd/v22/daemon"
 	"github.com/gorilla/mux"
 	"github.com/gorilla/schema"
@@ -108,7 +108,7 @@ func newServer(runtime *libpod.Runtime, listener net.Listener, opts entities.Ser
 
 	if opts.TLSClientCAFile != "" {
 		logrus.Debugf("will validate client certs against %s", opts.TLSClientCAFile)
-		pool, err := util.ReadCertBundle(opts.TLSClientCAFile)
+		pool, err := tlsutil.ReadCertBundle(opts.TLSClientCAFile)
 		if err != nil {
 			return nil, err
 		}

diff --git a/pkg/bindings/connection.go b/pkg/bindings/connection.go
@@ -19,7 +19,7 @@ import (
 
 	"github.com/blang/semver/v4"
 	"github.com/containers/common/pkg/ssh"
-	"github.com/containers/podman/v5/pkg/util"
+	"github.com/containers/podman/v5/pkg/util/tlsutil"
 	"github.com/containers/podman/v5/version"
 	"github.com/kevinburke/ssh_config"
 	"github.com/sirupsen/logrus"
@@ -326,12 +326,15 @@ func tcpClient(_url *url.URL, tlsCertFile, tlsKeyFile, tlsCAFile string) (Connec
 		connection.tls = true
 	}
 	if len(tlsCAFile) != 0 {
-		pool, err := util.ReadCertBundle(tlsCAFile)
+		pool, err := tlsutil.ReadCertBundle(tlsCAFile)
 		if err != nil {
 			return connection, fmt.Errorf("unable to read CA bundle: %w", err)
 		}
 		transport.TLSClientConfig.RootCAs = pool
 	}
+	if (len(tlsCertFile) == 0) != (len(tlsKeyFile) == 0) {
+		return connection, fmt.Errorf("TLS Key and Certificate must both or neither be provided")
+	}
 	if len(tlsCertFile) != 0 && len(tlsKeyFile) != 0 {
 		keyPair, err := tls.LoadX509KeyPair(tlsCertFile, tlsKeyFile)
 		if err != nil {

diff --git a/pkg/util/tls.go → pkg/util/tlsutil/tls.go b/pkg/util/tls.go → pkg/util/tlsutil/tls.go
@@ -1,4 +1,4 @@
-package util
+package tlsutil
 
 import (
 	"crypto/x509"