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

Crash using wasm-filter #13857

Closed
eloycoto opened this issue Nov 2, 2020 · 11 comments · Fixed by #13930
Closed

Crash using wasm-filter #13857

eloycoto opened this issue Nov 2, 2020 · 11 comments · Fixed by #13930
Assignees

Comments

@eloycoto
Copy link

eloycoto commented Nov 2, 2020

Description:

Working with a wasm filter, when you return 1 on proxy_on_request_headers and at the same time, you send back a 403 message, Envoy crashes with the following stacktrace.

proxy_1  | [2020-10-30 21:23:48.275][32][critical][backtrace] [bazel-out/k8-opt/bin/source/server/_virtual_includes/backtrace_lib/server/backtrace.h:104] Caught Segmentation fault, suspect faulting address 0x0
proxy_1  | [2020-10-30 21:23:48.275][32][critical][backtrace] [bazel-out/k8-opt/bin/source/server/_virtual_includes/backtrace_lib/server/backtrace.h:91] Backtrace (use tools/stack_decode.py to get line numbers):
proxy_1  | [2020-10-30 21:23:48.275][32][critical][backtrace] [bazel-out/k8-opt/bin/source/server/_virtual_includes/backtrace_lib/server/backtrace.h:92] Envoy version: 3a32d23c7c361b6ffd5860a707af8957326b2b17/1.17.0-dev/Clean/RELEASE/BoringSSL
proxy_1  | [2020-10-30 21:23:48.276][32][critical][backtrace] [bazel-out/k8-opt/bin/source/server/_virtual_includes/backtrace_lib/server/backtrace.h:96] #0: __restore_rt [0x7f7a678188a0]->[0x29a36a9a88a0] ??:0
proxy_1  | [2020-10-30 21:23:48.314][32][critical][backtrace] [bazel-out/k8-opt/bin/source/server/_virtual_includes/backtrace_lib/server/backtrace.h:96] #1: Envoy::ConnectionPool::ConnPoolImplBase::attachStreamToClient() [0x55d6fe79cd29]->[0x192cd29] bazel-out/k8-opt/bin/source/extensions/filters/network/kafka/_virtual_includes/serialization_lib/extensions/filters/network/kafka/serialization.h:455
proxy_1  | [2020-10-30 21:23:48.329][32][critical][backtrace] [bazel-out/k8-opt/bin/source/server/_virtual_includes/backtrace_lib/server/backtrace.h:96] #2: Envoy::ConnectionPool::ConnPoolImplBase::onUpstreamReady() [0x55d6fe79d669]->[0x192d669] /opt/llvm/bin/../include/c++/v1/memory:3483
proxy_1  | [2020-10-30 21:23:48.340][32][critical][backtrace] [bazel-out/k8-opt/bin/source/server/_virtual_includes/backtrace_lib/server/backtrace.h:96] #3: Envoy::ConnectionPool::ConnPoolImplBase::onConnectionEvent() [0x55d6fe79e4e0]->[0x192e4e0] /opt/llvm/bin/../include/c++/v1/vector:1635
proxy_1  | [2020-10-30 21:23:48.351][32][critical][backtrace] [bazel-out/k8-opt/bin/source/server/_virtual_includes/backtrace_lib/server/backtrace.h:96] #4: Envoy::Network::ConnectionImplBase::raiseConnectionEvent() [0x55d6fe630e3b]->[0x17c0e3b] /opt/llvm/bin/../include/c++/v1/memory:1876
proxy_1  | [2020-10-30 21:23:48.362][32][critical][backtrace] [bazel-out/k8-opt/bin/source/server/_virtual_includes/backtrace_lib/server/backtrace.h:96] #5: Envoy::Network::ConnectionImpl::raiseEvent() [0x55d6fe6299f9]->[0x17b99f9] /opt/llvm/bin/../include/c++/v1/vector:1540
proxy_1  | [2020-10-30 21:23:48.372][32][critical][backtrace] [bazel-out/k8-opt/bin/source/server/_virtual_includes/backtrace_lib/server/backtrace.h:96] #6: Envoy::Network::ConnectionImpl::onWriteReady() [0x55d6fe62bc6c]->[0x17bbc6c] bazel-out/k8-opt/bin/source/extensions/filters/network/kafka/_virtual_includes/tagged_fields_lib/extensions/filters/network/kafka/tagged_fields.h:101
proxy_1  | [2020-10-30 21:23:48.382][32][critical][backtrace] [bazel-out/k8-opt/bin/source/server/_virtual_includes/backtrace_lib/server/backtrace.h:96] #7: Envoy::Network::ConnectionImpl::onFileEvent() [0x55d6fe62ad09]->[0x17bad09] external/com_google_absl/absl/strings/str_cat.h:217
proxy_1  | [2020-10-30 21:23:48.392][32][critical][backtrace] [bazel-out/k8-opt/bin/source/server/_virtual_includes/backtrace_lib/server/backtrace.h:96] #8: Envoy::Event::FileEventImpl::assignEvents()::$_1::__invoke() [0x55d6fe620f66]->[0x17b0f66] bazel-out/k8-opt/bin/source/extensions/filters/network/kafka/_virtual_includes/kafka_request_parser_lib/extensions/filters/network/kafka/kafka_request_parser.h:188
proxy_1  | [2020-10-30 21:23:48.402][32][critical][backtrace] [bazel-out/k8-opt/bin/source/server/_virtual_includes/backtrace_lib/server/backtrace.h:96] #9: event_process_active_single_queue [0x55d6fea57128]->[0x1be7128] /opt/llvm/bin/../include/c++/v1/memory:3028
proxy_1  | [2020-10-30 21:23:48.412][32][critical][backtrace] [bazel-out/k8-opt/bin/source/server/_virtual_includes/backtrace_lib/server/backtrace.h:96] #10: event_base_loop [0x55d6fea55afe]->[0x1be5afe] /opt/llvm/bin/../include/c++/v1/memory:4056
proxy_1  | [2020-10-30 21:23:48.423][32][critical][backtrace] [bazel-out/k8-opt/bin/source/server/_virtual_includes/backtrace_lib/server/backtrace.h:96] #11: Envoy::Server::WorkerImpl::threadRoutine() [0x55d6fe612068]->[0x17a2068] /usr/include/x86_64-linux-gnu/bits/string_fortified.h:34
proxy_1  | [2020-10-30 21:23:48.432][32][critical][backtrace] [bazel-out/k8-opt/bin/source/server/_virtual_includes/backtrace_lib/server/backtrace.h:96] #12: Envoy::Thread::ThreadImplPosix::ThreadImplPosix()::{lambda()#1}::__invoke() [0x55d6fec06d13]->[0x1d96d13] /opt/llvm/bin/../include/c++/v1/__tree:1834
proxy_1  | [2020-10-30 21:23:48.432][32][critical][backtrace] [bazel-out/k8-opt/bin/source/server/_virtual_includes/backtrace_lib/server/backtrace.h:96] #13: start_thread [0x7f7a6780d6db]->[0x29a36a99d6db] ??:0

Repro:

git clone https://github.com/eloycoto/envoy_playground.git
cd envoy_dump
docker-compose up
curl http://127.0.0.1:18000

Config:

static_resources:
  listeners:
  - name: main
    address:
      socket_address:
        address: 0.0.0.0
        port_value: 80
    filter_chains:
    - filters:
      - name: envoy.http_connection_manager
        config:
          stat_prefix: ingress_http
          codec_type: auto
          route_config:
            name: local_route
            virtual_hosts:
            - name: local_service
              domains:
              - "*"
              routes:
              - match:
                  prefix: "/"
                route:
                  cluster: httpbin
          http_filters:
          - name: envoy.filters.http.wasm
            config:
              config:
                name: "my_plugin"
                root_id: "add_header"
                vm_config:
                  vm_id: "my_vm_id"
                  runtime: "envoy.wasm.runtime.v8"
                  code:
                    local:
                      filename: "/opt/filter.wasm"
                  allow_precompiled: true
          - name: envoy.router
            config: {}
  clusters:
  - name: httpbin
    connect_timeout: 1s
    type: logical_dns
    lb_policy: round_robin
    hosts:
    - socket_address:
        address: httpbin.org
        port_value: 80
admin:
  access_log_path: "/dev/null"
  address:
    socket_address:
      address: 0.0.0.0
      port_value: 8001

More info:

proxy-wasm/proxy-wasm-rust-sdk#44

Regards

@eloycoto eloycoto added bug triage Issue requires triage labels Nov 2, 2020
@PiotrSikora PiotrSikora added area/wasm and removed triage Issue requires triage labels Nov 2, 2020
@PiotrSikora
Copy link
Contributor

Envoy crashes when the processing is not stopped after sending local reply.

@PiotrSikora
Copy link
Contributor

@mathetake is this something that you want to take a look at?

@mathetake
Copy link
Member

Will do tomorrow(hopefully)

@PiotrSikora
Copy link
Contributor

@mathetake Thanks! Note that this is triggered by an invalid code deployed in Envoy, so it's not a top priority.

@antoniovicente
Copy link
Contributor

It sounds like you're pursuing a fix in the extension defined in proxy-wasm. Should we also consider some fixes to envoy core components to project about similar crashes caused by hard-to-avoid mistakes in extension implementations?

@PiotrSikora
Copy link
Contributor

I don't think there is any value in masking programming errors in native extensions. Wasm and ext_proc are a bit different, since we're expected to run untrusted/hostile third-party code in not too distant future, so we need to protect against anything that could crash the proxy.

@antoniovicente
Copy link
Contributor

cc @asraa

@asraa
Copy link
Contributor

asraa commented Nov 10, 2020

so we need to protect against anything that could crash the proxy.

I agree, I think core code should guard against continuation after a local reply is sent. This seems like the same error @rgs1 encountered with a custom filter (#13737).

Yan and I worked on guarding against filter misbehavior in the context of header removal (#13756), but not around additional code guards around filter state.

istio-testing pushed a commit to istio/envoy that referenced this issue Nov 20, 2020
* build: update rules_rust to allow Rustc in RBE (envoyproxy#13595)

Signed-off-by: Lizan Zhou <[email protected]>
Signed-off-by: Piotr Sikora <[email protected]>

* fix macos v8 build (envoyproxy#13572)

Signed-off-by: Rama Chavali <[email protected]>

* wasm: update proxy-wasm-cpp-host (envoyproxy#13606)

The PR updates proxy-wasm-cpp-host dependency for enhancing the capability and fixing a bug in WASM extensions.

The change consists of three PRs in proxy-wasm-cpp-host:

1. proxy-wasm/proxy-wasm-cpp-host#68 @PiotrSikora
2. proxy-wasm/proxy-wasm-cpp-host#65 @mathetake (me)
3. proxy-wasm/proxy-wasm-cpp-host#64 @mathetake (me)

The code change can be found at proxy-wasm/proxy-wasm-cpp-host@49ed20e...c5658d3 .

1 & 2 enhance WASM capability, and 3 fixes a bug in situations where users share vm_id for multiple filters. For details, please take a look at these original PRs.

Signed-off-by: mathetake <[email protected]>
Signed-off-by: Piotr Sikora <[email protected]>

* wasm: re-enable tests with precompiled modules. (envoyproxy#13583)

Fixes envoyproxy#12335.

Signed-off-by: Piotr Sikora <[email protected]>

* wasm: flip the meaning of the "repository" in envoy_wasm_cc_binary(). (envoyproxy#13621)

Change the meaning of the "repository" parameter to refer to an external
Bazel repository, instead of using "@envoy" in targets that are included
in the Envoy repository.

This aligns with other envoy_* rules.

Signed-off-by: Piotr Sikora <[email protected]>

* build: support ppc64le with wasm (envoyproxy#13657)

The build has only been tested with gn git sha 5da62d5 as
recommended by ppc64 maintainers of the v8 runtime.

Signed-off-by: Christopher M. Luciano <[email protected]>

* wasm: remove no longer needed Emscripten metadata. (envoyproxy#13667)

Signed-off-by: Piotr Sikora <[email protected]>

* fix wasm compilation (envoyproxy#13765)

Signed-off-by: Asra Ali <[email protected]>

* wasm: strip only Custom Sections with precompiled Wasm modules. (envoyproxy#13775)

Signed-off-by: Piotr Sikora <[email protected]>

* build: don't build shared libraries for zlib and zlib-ng. (envoyproxy#13652)

Signed-off-by: Piotr Sikora <[email protected]>

* wasm: update V8 to v8.7.220.10. (envoyproxy#13568)

Signed-off-by: Piotr Sikora <[email protected]>

* build: exclude wee8/out from inputs (envoyproxy#13866)

If you build without sandboxing for performance, the output files from
this custom build genrule contained timestamps which caused it to
rebuild every single build.

Signed-off-by: Keith Smiley <[email protected]>

* tls: fix detection of the upstream connection close event. (envoyproxy#13858)

Fixes envoyproxy#13856.

Signed-off-by: Piotr Sikora <[email protected]>

* wasm: Force stop iteration after local response is sent (envoyproxy#13930)

Resolves envoyproxy#13857

ref:
-proxy-wasm/proxy-wasm-rust-sdk#44
-proxy-wasm/proxy-wasm-cpp-host#88
-proxy-wasm/proxy-wasm-cpp-host#93

Signed-off-by: mathetake <[email protected]>
Signed-off-by: Piotr Sikora <[email protected]>

* wasm: fix order of callbacks for paused requests. (envoyproxy#13840)

Fixes proxy-wasm/proxy-wasm-rust-sdk#43.

Signed-off-by: Piotr Sikora <[email protected]>

* wasm: fix network leak (envoyproxy#13836)

Signed-off-by: Kuat Yessenov <[email protected]>

Co-authored-by: Lizan Zhou <[email protected]>
Co-authored-by: Rama Chavali <[email protected]>
Co-authored-by: Takeshi Yoneda <[email protected]>
Co-authored-by: cmluciano <[email protected]>
Co-authored-by: asraa <[email protected]>
Co-authored-by: Keith Smiley <[email protected]>
Co-authored-by: Takeshi Yoneda <[email protected]>
Co-authored-by: Kuat <[email protected]>
@antoniovicente
Copy link
Contributor

cc @asraa @htuch

Should we file an issue requesting the implementation of safety nets in envoy core to protect against this kind of issue?

@asraa
Copy link
Contributor

asraa commented Nov 23, 2020

I think that is this issue #13737 number (2), but right now I'm not sure who the owner is. @rgs1 are you thinking of adding protections against continuing after end stream/direct response? I may have bandwidth to give it a stab, but want to check.

@htuch
Copy link
Member

htuch commented Nov 24, 2020

I think anything we can do to make it easier for filters to do the right things and hard to do the wrong there here makes sense. Agree with @asraa that this is in scope plausibly for the error handling policy implementation, providing it's not already being worked on.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants