Implement proxy v2 architecture, in Rust

[legoktm]

This was migrated from https://github.com/freedomofpress/securedrop-proxy/pull/127

Status

Ready for review

Description

Implement the proxy v2 architecture; see individual commits messages for more detail.

• Fixes https://github.com/freedomofpress/securedrop-client/issues/1678.
• Fixes #1778
• Fixes https://github.com/freedomofpress/securedrop-workstation/issues/853
• Refs https://github.com/freedomofpress/securedrop-client/issues/1848
• Fixes https://github.com/freedomofpress/securedrop-client/issues/1831
• Fixes https://github.com/freedomofpress/securedrop-client/issues/1681
• Fixes https://github.com/freedomofpress/securedrop-client/issues/1684
• Fixes https://github.com/freedomofpress/securedrop-client/issues/1685
• Fixes https://github.com/freedomofpress/securedrop-client/issues/1690
• Fixes https://github.com/freedomofpress/securedrop-client/issues/1694
• Fixes https://github.com/freedomofpress/securedrop-client/issues/1717
• Fixes https://github.com/freedomofpress/securedrop-client/issues/1761
• Fixes https://github.com/freedomofpress/securedrop-security/issues/73

Test Plan

• [x] CI passes
• [ ] Visual review:
  • [x] @cfm reviews commit by commit
  • [ ] @legoktm reviews commit by commit
  • [ ] Third review (TBD) reviews the major commits specially, since @cfm and @legoktm co-authored most everything here:
    • [ ] df803d5ffcf638d4c6e027684d4255f420868100
    • [ ] 1f3c2c84466f350954531d2d60d066f5431d69fc
    • [ ] bc67129b44d76581ae29780ee5a5be12ac907ebe
  • [ ] Third reviewer (TBD) spot-checks VCR cassettes regenerated in:
    • [ ] 1f3c2c84466f350954531d2d60d066f5431d69fc
    • [ ] 98867666c6867f7711f04de2777b0661282700c6
• [x] Start dev instance with cd client && ./run.sh and test syncing, sending replies and downloading attachments works fine
• [ ] Build and install debs onto Qubes 4.2 system, test syncing, sending replies and downloading attachments works fine

Checklist

If these changes modify code paths involving cryptography, the opening of files in VMs or network (via the RPC service) traffic, Qubes testing in the staging environment is required. For fine tuning of the graphical user interface, testing in any environment in Qubes is required. Please check as applicable:

• [ ] I have tested these changes in the appropriate Qubes environment
• I do not have an appropriate Qubes OS workstation set up (the reviewer will need to test these changes)
• These changes should not need testing in Qubes

If these changes add or remove files other than client code, the AppArmor profile may need to be updated. Please check as applicable:

• I have updated the AppArmor profile
• No update to the AppArmor profile is required for these changes
• I don't know and would appreciate guidance
• [x] AppArmor will be done as a follow-up: https://github.com/freedomofpress/securedrop-client/issues/1889

If these changes modify the database schema, you should include a database migration. Please check as applicable:

• I have written a migration and upgraded a test database based on main and confirmed that the migration is self-contained and applies cleanly
• I have written a migration but have not upgraded a test database based on main and would like the reviewer to do so
• I need help writing a database migration
• [x] No database schema changes are needed

[legoktm]

I accidentally squashed the README commits when migrating and then dropped the CircleCI config since we need to redo it for GHA anyways.

[legoktm]

I just pushed "WIP: Have the sdk always shell out to the proxy" which is somewhat working, I messed up in negating a boolean somewhere so file downloading is a bit messed up, but the login and other API requests are happening via shelling out to a locally built proxy. Once that's fixed, the next step will be mocking HTTP traffic in tests, maybe with some vcr-lite system.

[cfm]

I'll pick this up from @legoktm after today, starting with the to-do list I've added in https://github.com/freedomofpress/securedrop-client/pull/1718#issue-2040487871. (Feel free to edit and add to it!)

[legoktm]

A bit of an update and what's left:

• [x] @cfm and I discussed that we're good with outputting headers on stderr in streaming mode, except that it should be all headers, and the proxy should not hardcode specific headers.
• [x] I fixed whatever subprocess issue was causing problems and now the client is able to use the proxy in development mode when started with the ./run.sh script.
• [x] Streaming is actually streaming now! Had to switch to async reqwest but the code flow should be relatively straightforward.
• [x] Tests need fixing, they're currently using vcr. We either need to reimplement vcr for our mock format or mock the proxying to use requests and then keep using vcr.
• [x] Also the proxy code needs a test that actually verifies streaming. Should be doable with the e.g. /drip?duration=5&numbytes=20&code=200&delay=0 endpoint
• [x] Packaging needs to run cargo build --release and install it; also need to bootstrap the Rust compiler into the container.
• [x] It would be good to clean up the git history with a rebase, we could try to split this into other PRs, e.g. I think we could maybe merge some of the cargo-vet infrastructure ahead of time?

[legoktm]

Oh also, it might be worth looking at the SDK error handling a bit closer, there's a few raised errors that I left with "FIXME", but also I saw at least one higher-level method that was directly catching a JSON error, which shouldn't be possible since _send_json_request() should already catch it.

[cfm]

https://github.com/freedomofpress/securedrop-client/pull/1718#issuecomment-1899482791 is done as suggested. Stack of work is now:

• https://github.com/freedomofpress/securedrop-client/pull/1718#issuecomment-1899199097
• https://github.com/freedomofpress/securedrop-client/pull/1718#issue-2040487871

[cfm]

https://github.com/freedomofpress/securedrop-client/pull/1718#issuecomment-1899199097:

• Tests need fixing, they're currently using vcr. We either need to reimplement vcr for our mock format or mock the proxying to use requests and then keep using vcr.

Tracking this in #1778.

[cfm]

#1778 is substantively done in f3a5c5a323177a1af89ea398fd5a8fe5fc508df4...b82720b70b04563cc3ebf9a490a3f6dc78fabcaa and just needs some documenting and Git-polishing.

[cfm]

#1778 is done and documented in f3a5c5a323177a1af89ea398fd5a8fe5fc508df4...b002a5f930216fed081af1d29c87e50138699a1d. @legoktm, let's sync up about this and the remaining checklist here tomorrow.

[legoktm]

I just rebased and squashed all the commits prior to the SDK/client test work and attempted to write longer commit message for the resulting commits and in a few cases moved stuff from commit messages to inline comments. I might have broken something given the number of CI failures, will investigate tomorrow when I finish rebasing the rest of the stack.

[legoktm]

client functional tests are failing because AFAICT those are still using the old VCR format. Will look at converting those tomorrow. We also need to add a Rust compiler to the proxy test job, I would rather do that after the CircleCI -> GHA PR lands to avoid conflicting with that.

[legoktm]

I've basically gotten the infrastructure for using the custom VCRAPI and Cassettes in the functional tests, but I'm debugging an issue in which the cassette seems to be re-recording its own playbacks. If I run make test-sdk, it is writing new requests to the YAML files, except I don't have a server running, so it's just appending its own prior recordings to the cassette?

[cfm]

You know, @legoktm, I'd thought the diff was getting a little large! This should be fixed in 2cd386af4ed6863372b07d04f1455c84ade8db9a, originally a one-line change with a resulting diffstat of 6306 insertions(+), 20765 deletions(-)....

[legoktm]

CI is now passing except the cargo vet (because we still need to vet the dependencies) \o/ I've also squashed it down to 11 commits.

Remaining steps:

• [ ] Documentation updates/tweaks
• [ ] cargo vet remaining Rust deps
• [ ] whatever integration of qubesdb we end up pursuing, c.f. https://github.com/freedomofpress/securedrop-workstation/issues/936#issuecomment-1970301425

[cfm]

@legoktm in https://github.com/freedomofpress/securedrop-client/pull/1718#issuecomment-1972248990:

Remaining steps:

• Documentation updates/tweaks

9c9a65e3302ccd6340f3d664431eae38493bd149 proposes a minimal documentation fixup for 817fec96ac530ef6d61a3f977b5b1b0466bf14e9. Let me know if there's more you'd like to have here, @legoktm.

[cfm]

Bilateral review in progress: I've read through everything and have review comments pending. Most of them are just "We should add a docstring here" or "We should rustdoc this", so rather than post those and have to follow up on them individually I'll just take this afternoon to turn them into suggestions.

[cfm]

• [x] Start dev instance with cd client && ./run.sh and test syncing, sending replies and downloading attachments works fine

Works fine, but led to #1996, with some follow-up documentation forthcoming.

[cfm]

430bd8427f10adde5e6f958ee42f9cdafacd1418 adds the diagram I found myself needing while tracing the new proxy in a development environment. The rest of the readme stands as it is, since securedrop-proxy is now always used, even with the origin is just localhost via make dev.

freedomofpress/securedrop-dev-docs#156 is just for establishing that a Rust toolchain is now a prerequisite for running the Client in development.

[legoktm]

Thanks, I approved all of your documentation improvements (might amend to break long lines if needed) - will review the rest shortly.

[legoktm]

For some reason the existing cassettes are no longer sufficient and therefore it's trying to invoke the proxy, but CI intentionally doesn't have a Rust toolchain, so it fails. So the question is why did the most recent set of changes affect the underlying requests made like this :/

[cfm]

@legoktm, I haven't bisected to confirm this hypothesis, but it occurs to me that c6a13d0ee428aa489ab14ca62a04a4233f9d5c33 changes the version of werkzeug, which appears in (e.g.):

https://github.com/freedomofpress/securedrop-client/blob/a93ca77dbd77ce9a2c039d4847911ca09c3205df/client/tests/functional/data/test_delete_source.yml#L15-L20

Might as well amend make regenerate-sdk-cassettes into c6a13d0ee428aa489ab14ca62a04a4233f9d5c33 and consider it more evidence in support of #1992.

[legoktm]

@legoktm, I haven't bisected to confirm this hypothesis, but it occurs to me that c6a13d0 changes the version of werkzeug, which appears in (e.g.):

I don't think it's that because you (correctly) ran poetry lock --no-update, so werkzeug is still on the same version, it's just no longer pinned via the pyproject.toml. Also that's for the proxy tests, not client. Let me try a proper bisection...

[legoktm]

I bisected to 18e19c3b and the solution is a bit embarrassing: ~/Downloads doesn't exist in the CI container. Now, ~/QubesIncoming didn't either, but we weren't actually checking that one existed. Fix incoming.

The reason it appeared to have something to do with the cassettes was because the request would fail, it would be caught, then the sync tries again, and the new request isn't in the VCR, so it fires off a real request. I think.

[legoktm]

Rebased with minimal squashing, CI is passing now, so what's left is:

• visual review from 3rd person
• verification testing in Qubes dev env (now that apparmor/Downloads are fixed)

[legoktm]

[ ] Build and install debs onto Qubes 4.2 system, test syncing, sending replies and downloading attachments works fine

I've verified this was working and ticked it off. @micahflee will be the third reviewer and look at the commits listed in the PR description.