From bf60bb073161ad705d7cf3fa106a51289d876e6a Mon Sep 17 00:00:00 2001 From: Daniel J Walsh Date: Tue, 27 Jun 2023 09:28:16 -0400 Subject: [PATCH] Display secret to user in inpspect It is pretty complicated to display the secret on the host, but is not really secured. This patch makes it easier to examine the secret. Partial fix for https://github.com/containers/podman/issues/18667 Signed-off-by: Daniel J Walsh --- cmd/podman/secrets/inspect.go | 6 +++++- docs/source/markdown/podman-secret-inspect.1.md | 5 +++++ docs/source/markdown/podman-secret-ls.1.md.in | 1 + pkg/api/handlers/compat/secrets.go | 17 ++++++++++++++++- pkg/api/server/register_secrets.go | 5 +++++ pkg/bindings/secrets/secrets.go | 10 ++++++++-- pkg/bindings/secrets/types.go | 1 + pkg/bindings/secrets/types_inspect_options.go | 15 +++++++++++++++ pkg/domain/entities/engine_container.go | 2 +- pkg/domain/entities/secrets.go | 13 +++++++++---- pkg/domain/infra/abi/secrets.go | 13 +++++++++++-- pkg/domain/infra/tunnel/secrets.go | 7 +++++-- test/e2e/secret_test.go | 14 +++++++++++++- 13 files changed, 95 insertions(+), 14 deletions(-) diff --git a/cmd/podman/secrets/inspect.go b/cmd/podman/secrets/inspect.go index 9054fc3b0e..a19be85ddd 100644 --- a/cmd/podman/secrets/inspect.go +++ b/cmd/podman/secrets/inspect.go @@ -43,6 +43,8 @@ Created at: {{.CreatedAt}} Updated at: {{.UpdatedAt}}` ) +var inspectOpts = entities.SecretInspectOptions{} + func init() { registry.Commands = append(registry.Commands, registry.CliCommand{ Command: inspectCmd, @@ -53,12 +55,14 @@ func init() { flags.StringVarP(&format, formatFlagName, "f", "", "Format inspect output using Go template") _ = inspectCmd.RegisterFlagCompletionFunc(formatFlagName, common.AutocompleteFormat(&entities.SecretInfoReport{})) + flags.BoolVar(&inspectOpts.ShowSecret, "showsecret", false, "Display the secret") + prettyFlagName := "pretty" flags.BoolVar(&pretty, prettyFlagName, false, "Print inspect output in human-readable format") } func inspect(cmd *cobra.Command, args []string) error { - inspected, errs, _ := registry.ContainerEngine().SecretInspect(context.Background(), args) + inspected, errs, _ := registry.ContainerEngine().SecretInspect(context.Background(), args, inspectOpts) // always print valid list if len(inspected) == 0 { diff --git a/docs/source/markdown/podman-secret-inspect.1.md b/docs/source/markdown/podman-secret-inspect.1.md index 1d03dc8293..056c6decf4 100644 --- a/docs/source/markdown/podman-secret-inspect.1.md +++ b/docs/source/markdown/podman-secret-inspect.1.md @@ -23,6 +23,7 @@ Format secret output using Go template. |--------------------------|-------------------------------------------------------------------| | .CreatedAt | When secret was created (relative timestamp, human-readable) | | .ID | ID of secret | +| .SecretData | Secret Data (Displayed only with --showsecret option) | | .Spec ... | Details of secret | | .Spec.Driver | Driver info | | .Spec.Driver.Name | Driver name (string) | @@ -39,12 +40,16 @@ Print usage statement. Print inspect output in human-readable format +#### **--showsecret** + +Display secret data ## EXAMPLES ``` $ podman secret inspect mysecret $ podman secret inspect --format "{{.Name} {{.Scope}}" mysecret +$ podman secret inspect --showsecret --format "{{.Name} {{.SecretData}}" mysecret ``` ## SEE ALSO diff --git a/docs/source/markdown/podman-secret-ls.1.md.in b/docs/source/markdown/podman-secret-ls.1.md.in index 00ba431c24..839467e4fc 100644 --- a/docs/source/markdown/podman-secret-ls.1.md.in +++ b/docs/source/markdown/podman-secret-ls.1.md.in @@ -34,6 +34,7 @@ Valid placeholders for the Go template are listed below: | ------------------------ | ----------------------------------------------------------------- | | .CreatedAt | When secret was created (relative timestamp, human-readable) | | .ID | ID of secret | +| .SecretData | Secret Data (Displayed only with --showsecret option) | | .Spec ... | Details of secret | | .Spec.Driver | Driver info | | .Spec.Driver.Name | Driver name (string) | diff --git a/pkg/api/handlers/compat/secrets.go b/pkg/api/handlers/compat/secrets.go index 847f05f273..f550143591 100644 --- a/pkg/api/handlers/compat/secrets.go +++ b/pkg/api/handlers/compat/secrets.go @@ -15,6 +15,7 @@ import ( "github.com/containers/podman/v4/pkg/domain/entities" "github.com/containers/podman/v4/pkg/domain/infra/abi" "github.com/containers/podman/v4/pkg/util" + "github.com/gorilla/schema" ) func ListSecrets(w http.ResponseWriter, r *http.Request) { @@ -51,11 +52,25 @@ func ListSecrets(w http.ResponseWriter, r *http.Request) { } func InspectSecret(w http.ResponseWriter, r *http.Request) { + decoder := r.Context().Value(api.DecoderKey).(*schema.Decoder) runtime := r.Context().Value(api.RuntimeKey).(*libpod.Runtime) name := utils.GetName(r) names := []string{name} + query := struct { + ShowSecret bool `schema:"showsecret"` + }{ + // override any golang type defaults + } + + if err := decoder.Decode(&query, r.URL.Query()); err != nil { + utils.Error(w, http.StatusBadRequest, fmt.Errorf("failed to parse parameters for %s: %w", r.URL.String(), err)) + return + } ic := abi.ContainerEngine{Libpod: runtime} - reports, errs, err := ic.SecretInspect(r.Context(), names) + opts := entities.SecretInspectOptions{} + opts.ShowSecret = query.ShowSecret + + reports, errs, err := ic.SecretInspect(r.Context(), names, opts) if err != nil { utils.InternalServerError(w, err) return diff --git a/pkg/api/server/register_secrets.go b/pkg/api/server/register_secrets.go index f5bda79d37..c50be3c971 100644 --- a/pkg/api/server/register_secrets.go +++ b/pkg/api/server/register_secrets.go @@ -79,6 +79,11 @@ func (s *APIServer) registerSecretHandlers(r *mux.Router) error { // type: string // required: true // description: the name or ID of the secret + // - in: query + // name: showsecret + // type: boolean + // description: Display Secret + // default: false // produces: // - application/json // responses: diff --git a/pkg/bindings/secrets/secrets.go b/pkg/bindings/secrets/secrets.go index d78e3902f8..2cd392a144 100644 --- a/pkg/bindings/secrets/secrets.go +++ b/pkg/bindings/secrets/secrets.go @@ -33,6 +33,9 @@ func List(ctx context.Context, options *ListOptions) ([]*entities.SecretInfoRepo // Inspect returns low-level information about a secret. func Inspect(ctx context.Context, nameOrID string, options *InspectOptions) (*entities.SecretInfoReport, error) { + if options == nil { + options = new(InspectOptions) + } var ( inspect *entities.SecretInfoReport ) @@ -40,12 +43,15 @@ func Inspect(ctx context.Context, nameOrID string, options *InspectOptions) (*en if err != nil { return nil, err } - response, err := conn.DoRequest(ctx, nil, http.MethodGet, "/secrets/%s/json", nil, nil, nameOrID) + params, err := options.ToParams() + if err != nil { + return nil, err + } + response, err := conn.DoRequest(ctx, nil, http.MethodGet, "/secrets/%s/json", params, nil, nameOrID) if err != nil { return inspect, err } defer response.Body.Close() - return inspect, response.Process(&inspect) } diff --git a/pkg/bindings/secrets/types.go b/pkg/bindings/secrets/types.go index a84658826b..71f8c07c70 100644 --- a/pkg/bindings/secrets/types.go +++ b/pkg/bindings/secrets/types.go @@ -11,6 +11,7 @@ type ListOptions struct { // //go:generate go run ../generator/generator.go InspectOptions type InspectOptions struct { + ShowSecret *bool } // RemoveOptions are optional options for removing secrets diff --git a/pkg/bindings/secrets/types_inspect_options.go b/pkg/bindings/secrets/types_inspect_options.go index fe26ae3b86..5ef8a1276c 100644 --- a/pkg/bindings/secrets/types_inspect_options.go +++ b/pkg/bindings/secrets/types_inspect_options.go @@ -16,3 +16,18 @@ func (o *InspectOptions) Changed(fieldName string) bool { func (o *InspectOptions) ToParams() (url.Values, error) { return util.ToParams(o) } + +// WithShowSecret set field ShowSecret to given value +func (o *InspectOptions) WithShowSecret(value bool) *InspectOptions { + o.ShowSecret = &value + return o +} + +// GetShowSecret returns value of field ShowSecret +func (o *InspectOptions) GetShowSecret() bool { + if o.ShowSecret == nil { + var z bool + return z + } + return *o.ShowSecret +} diff --git a/pkg/domain/entities/engine_container.go b/pkg/domain/entities/engine_container.go index 7983502339..1ae819c5ab 100644 --- a/pkg/domain/entities/engine_container.go +++ b/pkg/domain/entities/engine_container.go @@ -93,7 +93,7 @@ type ContainerEngine interface { //nolint:interfacebloat PodUnpause(ctx context.Context, namesOrIds []string, options PodunpauseOptions) ([]*PodUnpauseReport, error) SetupRootless(ctx context.Context, noMoveProcess bool) error SecretCreate(ctx context.Context, name string, reader io.Reader, options SecretCreateOptions) (*SecretCreateReport, error) - SecretInspect(ctx context.Context, nameOrIDs []string) ([]*SecretInfoReport, []error, error) + SecretInspect(ctx context.Context, nameOrIDs []string, options SecretInspectOptions) ([]*SecretInfoReport, []error, error) SecretList(ctx context.Context, opts SecretListRequest) ([]*SecretInfoReport, error) SecretRm(ctx context.Context, nameOrID []string, opts SecretRmOptions) ([]*SecretRmReport, error) SecretExists(ctx context.Context, nameOrID string) (*BoolReport, error) diff --git a/pkg/domain/entities/secrets.go b/pkg/domain/entities/secrets.go index 5686b90e9a..b485ca1d5d 100644 --- a/pkg/domain/entities/secrets.go +++ b/pkg/domain/entities/secrets.go @@ -16,6 +16,10 @@ type SecretCreateOptions struct { Labels map[string]string } +type SecretInspectOptions struct { + ShowSecret bool +} + type SecretListRequest struct { Filters map[string][]string } @@ -38,10 +42,11 @@ type SecretRmReport struct { } type SecretInfoReport struct { - ID string - CreatedAt time.Time - UpdatedAt time.Time - Spec SecretSpec + ID string + CreatedAt time.Time + UpdatedAt time.Time + Spec SecretSpec + SecretData string `json:"SecretData,omitempty"` } type SecretInfoReportCompat struct { diff --git a/pkg/domain/infra/abi/secrets.go b/pkg/domain/infra/abi/secrets.go index 45d7246418..1efb45434e 100644 --- a/pkg/domain/infra/abi/secrets.go +++ b/pkg/domain/infra/abi/secrets.go @@ -58,7 +58,11 @@ func (ic *ContainerEngine) SecretCreate(ctx context.Context, name string, reader }, nil } -func (ic *ContainerEngine) SecretInspect(ctx context.Context, nameOrIDs []string) ([]*entities.SecretInfoReport, []error, error) { +func (ic *ContainerEngine) SecretInspect(ctx context.Context, nameOrIDs []string, options entities.SecretInspectOptions) ([]*entities.SecretInfoReport, []error, error) { + var ( + secret *secrets.Secret + data []byte + ) manager, err := ic.Libpod.SecretsManager() if err != nil { return nil, nil, err @@ -66,7 +70,11 @@ func (ic *ContainerEngine) SecretInspect(ctx context.Context, nameOrIDs []string errs := make([]error, 0, len(nameOrIDs)) reports := make([]*entities.SecretInfoReport, 0, len(nameOrIDs)) for _, nameOrID := range nameOrIDs { - secret, err := manager.Lookup(nameOrID) + if options.ShowSecret { + secret, data, err = manager.LookupSecretData(nameOrID) + } else { + secret, err = manager.Lookup(nameOrID) + } if err != nil { if strings.Contains(err.Error(), "no such secret") { errs = append(errs, err) @@ -90,6 +98,7 @@ func (ic *ContainerEngine) SecretInspect(ctx context.Context, nameOrIDs []string }, Labels: secret.Labels, }, + SecretData: string(data), } reports = append(reports, report) } diff --git a/pkg/domain/infra/tunnel/secrets.go b/pkg/domain/infra/tunnel/secrets.go index 47ce328356..faf0b0d432 100644 --- a/pkg/domain/infra/tunnel/secrets.go +++ b/pkg/domain/infra/tunnel/secrets.go @@ -23,11 +23,14 @@ func (ic *ContainerEngine) SecretCreate(ctx context.Context, name string, reader return created, nil } -func (ic *ContainerEngine) SecretInspect(ctx context.Context, nameOrIDs []string) ([]*entities.SecretInfoReport, []error, error) { +func (ic *ContainerEngine) SecretInspect(ctx context.Context, nameOrIDs []string, options entities.SecretInspectOptions) ([]*entities.SecretInfoReport, []error, error) { allInspect := make([]*entities.SecretInfoReport, 0, len(nameOrIDs)) errs := make([]error, 0, len(nameOrIDs)) + opts := new(secrets.InspectOptions). + WithShowSecret(options.ShowSecret) + for _, name := range nameOrIDs { - inspected, err := secrets.Inspect(ic.ClientCtx, name, nil) + inspected, err := secrets.Inspect(ic.ClientCtx, name, opts) if err != nil { errModel, ok := err.(*errorhandling.ErrorModel) if !ok { diff --git a/test/e2e/secret_test.go b/test/e2e/secret_test.go index f57f62780d..a93b3d8c1a 100644 --- a/test/e2e/secret_test.go +++ b/test/e2e/secret_test.go @@ -6,6 +6,7 @@ import ( "path/filepath" . "github.com/containers/podman/v4/test/utils" + "github.com/containers/storage/pkg/stringid" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" . "github.com/onsi/gomega/gexec" @@ -48,8 +49,9 @@ var _ = Describe("Podman secret", func() { }) It("podman secret inspect", func() { + random := stringid.GenerateRandomID() secretFilePath := filepath.Join(podmanTest.TempDir, "secret") - err := os.WriteFile(secretFilePath, []byte("mysecret"), 0755) + err := os.WriteFile(secretFilePath, []byte(random), 0755) Expect(err).ToNot(HaveOccurred()) session := podmanTest.Podman([]string{"secret", "create", "a", secretFilePath}) @@ -61,6 +63,16 @@ var _ = Describe("Podman secret", func() { inspect.WaitWithDefaultTimeout() Expect(inspect).Should(Exit(0)) Expect(inspect.OutputToString()).To(BeValidJSON()) + + inspect = podmanTest.Podman([]string{"secret", "inspect", "--format", "{{ .SecretData }}", secrID}) + inspect.WaitWithDefaultTimeout() + Expect(inspect).Should(Exit(0)) + Expect(inspect.OutputToString()).To(Equal("")) + + inspect = podmanTest.Podman([]string{"secret", "inspect", "--showsecret", "--format", "{{ .SecretData }}", secrID}) + inspect.WaitWithDefaultTimeout() + Expect(inspect).Should(Exit(0)) + Expect(inspect.OutputToString()).To(Equal(random)) }) It("podman secret inspect with --format", func() {