[Ingest Manager] Agent includes pgp file #19480
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
michalpristas
added
enhancement
Team:Ingest Management
Ingest Management:beta1
botelastic
bot
added
needs_team
💔 Tests Failed
Expand to view the summary
Build stats
Test stats 🧪
Test errorsExpand to view the tests failures
Steps errorsExpand to view the steps failures
Log outputExpand to view the last 100 lines of log output
|
|
related to this is including ASC files for beats into our package. I contacted Chris to figure this out and come up with feasible strategy which would also help us with including endpoint probably |
michalpristas
added
the
needs_backport
|
I think this looks good. By default PGP is used and required, only |
|
not activating FS verifier just yet as for: https://github.com/elastic/infra/issues/21487 |
|
Pinging @elastic/ingest-management (Team:Ingest Management) |
|
talking to infra about including asc files not merging before then |
michalpristas
removed
the
Ingest Management:beta1
This was referenced Jul 23, 2020
|
@michalpristas I think everything is in place in infra to move this forward? |
|
@michalpristas This should work with nightly snapshot too correct? When this get in, we need to communicate it with the other teams. |
|
should work with snapshots yeah |
scunningham
suggested changes
Sep 3, 2020
There was a problem hiding this comment.
It appears this code pulls the PGP Key down from the CDN on the fly. It would be far safer to embed the key in the binary at build time. Unless I'm reading it wrong, this implementation fetches the PGP key dynamically via https and validates against it with no additional validation that we we received the proper key. A determined attacker may be able to man in the middle the HTTPS transaction and deliver an invalid key.
ruflin
reviewed
Sep 4, 2020
x-pack/elastic-agent/magefile.go
Outdated
| fetchPgp = false | ||
| } | ||
| } | ||
| fmt.Println("fetching pgp", fetchPgp) |
There was a problem hiding this comment.
this whole section will get removed
blakerouse
approved these changes
Sep 14, 2020
|
@michalpristas @blakerouse @ruflin we will need to document the |
scunningham
approved these changes
Sep 14, 2020
There was a problem hiding this comment.
Thanks. Question; does the PGP Key pull down during each release build; or we do we used a checked-in PGP Key unless somebody explicitly runs the code that pulls down the key and embeds it?
Pulling down the key on the fly during a build slightly concerns me as it allows an admittedly narrow window for attack. Subsequent unit tests should catch an injection, but we should validate that.
|
@scunningham nono key is prepacked in this PR and once we will need it updated we will update the file and run |
|
@michalpristas Fantastic, thank you! |
|
Faliures not related |
michalpristas
added a commit
to michalpristas/beats
that referenced
this pull request
Sep 22, 2020
[Ingest Manager] Agent includes pgp file (elastic#19480)
6 tasks
v1v
added a commit
to v1v/beats
that referenced
this pull request
Sep 24, 2020
…ne-2.0-arm * upstream/master: (29 commits) Fix librpm installation in auditbeat build (elastic#21239) Fix prometheus default config (elastic#21253) Fix dev guide test command (elastic#21254) Move aws lambda metricset to GA (elastic#21255) [Docs] Typo in table syntax (elastic#20227) [ECS] Adds related.hosts to capture all hostnames and host identifiers on an event. (elastic#21160) Add recursive split to httpjson (elastic#21214) [DOCS] Add beat specific start widgets (elastic#21217) Fix timestamp handling in remote_write (elastic#21166) Fix aws, azure and googlecloud compute dashboards (elastic#21098) Add acceptable event log keys to winlog (elastic#21205) Add elastic-agent to gitignore (elastic#21219) Add cloudfoundry tags to events (elastic#21177) [Ingest Manager] Agent includes pgp file (elastic#19480) Add compatibility note about ingress-controller-v0.34.1 (elastic#21209) [Ingest Manager] Support for UPGRADE_ACTION (elastic#21002) Fix libbeat.output.*.bytes metrics of Elasticsearch output (elastic#21197) [packaging] use docker.elastic.co/ubi8/ubi-minimal (elastic#21154) Add host inventory metrics to system module (elastic#20415) [Filebeat][Azure Module] Fixing event.outcome from result_type issue (elastic#20998) ...
v1v
added a commit
to v1v/beats
that referenced
this pull request
Sep 24, 2020
…ne-2.0 * upstream/master: (33 commits) Stop running agent container as root by default (elastic#21213) Stop running auditbeat container as root by default (elastic#21202) Fix autodiscover flaky tests (elastic#21242) [Ingest Manager] Enabled dev builds (elastic#21241) Fix librpm installation in auditbeat build (elastic#21239) Fix prometheus default config (elastic#21253) Fix dev guide test command (elastic#21254) Move aws lambda metricset to GA (elastic#21255) [Docs] Typo in table syntax (elastic#20227) [ECS] Adds related.hosts to capture all hostnames and host identifiers on an event. (elastic#21160) Add recursive split to httpjson (elastic#21214) [DOCS] Add beat specific start widgets (elastic#21217) Fix timestamp handling in remote_write (elastic#21166) Fix aws, azure and googlecloud compute dashboards (elastic#21098) Add acceptable event log keys to winlog (elastic#21205) Add elastic-agent to gitignore (elastic#21219) Add cloudfoundry tags to events (elastic#21177) [Ingest Manager] Agent includes pgp file (elastic#19480) Add compatibility note about ingress-controller-v0.34.1 (elastic#21209) [Ingest Manager] Support for UPGRADE_ACTION (elastic#21002) ...
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What does this PR do?
This PR introduces baked in PGP file with a flag.
If
DEV=trueis specified duringmage buildPGP is not included and checks are omitted .Otherwise PGP is provided and passing check is required.
The solution works well with connected agent to internet, but locally baked in packages are a bit tricky as we need to find a way of including asc files into agent package so they can be checked.
Why is it important?
More security running external binaries
Checklist
CHANGELOG.next.asciidocorCHANGELOG-developer.next.asciidoc.