Merge pull request 'optionally support puny code (fix #273)' (#1042) from trinity-1686a/garage:1686a/punnycode into main

Reviewed-on: https://git.deuxfleurs.fr/Deuxfleurs/garage/pulls/1042
This commit is contained in:
Alex 2025-05-22 12:49:10 +00:00
commit de8eeab4ad
13 changed files with 106 additions and 65 deletions

43
Cargo.lock generated
View file

@ -1300,7 +1300,6 @@ dependencies = [
"http-body-util",
"hyper 1.6.0",
"hyper-util",
"idna 0.5.0",
"md-5",
"nom",
"opentelemetry",
@ -2170,16 +2169,6 @@ version = "1.0.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "b9e0384b61958566e926dc50660321d12159025e767c18e043daf26b70104c39"
[[package]]
name = "idna"
version = "0.5.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "634d9b1461af396cad843f47fdba5597a4f9e6ddd4bfb6ff5d85028c25cb12f6"
dependencies = [
"unicode-bidi",
"unicode-normalization",
]
[[package]]
name = "idna"
version = "1.0.3"
@ -4252,21 +4241,6 @@ dependencies = [
"zerovec",
]
[[package]]
name = "tinyvec"
version = "1.9.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "09b3661f17e86524eccd4371ab0429194e0d7c008abb45f7a7495b1719463c71"
dependencies = [
"tinyvec_macros",
]
[[package]]
name = "tinyvec_macros"
version = "0.1.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "1f3ccbac311fea05f86f61904b462b55fb3df8837a366dfc601a0161d0532f20"
[[package]]
name = "tokio"
version = "1.44.1"
@ -4587,27 +4561,12 @@ version = "0.1.7"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "2896d95c02a80c6d6a5d6e953d479f5ddf2dfdb6a244441010e373ac0fb88971"
[[package]]
name = "unicode-bidi"
version = "0.3.18"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "5c1cb5db39152898a79168971543b1cb5020dff7fe43c8dc468b0885f5e29df5"
[[package]]
name = "unicode-ident"
version = "1.0.18"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "5a5f39404a5da50712a4c1eecf25e90dd62b613502b7e925fd4e4d19b5c96512"
[[package]]
name = "unicode-normalization"
version = "0.1.24"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "5033c97c4262335cded6d6fc3e5c18ab755e1a3dc96376350f3d8e9f009ad956"
dependencies = [
"tinyvec",
]
[[package]]
name = "unicode-segmentation"
version = "1.12.0"
@ -4655,7 +4614,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "32f8b686cadd1473f4bd0117a5d28d36b1ade384ea9b5069a1c40aefed7fda60"
dependencies = [
"form_urlencoded",
"idna 1.0.3",
"idna",
"percent-encoding",
]

View file

@ -58,7 +58,6 @@ git-version = "0.3.4"
hex = "0.4"
hexdump = "0.1"
hmac = "0.12"
idna = "0.5"
itertools = "0.12"
ipnet = "2.9.0"
lazy_static = "1.4"

View file

@ -46,6 +46,7 @@ bootstrap_peers = [
"212fd62eeaca72c122b45a7f4fa0f55e012aa5e24ac384a72a3016413fa724ff@[fc00:F::1]:3901",
]
allow_punycode = false
[consul_discovery]
api = "catalog"
@ -115,6 +116,7 @@ Top-level configuration options:
[`rpc_public_addr`](#rpc_public_addr),
[`rpc_public_addr_subnet`](#rpc_public_addr_subnet)
[`rpc_secret`/`rpc_secret_file`](#rpc_secret).
[`allow_punycode`](#allow_punycode).
The `[consul_discovery]` section:
[`api`](#consul_api),
@ -604,7 +606,7 @@ be obtained by running `garage node id` and then included directly in the
key will be returned by `garage node id` and you will have to add the IP
yourself.
### `allow_world_readable_secrets` or `GARAGE_ALLOW_WORLD_READABLE_SECRETS` (env) {#allow_world_readable_secrets}
#### `allow_world_readable_secrets` or `GARAGE_ALLOW_WORLD_READABLE_SECRETS` (env) {#allow_world_readable_secrets}
Garage checks the permissions of your secret files to make sure they're not
world-readable. In some cases, the check might fail and consider your files as
@ -616,6 +618,13 @@ permission verification.
Alternatively, you can set the `GARAGE_ALLOW_WORLD_READABLE_SECRETS`
environment variable to `true` to bypass the permissions check.
#### `allow_punycode` {#allow_punycode}
Allow creating buckets with names containing punycode. When used for buckets served
as websites, this allows using almost any unicode character in the domain name.
Default to `false`.
### The `[consul_discovery]` section
Garage supports discovering other nodes of the cluster using Consul. For this

View file

@ -277,7 +277,7 @@ pub async fn handle_create_bucket(
let helper = garage.locked_helper().await;
if let Some(ga) = &req.global_alias {
if !is_valid_bucket_name(ga) {
if !is_valid_bucket_name(ga, garage.config.allow_punycode) {
return Err(Error::bad_request(format!(
"{}: {}",
ga, INVALID_BUCKET_NAME_MESSAGE
@ -292,7 +292,7 @@ pub async fn handle_create_bucket(
}
if let Some(la) = &req.local_alias {
if !is_valid_bucket_name(&la.alias) {
if !is_valid_bucket_name(&la.alias, garage.config.allow_punycode) {
return Err(Error::bad_request(format!(
"{}: {}",
la.alias, INVALID_BUCKET_NAME_MESSAGE

View file

@ -28,7 +28,6 @@ err-derive.workspace = true
hex.workspace = true
hmac.workspace = true
md-5.workspace = true
idna.workspace = true
tracing.workspace = true
nom.workspace = true
pin-project.workspace = true

View file

@ -8,7 +8,6 @@ use hyper::{
body::{Body, Bytes},
Request, Response,
};
use idna::domain_to_unicode;
use serde::{Deserialize, Serialize};
use garage_model::bucket_table::BucketParams;
@ -97,7 +96,7 @@ pub fn authority_to_host(authority: &str) -> Result<String, Error> {
authority
))),
};
authority.map(|h| domain_to_unicode(h).0)
authority.map(|h| h.to_ascii_lowercase())
}
/// Extract the bucket name and the key name from an HTTP path and possibly a bucket provided in

View file

@ -172,7 +172,7 @@ pub async fn handle_create_bucket(
}
// Create the bucket!
if !is_valid_bucket_name(&bucket_name) {
if !is_valid_bucket_name(&bucket_name, garage.config.allow_punycode) {
return Err(Error::bad_request(format!(
"{}: {}",
bucket_name, INVALID_BUCKET_NAME_MESSAGE

View file

@ -126,7 +126,7 @@ impl AdminRpcHandler {
#[allow(clippy::ptr_arg)]
async fn handle_create_bucket(&self, name: &String) -> Result<AdminRpc, Error> {
if !is_valid_bucket_name(name) {
if !is_valid_bucket_name(name, self.garage.config.allow_punycode) {
return Err(Error::BadRequest(format!(
"{}: {}",
name, INVALID_BUCKET_NAME_MESSAGE

View file

@ -63,6 +63,8 @@ rpc_bind_addr = "127.0.0.1:{rpc_port}"
rpc_public_addr = "127.0.0.1:{rpc_port}"
rpc_secret = "{secret}"
allow_punycode = true
[s3_api]
s3_region = "{region}"
api_bind_addr = "127.0.0.1:{s3_port}"

View file

@ -533,3 +533,76 @@ async fn test_website_check_domain() {
})
);
}
#[tokio::test]
async fn test_website_puny() {
const BCKT_NAME: &str = "xn--pda.eu";
let ctx = common::context();
let bucket = ctx.create_bucket(BCKT_NAME);
let data = ByteStream::from_static(BODY);
ctx.client
.put_object()
.bucket(&bucket)
.key("index.html")
.body(data)
.send()
.await
.unwrap();
let client = Client::builder(TokioExecutor::new()).build_http();
let req = |suffix| {
Request::builder()
.method("GET")
.uri(format!("http://127.0.0.1:{}/", ctx.garage.web_port))
.header("Host", format!("{}{}", BCKT_NAME, suffix))
.body(Body::new(Bytes::new()))
.unwrap()
};
ctx.garage
.command()
.args(["bucket", "website", "--allow", BCKT_NAME])
.quiet()
.expect_success_status("Could not allow website on bucket");
let mut resp = client.request(req("")).await.unwrap();
assert_eq!(resp.status(), StatusCode::OK);
assert_eq!(
resp.into_body().collect().await.unwrap().to_bytes(),
BODY.as_ref()
);
resp = client.request(req(".web.garage")).await.unwrap();
assert_eq!(resp.status(), StatusCode::OK);
assert_eq!(
resp.into_body().collect().await.unwrap().to_bytes(),
BODY.as_ref()
);
for bname in [
BCKT_NAME.to_string(),
format!("{BCKT_NAME}.web.garage"),
format!("{BCKT_NAME}.s3.garage"),
] {
let admin_req = || {
Request::builder()
.method("GET")
.uri(format!(
"http://127.0.0.1:{0}/check?domain={1}",
ctx.garage.admin_port, bname
))
.body(Body::new(Bytes::new()))
.unwrap()
};
let admin_resp = client.request(admin_req()).await.unwrap();
assert_eq!(admin_resp.status(), StatusCode::OK);
assert_eq!(
admin_resp.into_body().collect().await.unwrap().to_bytes(),
format!("Domain '{bname}' is managed by Garage").as_bytes()
);
}
}

View file

@ -22,14 +22,10 @@ mod v08 {
pub use v08::*;
impl BucketAlias {
pub fn new(name: String, ts: u64, bucket_id: Option<Uuid>) -> Option<Self> {
if !is_valid_bucket_name(&name) {
None
} else {
Some(BucketAlias {
name,
state: crdt::Lww::raw(ts, bucket_id),
})
pub fn new(name: String, ts: u64, bucket_id: Option<Uuid>) -> Self {
BucketAlias {
name,
state: crdt::Lww::raw(ts, bucket_id),
}
}
@ -80,7 +76,7 @@ impl TableSchema for BucketAliasTable {
/// 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 {
pub fn is_valid_bucket_name(n: &str, puny: bool) -> 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,
@ -92,7 +88,9 @@ pub fn is_valid_bucket_name(n: &str) -> bool {
// Bucket names must not be formatted as an IP address
&& n.parse::<std::net::IpAddr>().is_err()
// Bucket names must not start with "xn--"
&& !n.starts_with("xn--")
&& (!n.starts_with("xn--") || puny)
// We are a bit stricter, to properly restrict punycode in all labels
&& (!n.contains(".xn--") || puny)
// Bucket names must not end with "-s3alias"
&& !n.ends_with("-s3alias")
}

View file

@ -57,7 +57,7 @@ impl<'a> LockedHelper<'a> {
bucket_id: Uuid,
alias_name: &String,
) -> Result<(), Error> {
if !is_valid_bucket_name(alias_name) {
if !is_valid_bucket_name(alias_name, self.0.config.allow_punycode) {
return Err(Error::InvalidBucketName(alias_name.to_string()));
}
@ -88,8 +88,7 @@ impl<'a> LockedHelper<'a> {
// writes are now done and all writes use timestamp alias_ts
let alias = match alias {
None => BucketAlias::new(alias_name.clone(), alias_ts, Some(bucket_id))
.ok_or_else(|| Error::InvalidBucketName(alias_name.clone()))?,
None => BucketAlias::new(alias_name.clone(), alias_ts, Some(bucket_id)),
Some(mut a) => {
a.state = Lww::raw(alias_ts, Some(bucket_id));
a
@ -218,7 +217,7 @@ impl<'a> LockedHelper<'a> {
) -> Result<(), Error> {
let key_helper = KeyHelper(self.0);
if !is_valid_bucket_name(alias_name) {
if !is_valid_bucket_name(alias_name, self.0.config.allow_punycode) {
return Err(Error::InvalidBucketName(alias_name.to_string()));
}

View file

@ -135,6 +135,10 @@ pub struct Config {
/// Configuration for the admin API endpoint
#[serde(default = "Default::default")]
pub admin: AdminConfig,
/// Allow punycode in bucket names
#[serde(default)]
pub allow_punycode: bool,
}
/// Value for data_dir: either a single directory or a list of dirs with attributes