Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

internal/envoy: always extract envoy #2160

Merged
merged 1 commit into from
Apr 30, 2021

Conversation

travisgroth
Copy link
Contributor

Summary

We currently optimize for startup time and attempt to cache the envoy binary once we've extracted it on a system. This creates starting security conditions that are hard to validate even with checksumming of the binary and atomic creation.

Changes:

  • switch to a randomized subdirectory created on-demand, to help ensure we are running the same binary we checksum
  • clean the entire base directory before extraction, so that we don't consume additional disk space every time we start on a system
  • always extract a fresh copy of envoy

Related issues

#1908

Checklist

  • reference any related issues
  • updated docs
  • updated unit tests
  • updated UPGRADING.md
  • add appropriate tag (improvement / bug / etc)
  • ready for review

@travisgroth travisgroth requested a review from a team as a code owner April 30, 2021 19:21
tmpDir, err := os.MkdirTemp(embeddedFilesBaseDirectory, "envoy-")
outPath = filepath.Join(tmpDir, "envoy")

log.Info(ctx).Str("path", outPath).Msg("extracting envoy binary")
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just in case the operation takes time, let people know why we're "paused".

@codecov
Copy link

codecov bot commented Apr 30, 2021

Codecov Report

Merging #2160 (3489d57) into master (699ebf0) will decrease coverage by 0.1%.
The diff coverage is 0.0%.

@@           Coverage Diff            @@
##           master   #2160     +/-   ##
========================================
- Coverage    59.3%   59.1%   -0.2%     
========================================
  Files         173     173             
  Lines       11670   11670             
========================================
- Hits         6925    6904     -21     
- Misses       3950    3966     +16     
- Partials      795     800      +5     
Impacted Files Coverage Δ
internal/envoy/embed.go 0.0% <0.0%> (ø)
internal/envoy/envoy.go 3.0% <0.0%> (ø)
internal/controlplane/events.go 64.5% <0.0%> (-13.0%) ⬇️
pkg/storage/inmemory/stream.go 69.3% <0.0%> (-4.1%) ⬇️
pkg/grpc/databroker/syncer.go 93.7% <0.0%> (-2.5%) ⬇️
pkg/storage/redis/redis.go 68.1% <0.0%> (-2.5%) ⬇️
pkg/storage/inmemory/backend.go 83.2% <0.0%> (-2.3%) ⬇️
internal/controlplane/xdsmgr/xdsmgr.go 70.8% <0.0%> (-1.6%) ⬇️
authenticate/state.go 65.4% <0.0%> (-0.9%) ⬇️
internal/sessions/cookie/cookie_store.go 97.6% <0.0%> (+<0.1%) ⬆️
... and 1 more

@travisgroth travisgroth merged commit dae1836 into pomerium:master Apr 30, 2021
@travisgroth travisgroth deleted the safer_binary_handling branch April 30, 2021 19:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants