chadtmiller.com/knotless
git that lives in your Atmosphere account
knotless-serve answers repositories' sites on their own domain
MergedThis adds --pages-domain to knotless-serve. Requests on that domain, or on a name under it, are answered from dev.knotless.site records, with files read from the owner's PDS and cached by hash. The index gains a site table. The path rules, 304s, redirects and the takedown filtering look right.
Must fix: arbitrary file read and write through sha256. SiteFile.sha256 comes from a record the repository owner controls. Nothing checks it before it goes into file paths:
fetchdoesdir.join(&file.sha256)and callstry_exists. A record withsha256: "../../../../../../../../../../../../../../../../../../etc/passwd"(padded to 64 characters) passes the lexicon's length rule. The PDS doesn't validate this unknown lexicon, and the index only runsserde_json::from_value. The file exists, sokeepis skipped andFile::openstreams it back to the requester.keepbuilds.{sha256}.{n}underdirand writes there. Asha256such as./../xgives../../x.0, which sits outside the cache directory. The hash check fails afterwards, but the file has already been created, and removing it is only best effort.
The fix is to require exactly 64 lowercase hex characters. Do it in the index, where a record that fails is passed by like a non-site record, and again in fetch (or SiteFile) as a second check. Add a test that a record with a traversal sha256 is not served.
Smaller: open redirect. answer redirects a bare /<name> to /{name}/ with name taken after percent-decoding. GET /%5Cevil.com gives Location: /\evil.com/, and browsers read that as //evil.com. Reject a name with \ or control characters, or redirect to the percent-encoded form.
crates/knotless-serve/src/sites.rsline 321Outdated
) -> anyhow::Result<tokio::fs::File> { | |
let ctx = &self.app.ctx; | |
let dir = crate::releases::kept_dir(ctx); | |
let at = dir.join(&file.sha256); |
file.sha256 is unvalidated record data, so this join can leave dir. With ../… as the hash, try_exists is true for any existing file and it is then opened and served. Require 64 lowercase hex characters before using it as a filename.
crates/knotless-serve/src/sites.rsline 345Outdated
let remote = Remote::open(ctx, &url).await?; | |
tokio::fs::create_dir_all(dir).await?; | |
let n = KEEPING.fetch_add(1, Ordering::Relaxed); | |
let keeping = dir.join(format!(".{}.{n}", file.sha256)); |
The same unvalidated hash goes into this temp-file name, so ./../x creates a file outside dir before the hash check fails.
crates/knotless-serve/src/sites.rsline 260Outdated
return moved(&format!("{}/{named}", self.home.trim_end_matches('/'))); | |
} | |
let Some(rest) = rest else { | |
return moved(&format!("/{name}/")); |
name is percent-decoded, so /%5Cevil.com redirects to /\evil.com/, which browsers treat as //evil.com. Reject or encode \ and control characters.
The head moved from 310e0e3 to 8928687, but only through merge commits. The serve code is unchanged, so nothing I asked for last round has been done.
sha256path traversal (must fix).fetchstill doesdir.join(&file.sha256)andkeepstill builds.{sha256}.{n}from the same unvalidated record value. A record owner can make the server serve any readable file, or create files outside the cache directory. Require exactly 64 lowercase hex characters in the index, where a record that fails is passed by like a non-site record. Check again infetchas a second line. Add a test that a record with a traversalsha256is not served.- Open redirect.
/%5Cevil.comstill redirects to/\evil.com/, which browsers read as//evil.com. Reject or percent-encode\and control characters inname.
Because this is a security fix, please don't merge until both are done. The pull is already queued, so you may want to remove it from the queue while you fix them.
crates/knotless-serve/src/sites.rsline 321Outdated
) -> anyhow::Result<tokio::fs::File> { | |
let ctx = &self.app.ctx; | |
let dir = crate::releases::kept_dir(ctx); | |
let at = dir.join(&file.sha256); |
Still unvalidated: file.sha256 can be ../…, so this join leaves dir and an existing file is opened and served. Check for 64 lowercase hex before use.
crates/knotless-serve/src/sites.rsline 345Outdated
let remote = Remote::open(ctx, &url).await?; | |
tokio::fs::create_dir_all(dir).await?; | |
let n = KEEPING.fetch_add(1, Ordering::Relaxed); | |
let keeping = dir.join(format!(".{}.{n}", file.sha256)); |
Still unvalidated: this temp name is built from the same sha256, so a value like ./../x creates a file outside dir.
crates/knotless-serve/src/sites.rsline 260Outdated
return moved(&format!("{}/{named}", self.home.trim_end_matches('/'))); | |
} | |
let Some(rest) = rest else { | |
return moved(&format!("/{name}/")); |
name is percent-decoded, so /%5Cevil.com redirects to /\evil.com/, an open redirect. Reject or encode \ and control characters.
The head moved from 8928687 to a885225 through a merge and two small commits. Neither fix I asked for is in, and the one new commit ("The firehose keeps a site's record for the index") touches neither.
sha256path traversal (must fix, still open).fetchstill doesdir.join(&file.sha256)(sites.rs:321).keepstill builds.{sha256}.{n}from the same unchecked record value (sites.rs:345). A repository owner can make the server serve any readable file, or create files outside the cache directory. The index test still stores a site withsha256: "a"(index.rs:7002), so the index checks nothing here. Require exactly 64 lowercase hex characters in the index, and treat a record that fails as not a site. Check again infetchas a second line. Add a test that a traversalsha256is not served. Intests/sites.rsthe hash only appears in the tamper test (tests/sites.rs:158), and that one uses valid hex.- Open redirect (still open).
moved(&format!("/{name}/"))atsites.rs:260uses the percent-decodedname, so/%5Cevil.comredirects to/\evil.com/. Browsers read that as//evil.com. Reject\and control characters inname, or percent-encode it.
I'm asking for changes again because the first one is a file-read hole reachable by any account. The queue's pre-flight check on 8928687 also failed, so check the checks on the new head.
crates/knotless-serve/src/sites.rsline 321Outdated
) -> anyhow::Result<tokio::fs::File> { | |
let ctx = &self.app.ctx; | |
let dir = crate::releases::kept_dir(ctx); | |
let at = dir.join(&file.sha256); |
Still unvalidated: file.sha256 can be ../…, so this join leaves dir and an existing file is opened and served. Require 64 lowercase hex before using it as a filename, in the index and here.
crates/knotless-serve/src/sites.rsline 345Outdated
let remote = Remote::open(ctx, &url).await?; | |
tokio::fs::create_dir_all(dir).await?; | |
let n = KEEPING.fetch_add(1, Ordering::Relaxed); | |
let keeping = dir.join(format!(".{}.{n}", file.sha256)); |
Same unvalidated sha256 in the temp name: ./../x creates a file outside dir before the hash check fails.
crates/knotless-serve/src/sites.rsline 260Outdated
return moved(&format!("{}/{named}", self.home.trim_end_matches('/'))); | |
} | |
let Some(rest) = rest else { | |
return moved(&format!("/{name}/")); |
name is percent-decoded, so /%5Cevil.com gives Location: /\evil.com/, an open redirect. Reject or encode \ and control characters.
The head moved from a885225 to 133bf27 with one new commit ("Every collection a backfill reads, the firehose keeps"). Neither fix I asked for is in.
sha256path traversal (must fix, still open).fetchstill doesdir.join(&file.sha256)(sites.rs:321).keepstill builds.{sha256}.{n}from the same unchecked record value (sites.rs:345). A repository owner can make the server serve any file it can read, or create files outside the cache directory. Require exactly 64 lowercase hex characters in the index, and treat a record that fails as not a site. Check again infetchas a second line. Add a test that a traversalsha256is not served.- Open redirect (still open).
moved(&format!("/{name}/"))atsites.rs:260uses the percent-decodedname./%5Cevil.comredirects to/\evil.com/, which browsers read as//evil.com. Reject\and control characters inname, or percent-encode it.
Please don't merge until both are done.
crates/knotless-serve/src/sites.rsline 321Outdated
) -> anyhow::Result<tokio::fs::File> { | |
let ctx = &self.app.ctx; | |
let dir = crate::releases::kept_dir(ctx); | |
let at = dir.join(&file.sha256); |
file.sha256 comes straight from the owner's record and is joined into a path unchecked. Require 64 lowercase hex here and in the index.
crates/knotless-serve/src/sites.rsline 260Outdated
return moved(&format!("{}/{named}", self.home.trim_end_matches('/'))); | |
} | |
let Some(rest) = rest else { | |
return moved(&format!("/{name}/")); |
name is percent-decoded, so /%5Cevil.com redirects to /\evil.com/. Reject \ and control characters, or percent-encode the name.
Both problems from my earlier rounds are now fixed at 8c5a70c.
sha256traversal:fetchnow checks the hash withvalid_sha256(sites.rs:327) before any path is built from it.valid_sha256requires exactly 64 lowercase hex characters, sokeep's.{sha256}.{n}name (sites.rs:353) is safe too.- Open redirect:
answerreturns a 404 for anamethat contains\or a control character, before it builds the/{name}/redirect (sites.rs:259).
I did not check whether the index also rejects a bad hash, or whether tests for these two cases exist. The check in fetch is enough to close the hole.
What changes
knotless-serve --pages-domain knotless.pageanswers repositories' sites, wrapped around every other route. A request whose host is that domain or a name under it is answered as a site and nothing else. OnlyGETandHEADare allowed (anything else is 405), and nothing else on the server is reachable from those hosts. The flag needs--public-url, since an owner's root (/) redirects to their page there.A request is answered in this order:
--for-and-for., and resolved to a DID. The DID's own claimed handle must match it. A 24-character label of[a-z2-7]is a plc id.dev.knotless.siterecord by the repository's key. Taken-down records, repositories and accounts are left out.x/servesx/index.html.xwith onlyx/index.htmlredirects tox/, and/<repo>to/<repo>/. A page no file has getsfallback, and anything else missing getsnotFoundwith 404, or a plain 404.contentType,nosniff, an ETag of the hash (304 on a match) andmax-age=60.The index gains a
sitetable. It keeps a publicdev.knotless.siterecord by URI, passing by one in a space or one that isn't a site's record. It's backfilled, dropped with its repository and purged with its account.siteis inGROWING, and its one query is by primary key.Why
This is the serving half of pages (docs/design/pages.md), on top of
knotless site. That pull is knotless site deploys a directory as a repository's website, and this branch holds its commits until it lands.How it was checked
tests/sites.rs: an appview following the PDS answers a site deployed withknotless site. It covers both owner labels, the ETag and 304, directory redirects, the owner's root, the 404 page and plain 404s (no site, no repository, a bad path, unknown and unresolvable labels), 405, other hosts passing through, a redeploy as a single-page app, a tampered hash giving 502, and taking the site down.is_page, percent-decoding, host parsing and the index'ssitetable.tests/serve.rsruns the binary with--pages-domain, and checks it refuses the flag without--public-url.Nothing was built here: CI builds and runs it.
Shortcomings
deploy/changes (Caddy, wildcard certificate, DNS) are later pulls.🤖 Generated with Claude Code