Author: Jason White [github@jasonwhite.io]
Hash: f8e90f774dd743e5ec3a17df26d0b23af47cd87a
Timestamp: Fri, 25 Jun 2021 05:16:33 +0000 (3 years ago)

+151 -3 +/-7 browse
Unbreak the S3 backend 
Unbreak the S3 backend

There was a check missing for `upload_url` causing all uploads to bypass
the encryption process. Then, when the same object gets downloaded and
decrypted, the SHA256 verification fails causing an infinite download
loop.

This also adds a test to help catch issues like this in the future.
1diff --git a/Cargo.lock b/Cargo.lock
2index 9d03408..58b9d79 100644
3--- a/Cargo.lock
4+++ b/Cargo.lock
5 @@ -1224,6 +1224,7 @@ dependencies = [
6 "tempfile",
7 "tokio",
8 "tokio-util",
9+ "toml",
10 "url",
11 "uuid",
12 ]
13 diff --git a/Cargo.toml b/Cargo.toml
14index 8d53523..d4f1a6a 100644
15--- a/Cargo.toml
16+++ b/Cargo.toml
17 @@ -48,6 +48,7 @@ rand = "0.8"
18 tempfile = "3"
19 duct = "0.13"
20 env_logger = "0.8"
21+ toml = "0.5"
22
23 [dependencies.rusoto_core]
24 version = "0.46"
25 diff --git a/src/lib.rs b/src/lib.rs
26index df3ea7f..2b53bd1 100644
27--- a/src/lib.rs
28+++ b/src/lib.rs
29 @@ -132,11 +132,26 @@ impl S3ServerBuilder {
30 /// Spawns the server. The server must be awaited on in order to accept
31 /// incoming client connections and run.
32 pub async fn spawn(
33- self,
34+ mut self,
35 addr: SocketAddr,
36- ) -> Result<Box<dyn Server + Unpin>, Box<dyn std::error::Error>> {
37+ ) -> Result<Box<dyn Server + Unpin + Send>, Box<dyn std::error::Error>>
38+ {
39 let prefix = self.prefix.unwrap_or_else(|| String::from("lfs"));
40
41+ if self.cdn.is_some() {
42+ log::warn!(
43+ "A CDN was specified. Since uploads and downloads do not flow \
44+ through Rudolfs in this case, they will *not* be encrypted."
45+ );
46+
47+ if let Some(_) = self.cache.take() {
48+ log::warn!(
49+ "A local disk cache does not work with a CDN and will be \
50+ disabled."
51+ );
52+ }
53+ }
54+
55 let s3 = S3::new(self.bucket, prefix, self.cdn)
56 .map_err(Error::from)
57 .await?;
58 diff --git a/src/storage/s3.rs b/src/storage/s3.rs
59index 668b468..abc9f81 100644
60--- a/src/storage/s3.rs
61+++ b/src/storage/s3.rs
62 @@ -347,6 +347,12 @@ where
63 key: &StorageKey,
64 expires_in: Duration,
65 ) -> Option<String> {
66+ // Don't use a presigned URL if we're not using a CDN. Otherwise,
67+ // uploads will bypass the encryption process and fail to download.
68+ if self.cdn.is_none() {
69+ return None;
70+ }
71+
72 let request = PutObjectRequest {
73 bucket: self.bucket.clone(),
74 key: self.key_to_path(&key),
75 diff --git a/tests/.gitignore b/tests/.gitignore
76new file mode 100644
77index 0000000..925af94
78--- /dev/null
79+++ b/tests/.gitignore
80 @@ -0,0 +1 @@
81+ .*.toml
82 diff --git a/tests/test_local.rs b/tests/test_local.rs
83index 58e9c39..2af9b6f 100644
84--- a/tests/test_local.rs
85+++ b/tests/test_local.rs
86 @@ -32,7 +32,7 @@ use tokio::sync::oneshot;
87 use common::{init_logger, GitRepo};
88
89 #[tokio::test(flavor = "multi_thread")]
90- async fn smoke_test() -> Result<(), Box<dyn std::error::Error>> {
91+ async fn local_smoke_test() -> Result<(), Box<dyn std::error::Error>> {
92 init_logger();
93
94 // Make sure our seed is deterministic. This makes it easier to reproduce
95 diff --git a/tests/test_s3.rs b/tests/test_s3.rs
96new file mode 100644
97index 0000000..c4c2e71
98--- /dev/null
99+++ b/tests/test_s3.rs
100 @@ -0,0 +1,124 @@
101+ // Copyright (c) 2021 Jason White
102+ //
103+ // Permission is hereby granted, free of charge, to any person obtaining a copy
104+ // of this software and associated documentation files (the "Software"), to deal
105+ // in the Software without restriction, including without limitation the rights
106+ // to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
107+ // copies of the Software, and to permit persons to whom the Software is
108+ // furnished to do so, subject to the following conditions:
109+ //
110+ // The above copyright notice and this permission notice shall be included in
111+ // all copies or substantial portions of the Software.
112+ //
113+ // THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
114+ // IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
115+ // FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
116+ // AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
117+ // LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
118+ // OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
119+ // SOFTWARE.
120+
121+ //! This integration test only runs if the file `.test_credentials.toml` is in
122+ //! the same directory. Otherwise, it succeeds and does nothing.
123+ //!
124+ //! To run this test, create `tests/.test_credentials.toml` with the following
125+ //! contents:
126+ //!
127+ //! ```toml
128+ //! access_key_id = "XXXXXXXXXXXXXXXXXXXX"
129+ //! secret_access_key = "XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX"
130+ //! default_region = "us-east-1"
131+ //! bucket = "my-test-bucket"
132+ //! ```
133+ //!
134+ //! Be sure to *only* use non-production credentials for testing purposes. We
135+ //! intentionally do not load the credentials from the environment to avoid
136+ //! clobbering any existing S3 bucket.
137+
138+ mod common;
139+
140+ use std::fs;
141+ use std::net::SocketAddr;
142+ use std::path::Path;
143+
144+ use futures::future::Either;
145+ use rand::rngs::StdRng;
146+ use rand::Rng;
147+ use rand::SeedableRng;
148+ use rudolfs::S3ServerBuilder;
149+ use serde::{Deserialize, Serialize};
150+ use tokio::sync::oneshot;
151+
152+ use common::{init_logger, GitRepo};
153+
154+ #[derive(Debug, Serialize, Deserialize)]
155+ struct Credentials {
156+ access_key_id: String,
157+ secret_access_key: String,
158+ default_region: String,
159+ bucket: String,
160+ }
161+
162+ #[tokio::test(flavor = "multi_thread")]
163+ async fn s3_smoke_test() -> Result<(), Box<dyn std::error::Error>> {
164+ init_logger();
165+
166+ let config = match fs::read("tests/.test_credentials.toml") {
167+ Ok(bytes) => bytes,
168+ Err(err) => {
169+ eprintln!("Skipping test. No S3 credentials available: {}", err);
170+ return Ok(());
171+ }
172+ };
173+
174+ // Try to load S3 credentials `.test_credentials.toml`. If they don't exist,
175+ // then we can't really run this test. Note that these should be completely
176+ // separate credentials than what is used in production.
177+ let creds: Credentials = toml::from_slice(&config)?;
178+
179+ std::env::set_var("AWS_ACCESS_KEY_ID", creds.access_key_id);
180+ std::env::set_var("AWS_SECRET_ACCESS_KEY", creds.secret_access_key);
181+ std::env::set_var("AWS_DEFAULT_REGION", creds.default_region);
182+
183+ // Make sure our seed is deterministic. This prevents us from filling up our
184+ // S3 bucket with a bunch of random files if this test gets ran a bunch of
185+ // times.
186+ let mut rng = StdRng::seed_from_u64(42);
187+
188+ let key = rng.gen();
189+
190+ let mut server = S3ServerBuilder::new(creds.bucket, key);
191+ server.prefix("test_lfs".into());
192+
193+ let server = server.spawn(SocketAddr::from(([0, 0, 0, 0], 0))).await?;
194+ let addr = server.addr();
195+
196+ let (shutdown_tx, shutdown_rx) = oneshot::channel();
197+
198+ let server = tokio::spawn(futures::future::select(shutdown_rx, server));
199+
200+ let repo = GitRepo::init(addr)?;
201+ repo.add_random(Path::new("4mb.bin"), 4 * 1024 * 1024, &mut rng)?;
202+ repo.add_random(Path::new("8mb.bin"), 8 * 1024 * 1024, &mut rng)?;
203+ repo.add_random(Path::new("16mb.bin"), 16 * 1024 * 1024, &mut rng)?;
204+ repo.commit("Add LFS objects")?;
205+
206+ // Make sure we can push LFS objects to the server.
207+ repo.lfs_push().unwrap();
208+
209+ // Make sure we can re-download the same objects.
210+ repo.clean_lfs().unwrap();
211+ repo.lfs_pull().unwrap();
212+
213+ // Push again. This should be super fast.
214+ repo.lfs_push().unwrap();
215+
216+ shutdown_tx.send(()).expect("server died too soon");
217+
218+ if let Either::Right((result, _)) = server.await.unwrap() {
219+ // If the server exited first, then propagate the error.
220+ result.expect("server failed unexpectedly");
221+ }
222+
223+ Ok(())
224+ }