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.

Sign up for GitHub

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

Jump to bottom

app: inbound: fuzzer: generalise fuzzers and solve coverage build #978

Merged
merged 5 commits into from
Apr 20, 2021
Merged

app: inbound: fuzzer: generalise fuzzers and solve coverage build #978

merged 5 commits into from
Apr 20, 2021

Conversation

DavidKorczynski
Copy link
Contributor

@DavidKorczynski DavidKorczynski commented Apr 16, 2021 •
edited
Loading

This PR is an effort to generalise the inbound fuzzer by allowing it to sending an arbitrary number of random packets.

This PR also moves the structs used by the structure-aware to be defined outside the fuzz_*.rs files. This solves the coverage compilation problems we currently have - as such, following this PR we will be able to get nice coverage reports shown by OSS-Fuzz.

Signed-off-by: davkor [email protected]

@DavidKorczynski DavidKorczynski requested a review from a team April 16, 2021 14:46
@DavidKorczynski
Copy link
Contributor Author

@hawkw you suggested also passing non utf8 strings, we could do this by creating our requests by way of the fuzz data as follows header_value = std::str::from_utf8_unchecked(&inp.header_value[..]), but this would mean we should wrap it in unsafe. Is this what you had in mind?

@DavidKorczynski
Copy link
Contributor Author

The CI looks to be complaining about something unrelated to the PR? https://github.com/linkerd/linkerd2-proxy/pull/978/checks?check_run_id=2363287950

@DavidKorczynski DavidKorczynski changed the title app: inbound: fuzzer: generalise fuzzers app: inbound: fuzzer: generalise fuzzers and solve coverage build Apr 20, 2021
olix0r
olix0r approved these changes Apr 20, 2021
View reviewed changes
Copy link
Member

@olix0r olix0r left a comment

Choose a reason for hiding this comment

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

lgtm

linkerd/app/inbound/fuzz/fuzz_targets/fuzz_target_1.rs Outdated Show resolved Hide resolved
linkerd/transport-header/fuzz/fuzz_targets/fuzz_target_structured.rs Outdated Show resolved Hide resolved
@olix0r olix0r requested a review from hawkw April 20, 2021 16:03
@hawkw
Copy link
Contributor

hawkw commented Apr 20, 2021

@hawkw you suggested also passing non utf8 strings, we could do this by creating our requests by way of the fuzz data as follows header_value = std::str::from_utf8_unchecked(&inp.header_value[..]), but this would mean we should wrap it in unsafe. Is this what you had in mind?

Yes, or using HeaderValue::from_maybe_shared_unchecked. The one concern with doing this is that we might end up testing the behavior of the hyper client used in the fuzz test when handling invalid header values, rather than the proxy's behavior --- we would want to make sure we were actually sending the invalid bytes to the test proxy, rather than just hitting an assertion in the test support code or something...

So, I think we can save this for a future change?

hawkw
hawkw approved these changes Apr 20, 2021
View reviewed changes
Copy link
Contributor

@hawkw hawkw left a comment

Choose a reason for hiding this comment

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

Overall, this looks good to me, although I had a couple of style nits that it would be good to address.

linkerd/app/inbound/src/http/mod.rs Outdated Show resolved Hide resolved
linkerd/app/inbound/src/http/mod.rs Outdated Show resolved Hide resolved
@olix0r olix0r merged commit 0eaf53d into linkerd:main Apr 20, 2021
olix0r added a commit to linkerd/linkerd2 that referenced this pull request Apr 21, 2021
@olix0r
proxy: v2.142.0
c52d07f 
This release primarily improves protocol detection error messages in the
admin server so that logs clearly indicate when the client expected a
different TLS server identity than that of the running proxy.

A number of internal improvements have been made, especially eliminating
some potential runtime panics detected by oss-fuzz. It is not expected
that these panics could be triggered in typical cluster configurations.

---

* discover: replace `linkerd-channel` with `tokio-util` `PollSender` (linkerd/linkerd2-proxy#969)
* replace `linkerd-channel` with `tokio-stream` (linkerd/linkerd2-proxy#970)
* concurrency-limit: use `tokio-util`'s `PollSemaphore` (linkerd/linkerd2-proxy#968)
* http: Do not fail fuzz tests when all IO is not read (linkerd/linkerd2-proxy#973)
* transport: Fix orig-dst compilation on non-Linux targets  (linkerd/linkerd2-proxy#974)
* Update trust-dns to fix possible panic (linkerd/linkerd2-proxy#975)
*  addr: fix `to_http_authority` panic with IPv6 (linkerd/linkerd2-proxy#976)
* outbound: skip logical stacks when no profile is discovered (linkerd/linkerd2-proxy#963)
* split: change traffic splits to require a profile (linkerd/linkerd2-proxy#964)
* inbound: only build profile route stacks when a profile is resolved (linkerd/linkerd2-proxy#966)
* profiles: make receiver param in `route_request` non-optional (linkerd/linkerd2-proxy#967)
* outbound: move target types into stack modules (linkerd/linkerd2-proxy#971)
* outbound: only build logical stacks for profiles with logical addrs (linkerd/linkerd2-proxy#972)
* app: inbound: add fuzzer (linkerd/linkerd2-proxy#977)
* admin: Fail connections when HTTP detection fails (linkerd/linkerd2-proxy#979)
* reduce error boilerplate with `thiserror` (linkerd/linkerd2-proxy#980)
* app: inbound: fuzzer: generalise fuzzers and solve coverage build (linkerd/linkerd2-proxy#978)
* admin: Assume meshed connections are HTTP/2 (linkerd/linkerd2-proxy#982)
* chore: Fix deprecations on nightly (linkerd/linkerd2-proxy#983)
@olix0r olix0r mentioned this pull request Apr 21, 2021
Closed
olix0r added a commit to linkerd/linkerd2 that referenced this pull request May 12, 2021
@olix0r
proxy: v2.143.0
0d6eada 
This release simplifies internals so that endpoint-forwarding logic is
completely distinct from handling of load balanced services.

The ingress-mode outbound proxy has been simplified to *require* the
`l5d-dst-override` header and to fail non-HTTP communication. This
ensures that the ingress-mode proxy does not unexpectedly revert to
insecure communication.

Finally, a regression was recently introduced that caused all proxy logs
to be output with ANSI control characters. Logs are now output in
plaintext by default

---

* discover: replace `linkerd-channel` with `tokio-util` `PollSender` (linkerd/linkerd2-proxy#969)
* replace `linkerd-channel` with `tokio-stream` (linkerd/linkerd2-proxy#970)
* concurrency-limit: use `tokio-util`'s `PollSemaphore` (linkerd/linkerd2-proxy#968)
* http: Do not fail fuzz tests when all IO is not read (linkerd/linkerd2-proxy#973)
* transport: Fix orig-dst compilation on non-Linux targets  (linkerd/linkerd2-proxy#974)
* Update trust-dns to fix possible panic (linkerd/linkerd2-proxy#975)
*  addr: fix `to_http_authority` panic with IPv6 (linkerd/linkerd2-proxy#976)
* outbound: skip logical stacks when no profile is discovered (linkerd/linkerd2-proxy#963)
* split: change traffic splits to require a profile (linkerd/linkerd2-proxy#964)
* inbound: only build profile route stacks when a profile is resolved (linkerd/linkerd2-proxy#966)
* profiles: make receiver param in `route_request` non-optional (linkerd/linkerd2-proxy#967)
* outbound: move target types into stack modules (linkerd/linkerd2-proxy#971)
* outbound: only build logical stacks for profiles with logical addrs (linkerd/linkerd2-proxy#972)
* app: inbound: add fuzzer (linkerd/linkerd2-proxy#977)
* admin: Fail connections when HTTP detection fails (linkerd/linkerd2-proxy#979)
* reduce error boilerplate with `thiserror` (linkerd/linkerd2-proxy#980)
* app: inbound: fuzzer: generalise fuzzers and solve coverage build (linkerd/linkerd2-proxy#978)
* admin: Assume meshed connections are HTTP/2 (linkerd/linkerd2-proxy#982)
* chore: Fix deprecations on nightly (linkerd/linkerd2-proxy#983)
* fuzz: Add logging to fuzz targets (linkerd/linkerd2-proxy#985)
* fuzz: don't panic when the proxy closes the conn (linkerd/linkerd2-proxy#986)
* Commit lock files for fuzzers (linkerd/linkerd2-proxy#984)
* fuzz: allow client requests to fail  (linkerd/linkerd2-proxy#989)
* tower: update dependency to 0.4.7 (linkerd/linkerd2-proxy#990)
* outbound: Make the Endpoint::logical_addr field optional (linkerd/linkerd2-proxy#991)
* trace: explicitly disable ANSI terminal colors (linkerd/linkerd2-proxy#994)
* ingress: Require the l5d-dst-override header (linkerd/linkerd2-proxy#992)
* outbound: Do not support TCP-forwarding in ingress-mode (linkerd/linkerd2-proxy#995)
* Decouple tcp forward stack from Endpoint target (linkerd/linkerd2-proxy#996)
* Pickup linkerd-await wrapper in docker build (linkerd/linkerd2-proxy#999)
* docs: Add fuzzing report (linkerd/linkerd2-proxy#1000)
* tests: Use io::Error in mocked connector (linkerd/linkerd2-proxy#1001)
* outbound: Decouple endpoint & logical stacks (linkerd/linkerd2-proxy#1002)
@olix0r olix0r mentioned this pull request May 12, 2021
Merged
olix0r added a commit to linkerd/linkerd2 that referenced this pull request May 18, 2021
@olix0r
proxy: v2.143.0 (#6107)
847b744 
This release simplifies internals so that endpoint-forwarding logic is
completely distinct from handling of load balanced services.

The ingress-mode outbound proxy has been simplified to *require* the
`l5d-dst-override` header and to fail non-HTTP communication. This
ensures that the ingress-mode proxy does not unexpectedly revert to
insecure communication.

Finally, a regression was recently introduced that caused all proxy logs
to be output with ANSI control characters. Logs are now output in
plaintext by default

---

* discover: replace `linkerd-channel` with `tokio-util` `PollSender` (linkerd/linkerd2-proxy#969)
* replace `linkerd-channel` with `tokio-stream` (linkerd/linkerd2-proxy#970)
* concurrency-limit: use `tokio-util`'s `PollSemaphore` (linkerd/linkerd2-proxy#968)
* http: Do not fail fuzz tests when all IO is not read (linkerd/linkerd2-proxy#973)
* transport: Fix orig-dst compilation on non-Linux targets  (linkerd/linkerd2-proxy#974)
* Update trust-dns to fix possible panic (linkerd/linkerd2-proxy#975)
*  addr: fix `to_http_authority` panic with IPv6 (linkerd/linkerd2-proxy#976)
* outbound: skip logical stacks when no profile is discovered (linkerd/linkerd2-proxy#963)
* split: change traffic splits to require a profile (linkerd/linkerd2-proxy#964)
* inbound: only build profile route stacks when a profile is resolved (linkerd/linkerd2-proxy#966)
* profiles: make receiver param in `route_request` non-optional (linkerd/linkerd2-proxy#967)
* outbound: move target types into stack modules (linkerd/linkerd2-proxy#971)
* outbound: only build logical stacks for profiles with logical addrs (linkerd/linkerd2-proxy#972)
* app: inbound: add fuzzer (linkerd/linkerd2-proxy#977)
* admin: Fail connections when HTTP detection fails (linkerd/linkerd2-proxy#979)
* reduce error boilerplate with `thiserror` (linkerd/linkerd2-proxy#980)
* app: inbound: fuzzer: generalise fuzzers and solve coverage build (linkerd/linkerd2-proxy#978)
* admin: Assume meshed connections are HTTP/2 (linkerd/linkerd2-proxy#982)
* chore: Fix deprecations on nightly (linkerd/linkerd2-proxy#983)
* fuzz: Add logging to fuzz targets (linkerd/linkerd2-proxy#985)
* fuzz: don't panic when the proxy closes the conn (linkerd/linkerd2-proxy#986)
* Commit lock files for fuzzers (linkerd/linkerd2-proxy#984)
* fuzz: allow client requests to fail  (linkerd/linkerd2-proxy#989)
* tower: update dependency to 0.4.7 (linkerd/linkerd2-proxy#990)
* outbound: Make the Endpoint::logical_addr field optional (linkerd/linkerd2-proxy#991)
* trace: explicitly disable ANSI terminal colors (linkerd/linkerd2-proxy#994)
* ingress: Require the l5d-dst-override header (linkerd/linkerd2-proxy#992)
* outbound: Do not support TCP-forwarding in ingress-mode (linkerd/linkerd2-proxy#995)
* Decouple tcp forward stack from Endpoint target (linkerd/linkerd2-proxy#996)
* Pickup linkerd-await wrapper in docker build (linkerd/linkerd2-proxy#999)
* docs: Add fuzzing report (linkerd/linkerd2-proxy#1000)
* tests: Use io::Error in mocked connector (linkerd/linkerd2-proxy#1001)
* outbound: Decouple endpoint & logical stacks (linkerd/linkerd2-proxy#1002)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hawkw hawkw hawkw approved these changes

@olix0r olix0r olix0r approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@DavidKorczynski @hawkw @olix0r