tkhq / qos

QuorumOS is a computation layer for running applications inside Trusted Execution Environments (TEEs)
GNU Affero General Public License v3.0
5 stars 2 forks source link

SOC2: Updates tokio and its sub deps: libc #473

Closed taylorjdawson closed 4 months ago

taylorjdawson commented 4 months ago

Summary & Motivation (Problem vs. Solution)

How I Tested These Changes

Pre merge check list

cr-tk commented 4 months ago

Also, the E2E tests seem to fail:

running 1 test
Manifest written to: /tmp/boot-e2e/boot-dir/manifest
`SocketServer` listening on Unix(UnixAddr { sun: sockaddr_un { sun_family: 1 }, sun_len: 29 })
thread '<unnamed>' panicked at qos_core/src/reaper.rs:53:51:
called `Result::unwrap()` on an `Err` value: IOError(NixError(EINVAL))
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
HostServer listening on 127.0.0.1:30766
Manifest Envelope written to: /tmp/boot-e2e/boot-dir/manifest_envelope
Error while trying to send request over socket to enclave: IOError(ConnectNixError(ECONNREFUSED))thread 'main' panicked at qos_client/src/cli/services.rs:1123:54:
called `Result::unwrap()` on an `Err` value: "http_post error: [url: http://127.0.0.1:30766/qos/message, status: 500, body: Ok(\"\\0\\n\")]"
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
test standard_boot_e2e ... FAILED
cr-tk commented 4 months ago

@taylorjdawson after the force-pushed change down to libc 0.2.149, the tests don't fail anymore and your additional code changes aren't needed (yet), right? That's definitely a good argument to stay with the lower version compared to -155, let's tackle the failures another time :+1: :slightly_smiling_face:

cr-tk commented 4 months ago

@taylorjdawson I just re-ran my (experimental, local) cargo crate diffing tool and the set of changes is now much smaller:

libc,0.2.148,0.2.149,,
mio,0.8.6|0.8.8,0.8.11,,
socket2,0.4.9|0.5.4,0.5.5,,
tokio,1.28.0|1.33.0,1.38.0,,
tokio-macros,2.1.0,2.3.0,,

This is great for the review-ability of this PR.

Notably, nix is now no longer getting changed, but if that wasn't necessary in itself (to fix a known vulnerability), that's not a problem.

cr-tk commented 4 months ago

I'm planning to review the dependency changes tomorrow, and expect that we can then approve and merge this PR (once one of the people with special qos-operator review permission signed off on it).

cr-tk commented 4 months ago

@taylorjdawson do you want to merge this (via the CLI), or should I do it?