From b1cfd16913e6957739958ef729b87c1bf3674a5d Mon Sep 17 00:00:00 2001 From: Alex Auvolat Date: Fri, 17 Dec 2021 11:53:13 +0100 Subject: New buckets for 0.6.0: small fixes, including: - ensure bucket names are correct aws s3 names - when making aliases, ensure timestamps of links in both ways are the same - fix small remarks by trinity - don't have a separate website_access field --- src/model/bucket_alias_table.rs | 43 ++++++++++++++++++++++++++++++++++++----- src/model/bucket_helper.rs | 12 ++++++++++-- src/model/bucket_table.rs | 9 ++++----- src/model/key_table.rs | 2 +- src/model/migrate.rs | 32 ++++++++++++++++++++---------- src/model/permission.rs | 3 +++ 6 files changed, 78 insertions(+), 23 deletions(-) (limited to 'src/model') diff --git a/src/model/bucket_alias_table.rs b/src/model/bucket_alias_table.rs index 52484c5b..904a5255 100644 --- a/src/model/bucket_alias_table.rs +++ b/src/model/bucket_alias_table.rs @@ -8,7 +8,7 @@ use garage_util::data::*; /// in the global namespace. #[derive(PartialEq, Clone, Debug, Serialize, Deserialize)] pub struct BucketAlias { - pub name: String, + name: String, pub state: crdt::Lww>, } @@ -22,15 +22,22 @@ impl AutoCrdt for AliasParams { } impl BucketAlias { - pub fn new(name: String, bucket_id: Uuid) -> Self { - BucketAlias { - name, - state: crdt::Lww::new(crdt::Deletable::present(AliasParams { bucket_id })), + pub fn new(name: String, bucket_id: Uuid) -> Option { + if !is_valid_bucket_name(&name) { + None + } else { + Some(BucketAlias { + name, + state: crdt::Lww::new(crdt::Deletable::present(AliasParams { bucket_id })), + }) } } pub fn is_deleted(&self) -> bool { self.state.get().is_deleted() } + pub fn name(&self) -> &str { + &self.name + } } impl Crdt for BucketAlias { @@ -62,3 +69,29 @@ impl TableSchema for BucketAliasTable { filter.apply(entry.is_deleted()) } } + +/// Check if a bucket name is valid. +/// +/// The requirements are listed here: +/// +/// +/// +/// In the case of Garage, bucket names must not be hex-encoded +/// 32 byte string, which is excluded thanks to the +/// maximum length of 63 bytes given in the spec. +pub fn is_valid_bucket_name(n: &str) -> bool { + // Bucket names must be between 3 and 63 characters + n.len() >= 3 && n.len() <= 63 + // Bucket names must be composed of lowercase letters, numbers, + // dashes and dots + && n.chars().all(|c| matches!(c, '.' | '-' | 'a'..='z' | '0'..='9')) + // Bucket names must start and end with a letter or a number + && !n.starts_with(&['-', '.'][..]) + && !n.ends_with(&['-', '.'][..]) + // Bucket names must not be formated as an IP address + && n.parse::().is_err() + // Bucket names must not start wih "xn--" + && !n.starts_with("xn--") + // Bucket names must not end with "-s3alias" + && !n.ends_with("-s3alias") +} diff --git a/src/model/bucket_helper.rs b/src/model/bucket_helper.rs index c1280afa..b55ebc4b 100644 --- a/src/model/bucket_helper.rs +++ b/src/model/bucket_helper.rs @@ -8,12 +8,21 @@ use crate::garage::Garage; pub struct BucketHelper<'a>(pub(crate) &'a Garage); -#[allow(clippy::ptr_arg)] impl<'a> BucketHelper<'a> { + #[allow(clippy::ptr_arg)] pub async fn resolve_global_bucket_name( &self, bucket_name: &String, ) -> Result, Error> { + // Bucket names in Garage are aliases, true bucket identifiers + // are 32-byte UUIDs. This function resolves bucket names into + // their full identifier by looking up in the bucket_alias_table. + // This function also allows buckets to be identified by their + // full UUID (hex-encoded). Here, if the name to be resolved is a + // hex string of the correct length, it is directly parsed as a bucket + // identifier which is returned. There is no risk of this conflicting + // with an actual bucket name: bucket names are max 63 chars long by + // the AWS spec, and hex-encoded UUIDs are 64 chars long. let hexbucket = hex::decode(bucket_name.as_str()) .ok() .map(|by| Uuid::try_from(&by)) @@ -37,7 +46,6 @@ impl<'a> BucketHelper<'a> { } } - #[allow(clippy::ptr_arg)] pub async fn get_existing_bucket(&self, bucket_id: Uuid) -> Result { self.0 .bucket_table diff --git a/src/model/bucket_table.rs b/src/model/bucket_table.rs index 6ae719ae..00e03899 100644 --- a/src/model/bucket_table.rs +++ b/src/model/bucket_table.rs @@ -8,6 +8,8 @@ use garage_util::time::*; use crate::permission::BucketKeyPerm; +pub const DEFAULT_WEBSITE_CONFIGURATION: &[u8] = b""; // TODO (an XML WebsiteConfiguration document per the AWS spec) + /// A bucket is a collection of objects /// /// Its parameters are not directly accessible as: @@ -29,9 +31,8 @@ pub struct BucketParams { /// Map of key with access to the bucket, and what kind of access they give pub authorized_keys: crdt::Map, /// Whether this bucket is allowed for website access - /// (under all of its global alias names) - pub website_access: crdt::Lww, - /// The website configuration XML document + /// (under all of its global alias names), + /// and if so, the website configuration XML document pub website_config: crdt::Lww>, /// Map of aliases that are or have been given to this bucket /// in the global namespace @@ -50,7 +51,6 @@ impl BucketParams { BucketParams { creation_date: now_msec(), authorized_keys: crdt::Map::new(), - website_access: crdt::Lww::new(false), website_config: crdt::Lww::new(None), aliases: crdt::LwwMap::new(), local_aliases: crdt::LwwMap::new(), @@ -61,7 +61,6 @@ impl BucketParams { impl Crdt for BucketParams { fn merge(&mut self, o: &Self) { self.authorized_keys.merge(&o.authorized_keys); - self.website_access.merge(&o.website_access); self.website_config.merge(&o.website_config); self.aliases.merge(&o.aliases); self.local_aliases.merge(&o.local_aliases); diff --git a/src/model/key_table.rs b/src/model/key_table.rs index 526ed496..3285e355 100644 --- a/src/model/key_table.rs +++ b/src/model/key_table.rs @@ -190,7 +190,7 @@ impl TableSchema for KeyTable { local_aliases: crdt::LwwMap::new(), }) }; - let name = crdt::Lww::migrate_from_raw(old_k.name.timestamp(), old_k.name.get().clone()); + let name = crdt::Lww::raw(old_k.name.timestamp(), old_k.name.get().clone()); Some(Key { key_id: old_k.key_id, secret_key: old_k.secret_key, diff --git a/src/model/migrate.rs b/src/model/migrate.rs index 35ff1807..e4469e64 100644 --- a/src/model/migrate.rs +++ b/src/model/migrate.rs @@ -1,5 +1,7 @@ use std::sync::Arc; +use serde_bytes::ByteBuf; + use garage_table::util::EmptyKey; use garage_util::crdt::*; use garage_util::data::*; @@ -38,6 +40,16 @@ impl Migrate { old_bucket: &old_bucket::Bucket, old_bucket_p: &old_bucket::BucketParams, ) -> Result<(), Error> { + let bucket_id = blake2sum(old_bucket.name.as_bytes()); + + let new_name = if is_valid_bucket_name(&old_bucket.name) { + old_bucket.name.clone() + } else { + // if old bucket name was not valid, replace it by + // a hex-encoded name derived from its identifier + hex::encode(&bucket_id.as_slice()[..16]) + }; + let mut new_ak = Map::new(); for (k, ts, perm) in old_bucket_p.authorized_keys.items().iter() { new_ak.put( @@ -52,27 +64,27 @@ impl Migrate { } let mut aliases = LwwMap::new(); - aliases.update_in_place(old_bucket.name.clone(), true); + aliases.update_in_place(new_name.clone(), true); + + let website = if *old_bucket_p.website.get() { + Some(ByteBuf::from(DEFAULT_WEBSITE_CONFIGURATION.to_vec())) + } else { + None + }; let new_bucket = Bucket { - id: blake2sum(old_bucket.name.as_bytes()), + id: bucket_id, state: Deletable::Present(BucketParams { creation_date: now_msec(), authorized_keys: new_ak.clone(), - website_access: Lww::new(*old_bucket_p.website.get()), - website_config: Lww::new(None), + website_config: Lww::new(website), aliases, local_aliases: LwwMap::new(), }), }; self.garage.bucket_table.insert(&new_bucket).await?; - let new_alias = BucketAlias { - name: old_bucket.name.clone(), - state: Lww::new(Deletable::Present(AliasParams { - bucket_id: new_bucket.id, - })), - }; + let new_alias = BucketAlias::new(new_name.clone(), new_bucket.id).unwrap(); self.garage.bucket_alias_table.insert(&new_alias).await?; for (k, perm) in new_ak.items().iter() { diff --git a/src/model/permission.rs b/src/model/permission.rs index 04bb2bc5..ebb24a32 100644 --- a/src/model/permission.rs +++ b/src/model/permission.rs @@ -34,6 +34,9 @@ impl Crdt for BucketKeyPerm { if !other.allow_write { self.allow_write = false; } + if !other.allow_owner { + self.allow_owner = false; + } } _ => (), } -- cgit v1.2.3