Conversation
A recent [Twitter thread][mudge] suggested that tools like [`checksec`][checksec] be used to validate release binaries. Checksec reports whether modern security features like stack canaries are employed. Proxy builds appear to do pretty well out-of-the-box. This change introduces a checksec.sh wrapper that is used by the Makefile during packaging. A new _package_ github action is introduced to provide `checksec` and `jq` dependencies at runtime. (Note: the version of checksec provided by debian does not include JSON output, so it is instead fetched directly from GitHub). During an automated release, the generated checksec is compared to an expected set of values and, if a regression is detected, the release will fail. [mudge]: https://twitter.com/dotMudge/status/1249359519471341569 [checksec]: https://github.com/slimm609/checksec.sh
hawkw
approved these changes
Apr 14, 2020
Contributor
hawkw
left a comment
hawkw
left a comment
There was a problem hiding this comment.
couple nits, but otherwise, this seems good to me!
Comment on lines
+7
to
+9
| "rpath": "no", | ||
| "runpath": "no", | ||
| "symbols": "no" |
Contributor
There was a problem hiding this comment.
nit/tioli: it might be nice to have comments explaining why we expect to see "no" for these?
Member
Author
There was a problem hiding this comment.
I didn't do too much research into all of the various checks checksec does, but i'd welcome additional docs.
I think in the case of symbols, particularly, the guidance is intended to make reverse-engineering more difficult. In our case, we mostly care that the binary is small (and we have another build option that includes symbols as a separate file)
Member
Author
There was a problem hiding this comment.
It appears that rpath/runpath can be used to influence dynamic linking in dangerous ways...
Contributor
There was a problem hiding this comment.
yeah, i thought that was the case for symbols. I wasn't super familiar with the rpath/rumpath stuff.
olix0r
added a commit
to linkerd/linkerd2
that referenced
this pull request
Apr 15, 2020
This release includes a new protocol detection timeout, which prevents clients from consuming resources indefinitely when they do not send any data. Additionally: the proxy's admin endpoint now supports a `/live` endpoint for liveness checks, and a feature has been added to enrich tracing metadata from a file of label/values. --- * Add Labels from a path as oc-collector attributes (linkerd/linkerd2-proxy#463) * Add liveness endpoint to admin server (linkerd/linkerd2-proxy#470) * docker: Use buildkit for caching (linkerd/linkerd2-proxy#472) * Makefile: Use STRIP variable with strip as default (linkerd/linkerd2-proxy#475) * Add checksec to the release process (linkerd/linkerd2-proxy#476) * Time out protocol detect futures (linkerd/linkerd2-proxy#464) * Ensure that checksec is executable (linkerd/linkerd2-proxy#477) * Fix the checksec URL (linkerd/linkerd2-proxy#478) * Undo hardcoded release version (linkerd/linkerd2-proxy#479)
olix0r
added a commit
to linkerd/linkerd2
that referenced
this pull request
Apr 16, 2020
This release includes a new protocol detection timeout, which prevents clients from consuming resources indefinitely when they do not send any data. Additionally: the proxy's admin endpoint now supports a `/live` endpoint for liveness checks, and a feature has been added to enrich tracing metadata from a file of label/values. --- * Add Labels from a path as oc-collector attributes (linkerd/linkerd2-proxy#463) * Add liveness endpoint to admin server (linkerd/linkerd2-proxy#470) * docker: Use buildkit for caching (linkerd/linkerd2-proxy#472) * Makefile: Use STRIP variable with strip as default (linkerd/linkerd2-proxy#475) * Add checksec to the release process (linkerd/linkerd2-proxy#476) * Time out protocol detect futures (linkerd/linkerd2-proxy#464) * Ensure that checksec is executable (linkerd/linkerd2-proxy#477) * Fix the checksec URL (linkerd/linkerd2-proxy#478) * Undo hardcoded release version (linkerd/linkerd2-proxy#479)
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
A recent Twitter thread suggested that tools like
checksecbe used to validate release binaries. Checksecreports whether modern security features like stack canaries are
employed. Proxy builds appear to do pretty well out-of-the-box.
This change introduces a checksec.sh wrapper that is used by the
Makefile during packaging. A new package github action is introduced
to provide
checksecandjqdependencies at runtime. (Note: theversion of checksec provided by debian does not include JSON output, so
it is instead fetched directly from GitHub).
During an automated release, the generated checksec is compared to an
expected set of values and, if a regression is detected, the release
will fail.