Skip to content

Commit

Permalink
[8.7](backport #2246) Enable verification with alternative PGP keys (#…
Browse files Browse the repository at this point in the history
…2258)

* Enable verification with alternative PGP keys (#2246)

Enable verification with alternative PGP keys (#2246)

(cherry picked from commit f8776d5)

* Support only HTTPS for remote upgrade PGP (#2268)

Support only HTTPS for remote upgrade PGP (#2268)

---------

Co-authored-by: Michal Pristas <michal.pristas@gmail.com>
  • Loading branch information
mergify[bot] and michalpristas committed Feb 16, 2023
1 parent 31c4dcf commit 3dc3248
Show file tree
Hide file tree
Showing 32 changed files with 517 additions and 280 deletions.
3 changes: 0 additions & 3 deletions _meta/config/common.p2.yml.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -114,9 +114,6 @@ inputs:
# target_directory: "${path.data}/downloads"
# # timeout for downloading package
# timeout: 120s
# # file path to a public key used for verifying downloaded artifacts
# # if not file is present agent will try to load public key from elastic.co website.
# pgpfile: "${path.data}/elastic.pgp"
# # install_path describes the location of installed packages/programs. It is also used
# # for reading program specifications.
# install_path: "${path.data}/install"
Expand Down
3 changes: 0 additions & 3 deletions _meta/config/common.reference.p2.yml.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,6 @@ inputs:
# target_directory: "${path.data}/downloads"
# # timeout for downloading package
# timeout: 120s
# # file path to a public key used for verifying downloaded artifacts
# # if not file is present agent will try to load public key from elastic.co website.
# pgpfile: "${path.data}/elastic.pgp"
# # install_path describes the location of installed packages/programs. It is also used
# # for reading program specifications.
# install_path: "${path.data}/install"
Expand Down
3 changes: 0 additions & 3 deletions _meta/config/elastic-agent.docker.yml.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,6 @@ inputs:
# target_directory: "${path.data}/downloads"
# # timeout for downloading package
# timeout: 120s
# # file path to a public key used for verifying downloaded artifacts
# # if not file is present agent will try to load public key from elastic.co website.
# pgpfile: "${path.data}/elastic.pgp"
# # install_path describes the location of installed packages/programs. It is also used
# # for reading program specifications.
# install_path: "${path.data}/install"
Expand Down
3 changes: 0 additions & 3 deletions _meta/elastic-agent.fleet.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,6 @@ fleet:
# target_directory: "${path.data}/downloads"
# # timeout for downloading package
# timeout: 120s
# # file path to a public key used for verifying downloaded artifacts
# # if not file is present Elastic Agent will try to load public key from elastic.co website.
# pgpfile: "${path.data}/elastic.pgp"
# # install_path describes the location of installed packages/programs. It is also used
# # for reading program specifications.
# install_path: "${path.data}/install"
Expand Down
3 changes: 0 additions & 3 deletions _meta/elastic-agent.yml
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,6 @@ inputs:
# target_directory: "${path.data}/downloads"
# # timeout for downloading package
# timeout: 120s
# # file path to a public key used for verifying downloaded artifacts
# # if not file is present agent will try to load public key from elastic.co website.
# pgpfile: "${path.data}/elastic.pgp"
# # install_path describes the location of installed packages/programs. It is also used
# # for reading program specifications.
# install_path: "${path.data}/install"
Expand Down
10 changes: 10 additions & 0 deletions control_v2.proto
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,16 @@ message UpgradeRequest {
// If provided the upgrade process will use the provided sourceURI instead of the configured
// sourceURI in the configuration.
string sourceURI = 2;

// (Optional) Overrides predefined behavior for agent package verification.
//
// If provided Elastic Agent package is not checked for signature during upgrade.
bool skipVerify = 3;

// (Optional) Overrides predefined behavior for agent package verification.
//
// If provided Elastic Agent package is checked against these pgp keys as well.
repeated string pgpBytes = 4;
}

// A upgrade response message.
Expand Down
16 changes: 10 additions & 6 deletions dev-tools/cmd/buildpgp/build_pgp.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 0 additions & 3 deletions elastic-agent.docker.yml
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,6 @@ inputs:
# target_directory: "${path.data}/downloads"
# # timeout for downloading package
# timeout: 120s
# # file path to a public key used for verifying downloaded artifacts
# # if not file is present agent will try to load public key from elastic.co website.
# pgpfile: "${path.data}/elastic.pgp"
# # install_path describes the location of installed packages/programs. It is also used
# # for reading program specifications.
# install_path: "${path.data}/install"
Expand Down
3 changes: 0 additions & 3 deletions elastic-agent.reference.yml
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,6 @@ inputs:
# target_directory: "${path.data}/downloads"
# # timeout for downloading package
# timeout: 120s
# # file path to a public key used for verifying downloaded artifacts
# # if not file is present agent will try to load public key from elastic.co website.
# pgpfile: "${path.data}/elastic.pgp"
# # install_path describes the location of installed packages/programs. It is also used
# # for reading program specifications.
# install_path: "${path.data}/install"
Expand Down
3 changes: 0 additions & 3 deletions elastic-agent.yml
Original file line number Diff line number Diff line change
Expand Up @@ -120,9 +120,6 @@ inputs:
# target_directory: "${path.data}/downloads"
# # timeout for downloading package
# timeout: 120s
# # file path to a public key used for verifying downloaded artifacts
# # if not file is present agent will try to load public key from elastic.co website.
# pgpfile: "${path.data}/elastic.pgp"
# # install_path describes the location of installed packages/programs. It is also used
# # for reading program specifications.
# install_path: "${path.data}/install"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,5 +38,6 @@ func (h *Upgrade) Handle(ctx context.Context, a fleetapi.Action, _ acker.Acker)
return fmt.Errorf("invalid type, expected ActionUpgrade and received %T", a)
}

return h.coord.Upgrade(ctx, action.Version, action.SourceURI, action)
// always perform PGP checks
return h.coord.Upgrade(ctx, action.Version, action.SourceURI, action, false)
}
6 changes: 3 additions & 3 deletions internal/pkg/agent/application/coordinator/coordinator.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ type UpgradeManager interface {
Reload(rawConfig *config.Config) error

// Upgrade upgrades running agent.
Upgrade(ctx context.Context, version string, sourceURI string, action *fleetapi.ActionUpgrade) (_ reexec.ShutdownCallbackFn, err error)
Upgrade(ctx context.Context, version string, sourceURI string, action *fleetapi.ActionUpgrade, skipVerifyOverride bool, pgpBytes ...string) (_ reexec.ShutdownCallbackFn, err error)

// Ack is used on startup to check if the agent has upgraded and needs to send an ack for the action
Ack(ctx context.Context, acker acker.Acker) error
Expand Down Expand Up @@ -274,7 +274,7 @@ func (c *Coordinator) ReExec(callback reexec.ShutdownCallbackFn, argOverrides ..
}

// Upgrade runs the upgrade process.
func (c *Coordinator) Upgrade(ctx context.Context, version string, sourceURI string, action *fleetapi.ActionUpgrade) error {
func (c *Coordinator) Upgrade(ctx context.Context, version string, sourceURI string, action *fleetapi.ActionUpgrade, skipVerifyOverride bool, pgpBytes ...string) error {
// early check outside of upgrader before overridding the state
if !c.upgradeMgr.Upgradeable() {
return ErrNotUpgradable
Expand All @@ -295,7 +295,7 @@ func (c *Coordinator) Upgrade(ctx context.Context, version string, sourceURI str
state: agentclient.Upgrading,
message: fmt.Sprintf("Upgrading to version %s", version),
}
cb, err := c.upgradeMgr.Upgrade(ctx, version, sourceURI, action)
cb, err := c.upgradeMgr.Upgrade(ctx, version, sourceURI, action, skipVerifyOverride, pgpBytes...)
if err != nil {
c.state.overrideState = nil
return err
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,13 @@ func NewVerifier(verifiers ...download.Verifier) *Verifier {
}

// Verify checks the package from configured source.
func (e *Verifier) Verify(a artifact.Artifact, version string) error {
func (e *Verifier) Verify(a artifact.Artifact, version string, pgpBytes ...string) error {
var err error
var checksumMismatchErr *download.ChecksumMismatchError
var invalidSignatureErr *download.InvalidSignatureError

for _, v := range e.vv {
e := v.Verify(a, version)
e := v.Verify(a, version, pgpBytes...)
if e == nil {
// Success
return nil
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ type ErrorVerifier struct {
called bool
}

func (d *ErrorVerifier) Verify(a artifact.Artifact, version string) error {
func (d *ErrorVerifier) Verify(a artifact.Artifact, version string, _ ...string) error {
d.called = true
return errors.New("failing")
}
Expand All @@ -29,7 +29,7 @@ type FailVerifier struct {
called bool
}

func (d *FailVerifier) Verify(a artifact.Artifact, version string) error {
func (d *FailVerifier) Verify(a artifact.Artifact, version string, _ ...string) error {
d.called = true
return &download.InvalidSignatureError{}
}
Expand All @@ -40,7 +40,7 @@ type SuccVerifier struct {
called bool
}

func (d *SuccVerifier) Verify(a artifact.Artifact, version string) error {
func (d *SuccVerifier) Verify(a artifact.Artifact, version string, _ ...string) error {
d.called = true
return nil
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,15 @@ package fs
import (
"fmt"
"io/ioutil"
"net/http"
"os"
"path/filepath"

"github.com/elastic/elastic-agent-libs/transport/httpcommon"
"github.com/elastic/elastic-agent/internal/pkg/agent/application/upgrade/artifact"
"github.com/elastic/elastic-agent/internal/pkg/agent/application/upgrade/artifact/download"
"github.com/elastic/elastic-agent/internal/pkg/agent/errors"
"github.com/elastic/elastic-agent/pkg/core/logger"
)

const (
Expand All @@ -24,29 +27,43 @@ const (
// embedded into Elastic Agent.
type Verifier struct {
config *artifact.Config
client http.Client
pgpBytes []byte
allowEmptyPgp bool
log *logger.Logger
}

// NewVerifier creates a verifier checking downloaded package on preconfigured
// location against a key stored on elastic.co website.
func NewVerifier(config *artifact.Config, allowEmptyPgp bool, pgp []byte) (*Verifier, error) {
func NewVerifier(log *logger.Logger, config *artifact.Config, allowEmptyPgp bool, pgp []byte) (*Verifier, error) {
if len(pgp) == 0 && !allowEmptyPgp {
return nil, errors.New("expecting PGP but retrieved none", errors.TypeSecurity)
}

client, err := config.HTTPTransportSettings.Client(
httpcommon.WithAPMHTTPInstrumentation(),
httpcommon.WithModRoundtripper(func(rt http.RoundTripper) http.RoundTripper {
return download.WithHeaders(rt, download.Headers)
}),
)
if err != nil {
return nil, err
}

v := &Verifier{
config: config,
client: *client,
allowEmptyPgp: allowEmptyPgp,
pgpBytes: pgp,
log: log,
}

return v, nil
}

// Verify checks downloaded package on preconfigured
// location against a key stored on elastic.co website.
func (v *Verifier) Verify(a artifact.Artifact, version string) error {
func (v *Verifier) Verify(a artifact.Artifact, version string, pgpBytes ...string) error {
filename, err := artifact.GetArtifactName(a, version, v.config.OS(), v.config.Arch())
if err != nil {
return errors.New(err, "retrieving package name")
Expand All @@ -63,7 +80,7 @@ func (v *Verifier) Verify(a artifact.Artifact, version string) error {
return err
}

if err = v.verifyAsc(fullPath); err != nil {
if err = v.verifyAsc(fullPath, pgpBytes...); err != nil {
var invalidSignatureErr *download.InvalidSignatureError
if errors.As(err, &invalidSignatureErr) {
os.Remove(fullPath + ".asc")
Expand All @@ -74,11 +91,52 @@ func (v *Verifier) Verify(a artifact.Artifact, version string) error {
return nil
}

func (v *Verifier) verifyAsc(fullPath string) error {
if len(v.pgpBytes) == 0 {
func (v *Verifier) Reload(c *artifact.Config) error {
// reload client
client, err := c.HTTPTransportSettings.Client(
httpcommon.WithAPMHTTPInstrumentation(),
httpcommon.WithModRoundtripper(func(rt http.RoundTripper) http.RoundTripper {
return download.WithHeaders(rt, download.Headers)
}),
)
if err != nil {
return errors.New(err, "http.verifier: failed to generate client out of config")
}

v.client = *client
v.config = c

return nil
}

func (v *Verifier) verifyAsc(fullPath string, pgpSources ...string) error {
var pgpBytes [][]byte
if len(v.pgpBytes) > 0 {
v.log.Infof("Default PGP being appended")
pgpBytes = append(pgpBytes, v.pgpBytes)
}

for _, check := range pgpSources {
if len(check) == 0 {
continue
}
raw, err := download.PgpBytesFromSource(check, v.client)
if err != nil {
return err
}
if len(raw) == 0 {
continue
}

pgpBytes = append(pgpBytes, raw)
}

if len(pgpBytes) == 0 {
// no pgp available skip verification process
v.log.Infof("No checks defined")
return nil
}
v.log.Infof("Using %d PGP keys", len(pgpBytes))

ascBytes, err := v.getPublicAsc(fullPath)
if err != nil && v.allowEmptyPgp {
Expand All @@ -88,7 +146,20 @@ func (v *Verifier) verifyAsc(fullPath string) error {
return err
}

return download.VerifyGPGSignature(fullPath, ascBytes, v.pgpBytes)
for i, check := range pgpBytes {
err = download.VerifyGPGSignature(fullPath, ascBytes, check)
if err == nil {
// verify successful
v.log.Infof("Verification with PGP[%d] successful", i)
return nil
}
v.log.Warnf("Verification with PGP[%d] succfailed: %v", i, err)
}

v.log.Warnf("Verification failed")

// return last error
return err
}

func (v *Verifier) getPublicAsc(fullPath string) ([]byte, error) {
Expand Down
Loading

0 comments on commit 3dc3248

Please sign in to comment.