Feat/file upload hosting - #1407
Open
nisay759 wants to merge 20 commits into
Open
Conversation
The /metrics handler assigned h.options.Stats (a *Metrics) to a local variable, which copied the pointer rather than the struct. Every request therefore mutated the single shared Metrics struct when setting Cache, Cpu, Memory and Network, racing with concurrent /metrics requests, and read the counters non-atomically while the protocol servers updated them with atomic adds. Snapshot the counters into a local value using atomic loads instead, so the shared struct is never written and the encoded values are consistent. Add a regression test asserting the handler leaves the shared struct untouched, plus a concurrent metrics test that fails under -race on the old code. Co-Authored-By: Claude <noreply@anthropic.com>
Groundwork for session-scoped file hosting. Uploaded file bytes will live
on disk, but their lifecycle has to be driven by the correlation-id they
belong to, so the metadata lives alongside the rest of the session state.
- UploadedFile{Name,Size,SHA256,Timestamp} and a Files slice on
CorrelationData, guarded by its existing mutex. Reusing CorrelationData
rather than a second cache key means TTL expiry, capacity eviction,
RemoveID and Close all cover the metadata with no second key to keep in
sync. Not persisted: only interaction blobs go to leveldb, and uploads
are not expected to survive a restart.
- Options.OnEviction, invoked whenever a correlation-id leaves the cache,
so the server can delete the corresponding files. This sits alongside
the existing OnRemoval hook rather than replacing it: OnRemoval counts
client sessions, while OnEviction fires for every entry and receives the
evicted data, which is what file cleanup needs.
- UpdateUploads verifies the secret key and runs a callback under the
correlation-id lock. Callers do their disk write inside the callback,
which makes the quota check and the commit atomic against concurrent
uploads to the same session. ListUploads is the unauthenticated read
path used when serving, and returns a copy.
Both live on a separate UploadStorage interface rather than on Storage.
Uploaded bytes are written to the local filesystem of one instance and the
capacity quota is an in-process counter, so the capability is only coherent
for instance-local backends. StorageDB implements it; the Redis backend
deliberately does not, since a shared backend would let one instance
advertise files whose bytes only exist on another instance's disk.
Co-Authored-By: Claude <noreply@anthropic.com>
Owns the on-disk lifecycle of client-uploaded files, laid out as
<root>/.interactsh-user-uploads/<correlationID>/<filename>. The session
directory is named by correlation ID because FTP has no Host header, so the
identifier has to travel in the path for the FTP view to resolve it.
Sessions sit under .interactsh-user-uploads rather than directly at the root
so that the root can be shared with -ftp-dir safely. Everything this store
creates, enumerates and deletes lives under that one directory, so an
operator directory that happens to share the correlation-ID name shape can
never be reaped -- and that shape is not hard to collide with, since -cidl
goes as low as 3.
Root resolution prefers -upload-directory, then falls back to the FTP root
(so -ftp serves uploads with no extra configuration), then to a temporary
directory. Creating the sessions directory creates the root with it, so a
-ftp-dir that does not exist yet -- the FTP server never required it to --
does not turn into a boot failure when -upload is added.
Anything already inside the sessions directory is purged at startup: upload
metadata lives only in the cache, so no session survives a restart and any
surviving directory is an orphan. The purge and the janitor both skip entries
that do not look like a correlation ID, which is a second line of defence for
anything unexpected in there; operator files are kept safe structurally, by
being outside that directory entirely.
File names go through a strict allowlist rather than a sanitiser. The user
needs the exact byte-for-byte name to reference the file from a DTD, so
mangling it silently is worse than rejecting it; it also means the name
needs no escaping in a Content-Disposition header. Reads resolve through an
os.Root handle, which refuses to escape the root or follow a symlink out of
it on every platform we build for. Writes go to a temp name and are renamed
into place, so neither the HTTP handler nor the FTP file driver can observe
a partially written file.
Cleanup runs three ways, because none of them is sufficient alone:
- RemoveSession, queued to a deleter goroutine via a non-blocking send.
It is called from the storage cache's event goroutine, where a
synchronous RemoveAll would serialise all cache maintenance behind a
64-slot channel.
- A mtime-based janitor, which is the authoritative collector.
goburrow/cache has no background janitor, so an idle server never
evicts and would otherwise leak every uploaded file indefinitely.
- The startup purge above.
The janitor deliberately judges liveness by directory mtime and never asks
the cache: GetIfPresent refreshes access time, so probing each swept session
would make exactly those sessions immortal under sliding eviction.
Co-Authored-By: Claude <noreply@anthropic.com>
The upload directory is probed for writability before the store is returned.
MkdirAll reports success for a directory that already exists but cannot be
written to -- a read-only mount, or one owned by another user -- so without the
probe that misconfiguration would first surface as a 500 on a client's initial
upload, long after startup. The probe creates, writes and removes a file rather
than reading permission bits, which answer for the wrong subject on a setuid
binary and say nothing at all about a read-only mount, an exhausted filesystem
or a restrictive ACL.
Adds the Upload flag group to interactsh-server and plumbs it through CLIServerOptions into server.Options. -upload forces authentication, joining -responder/-smb/-ftp/-ldap in the existing condition. Anonymous file hosting on a wildcard-TLS domain is not something to leave open by default, and the flag help says self-hosted only. The upload store is constructed before storage.New so it can install the OnEviction hook, and before the HTTP and FTP servers since both serve from its root. When no -ftp-dir is given the FTP root is pointed at the upload root, which is what makes -ftp serve uploaded files with no extra config; when the operator has pinned both to different directories we warn rather than silently serving over HTTP only. On shutdown the storage is closed first: goburrow's cache.Close blocks until every removal callback has run, so all session deletions are queued by the time the upload store drains them. Co-Authored-By: Claude <noreply@anthropic.com> The FTP root and the upload root have to be the same directory for hosted files to be reachable over ftp://, since the FTP file driver serves a real directory tree. They are compared as resolved directories rather than as the strings the operator typed: filepath.Abs and EvalSymlinks on both sides, falling back to the cleaned absolute form when a path does not exist yet, because -ftp-dir legitimately may not. Comparing the raw flag against an already absolutised root would report a mismatch for "-ud ./shared -ftp-dir ./shared", for a trailing slash, for a /./ segment and for a symlink -- four spellings of one directory -- and warning about a configuration that demonstrably works teaches the operator to ignore the warning that matters. The answer is recorded in Options.FTPServesUploads rather than recomputed, so that capability advertisement can be driven from it rather than from the -ftp flag alone. A genuine mismatch is reported with gologger.Error, not Warning: gologger orders LevelWarning above LevelInfo and filters on level <= maxLevel, so a warning is invisible unless -debug is passed, and this condition silently disables a capability the server would otherwise advertise.
POST /upload accepts JSON with base64 file bodies, authenticated by the correlation-id and secret-key pair -- the same ownership proof RemoveID requires, so only the client that owns a session can attach files to it. The route is registered on the mux rather than under "/", so it never passes through the logger middleware; that middleware dumps whole requests into interaction records, which for an upload endpoint would mean storing and re-encrypting every uploaded file. It is registered even when uploads are disabled, so that a request to a server without -upload is answered with 501 instead of falling through to exactly that path. Clients use the 501 as the signal to stop rather than blindly posting files at a server that will quietly swallow them. JSON with base64 rather than multipart: it matches every other endpoint, needs no new dependency, and at five 1MB files the 33% overhead is immaterial. Everything is decoded and validated before storage is touched, so a bad file part way through a batch cannot leave a session half-populated. The writes themselves run inside UpdateUploads, under the correlation-id lock, so the per-session quota check and the commit are atomic against a concurrent upload for the same session. Re-uploading a name replaces it and reuses its slot rather than consuming another. Status codes are distinct enough for the client to act on: 501 disabled, 404 unknown session, 403 wrong secret, 413 over a size or count limit, 507 server capacity exhausted, 400 for anything malformed. Registration now answers with RegisterResponse carrying a Capabilities block, so a client learns whether uploads are available, and the limits, at registration rather than by trial and error. The message field keeps its exact previous value, which older clients match on, and an older server simply yields a nil Capabilities. Co-Authored-By: Claude <noreply@anthropic.com> A request to /upload that no legitimate client could have sent is a target poking at the endpoint, so it is recorded as an interaction rather than being swallowed by the 401 or the 501. Nothing legitimate arrives there to confuse it with: the client reads the advertised capabilities and refuses to send when uploads are off, and -upload forces -auth with a randomly generated token, so a target cannot authenticate. The request body is summarised as its length rather than stored -- it is attacker-controlled and may be megabytes, which is the whole reason this route stays off the logger middleware. A 256KB probe retains ~240 bytes. Authenticated uploads are deliberately not recorded. They are the operator's own traffic, and filing them as interactions would attribute the operator's actions to the target, which is a false positive in the evidence rather than just noise. That is why the token check sits in uploadHandler rather than authMiddleware: the middleware writes its 401 and returns, leaving nowhere to record from. It also keeps the check reachable from tests, which call uploadHandler directly and never assemble the middleware chain. extractCorrelationID moves here from the file-serving commit, since this is now the first caller: recordUploadProbe has to resolve a session before it can file anything, and drops the interaction when the host carries no correlation id, because handleInteraction slices one out of uniqueID unconditionally. The FTP capability is advertised from Options.FTPServesUploads rather than from the -ftp flag, so it answers the question the client is actually asking: whether an ftp:// URL for a hosted file is worth printing. Taking it from -ftp alone would advertise FTP on a server whose FTP root is a different directory from the upload root, and the client would print a payload URL that resolves to nothing -- the target follows it, gets a 550, and the operator reads the silence as "not vulnerable", which is indistinguishable from a target that is not vulnerable.
Files uploaded against a correlation id are now reachable at http(s)://<correlationID><nonce>.<domain>/f/<name>, and each fetch is recorded as an interaction so the tester sees the second stage fire. The route sits outside the logger middleware and records the interaction itself. Routing it through the logger instead would copy the response body into Interaction.RawResponse, which is then JSON-marshalled and appended to the session buffer -- a buffer with no cap in memory mode. jsoniter escapes each invalid UTF-8 byte as �, so a fetch of a 512KiB file of 0xff would retain roughly 3MB, from an unauthenticated GET, and the retained copy is mangled by that escaping anyway. TestServeUploadedFileElidesBody measures 2,935 bytes retained across five such fetches. Staying off defaultHandler also avoids three ways it would have been shadowed, each covered by a regression test: -dhr returns early for every request, the .json and .xml suffix branches would swallow payload.xml -- precisely the XXE case -- and -dr header injection could have stripped the forced Content-Type. Responses are always application/octet-stream with an attachment disposition and nosniff. DTD, XSLT and JNDI consumers ignore content type, so nothing is lost for the intended use, while the server never renders client-supplied HTML or SVG on its own domain. A file is only served to the session that owns it: the correlation id comes from the Host header via extractCorrelationID, which mirrors the logger's sliding-window extraction so serving and recording always agree on the session; TestExtractCorrelationIDMatchesLogger drives both implementations from one table so they cannot drift. The metadata record is consulted before touching disk, and the name is re-validated against the allowlist. Co-Authored-By: Claude <noreply@anthropic.com> Recording covers every exit, not only the successful one, through a single deferred call. A miss is evidence too: it is how the operator separates "the target never fetched the payload" from "the target asked for a name I am not hosting", or from a fetch arriving after the file expired. One call site rather than one per return, because this handler has six early exits and a seventh added later must not be able to drop the record silently. A hostedFetchRecorder passes writes through to the real ResponseWriter while noting the status and the bytes written, so the stored record states what was actually sent rather than assuming: ServeContent answers a conditional request with 304 and a ranged one with 206, and a record claiming 200 with the full length would assert a delivery that never happened. Wrapping the writer costs the io.ReaderFrom fast path in ServeContent's copy loop, which does not matter at the 1MiB default file cap. Two cases stay unrecorded because neither can be delivered: a host carrying no correlation id has nothing to be filed under, and a session that has left the cache has no bucket and no client polling it. So a fetch after deregistration is unrecoverable, while one after the file expired is recorded, the session outliving the file. Stats.Http is incremented where the interaction is recorded, so /metrics and the interaction stream cannot disagree about what arrived. Only the /f/ subtree is handled: CutPrefix rejects any other path instead of reading it as a file name. A bare /f is redirected to /f/ by ServeMux before this handler runs, so it is not recorded; the request that follows the redirect is.
Two changes to make FTP a safe way to serve hosted files. NopAuth accepts any credentials, and NopDriver forwards ListDir to the real file driver. Once the upload root is also the FTP root, that combination lets an anonymous client enumerate every correlation id that currently has hosted files and then walk into each one. Sessions live under a single .interactsh-user-uploads directory, so ListDir closes that off with two rules: that directory never lists its own contents, and it is filtered out of the root listing so it cannot be discovered in the first place. Both go through one path.Clean-based helper, so the //, /./ and /x/../ spellings cannot slip past, and the same guard covers NLST, MLSD and STAT, which all route through ListDir. A client that knows its own correlation id can still list inside it, and RETR by full path is untouched. Deliberately not a blanket refusal on the root: -ftp-dir is documented as listing the operator's own directory in read-only mode, and refusing the root silently broke that for every -ftp user, whether or not -upload was enabled. FTP interactions were all stored under options.Token, the shared bucket that pollHandler fans out to every authenticated client. That is reasonable for connection noise, but a fetch of one session's hosted DTD would be reported to everybody and attributed to nobody. The download hooks now derive the correlation id from the path segment inside .interactsh-user-uploads, verify it names a live session with uploads, and record against that session instead. The path is cleaned first, so traversal can only resolve to the session it actually points at, and a path anywhere else under the FTP root is the operator's rather than ours and is never attributed. Everything unattributable -- logins, directory changes, downloads outside any session -- keeps its existing behaviour. Co-Authored-By: Claude <noreply@anthropic.com>
performRegistration now decodes the typed RegisterResponse and stores the advertised capabilities, so the client knows whether the server hosts files, and its limits, before trying. Capabilities live in an atomic.Value because the keep-alive goroutine re-registers periodically. The check on the message field is unchanged, so behaviour against an older server is identical and a missing capabilities block simply reads as "unknown". UploadFiles targets only the server the client registered with. A Client holds one correlation id, registered with whichever server answered first, so the rest of -s never saw it and would reject the upload. Posting to them anyway would be worse than useless: a server without -upload has no route for the request, so it falls through to the catch-all handler that records whole requests as interactions, and the file would end up stored there. For the same reason the client fails closed. A 501, 404 or 405 is reported as ErrUploadUnsupported rather than retried or ignored, and a server that has advertised no upload support is not contacted at all. Uploads refuse the plaintext HTTP fallback that registration is allowed to use, since the request carries both the file and the session secret key. Loopback is exempt so local testing still works. Files are validated locally first -- exists, regular, non-empty, within the advertised size and count limits, name acceptable to the server, no two paths sharing a basename -- so mistakes surface immediately with a clear message instead of as a 400. Co-Authored-By: Claude <noreply@anthropic.com> ErrUploadUnsupported is declared with errors.New rather than errkit.New: errkit compares errors by message, so errors.Is against an errkit sentinel matches anything whose message contains it, in either direction. A plain error keeps the comparison exact, which matters as soon as a more specific sentinel is built on top of this one.
interactsh-client -file evil.dtd uploads the file to the registered server and prints the URL a target should fetch, alongside the usual payload URLs. The flag uses goflags.StringSliceOptions rather than the FileCommaSeparatedStringSliceOptions used by -match and -filter: that variant reads the named file and splits its contents, which here would turn a DTD into a list of filenames. Short name is -fl because -f is filter. All files share a single payload host, so the target performs one DNS lookup and the output stays consistent; only the correlation id prefix is significant to the server, so any nonce works. The ftp:// URL is printed only when the server advertises an FTP listener, since otherwise it would never connect. Upload failure is fatal. The user asked to host a payload, and continuing without it yields a confusing run where no interaction ever arrives; a server without -upload gets a message naming the flag it needs. Co-Authored-By: Claude <noreply@anthropic.com>
Three fixes found by running the feature end to end. deregisterHandler now deletes the session's files synchronously. The cache eviction hook reached through RemoveID only enqueues the directory, leaving a window in which a client that had just deregistered could still fetch its own hosted files. Blocking is safe in the handler -- unlike the cache event goroutine, which is why the queue exists at all -- and the deletion is idempotent, so the queued removal that follows is a no-op. TestDeregisterRemovesFilesSynchronously pins the ordering: the fixture never starts the deleter goroutine, so a queued-only removal leaves the directory in place and fails the assertion. FTPFileURL carried the port from the payload host, which is the HTTP listener's port and says nothing about where FTP is bound -- against a server on :8080 the client printed ftp://host:8080/... which cannot connect. Any port is now dropped so the URL uses the FTP default. README gains a Client File Hosting section covering the flags, the URL shapes and the cleanup behaviour, and stating plainly that hosted files are readable by anyone who learns the correlation id. That id is deliberately leaked to the target, so it appears in the target's DNS logs and in passive DNS; a target can fetch the payload to fingerprint the tester, and that fetch shows up as an interaction. The self-hosted-only warning and the tmpfs caveat for the default upload directory are documented alongside. Verified end to end against a real server: upload, HTTP fetch with matching bytes and hardened headers, FTP fetch with matching bytes, empty FTP root listing, an interaction recorded for the fetch with the body elided and no file content reaching the client, and the session directory removed on deregistration. Co-Authored-By: Claude <noreply@anthropic.com>
Upload support is only advertised in the register response, so -file has to register before it can discover the server does not accept uploads. The failure paths then called gologger.Fatal(), which exits without unwinding, leaving a registered session behind on the server until the eviction TTL reclaimed it. Observed end to end: three consecutive `-file` runs against a server started without -upload drove the reported session count to 1, 2, 3 while nothing was actually connected. Wind the session down the same way the signal handler does instead of leaving it stranded: persist it when -session-file was requested, otherwise deregister. The previous behaviour was the worst of both for -session-file users, since the session was neither released nor written anywhere they could resume it from. Verified against a live server: the session count now stays flat across repeated failures, -session-file writes a resumable session and deliberately keeps it registered, and a successful upload run is unchanged. Co-Authored-By: Claude <noreply@anthropic.com>
The client registers with one server out of -s, so "Server does not accept file uploads" left the reader guessing which one when several were listed. Both upload failure paths now name the elected server. Election makes this worse than it looks: parseServerURLs walks -s under sliceutil.VisitRandom and keeps the first server that registers, and upload support cannot influence that choice because it is only advertised in the registration response. A list mixing upload and non-upload servers therefore succeeds or fails at random, at roughly 1/N per run, regardless of the order the servers were written in. Measured over 20 runs against two local servers where only one had -upload: 6/4 with the capable server last, 4/6 with it first. So when -s named more than one server the message also says how this one was chosen and what to do about it, rather than reading as "none of my servers support uploads" on the runs that happen to elect one that does not: Server http://127.0.0.1:8080 does not accept file uploads; it must be started with -upload (chosen at random from the 2 servers in -s, so this may differ between runs; pass a single server with -file) uploadFiles now takes the CLI options rather than three positional arguments, two of them adjacent strings that were easy to transpose. Documented in the README alongside the existing single-server note. Co-Authored-By: Claude <noreply@anthropic.com>
Both usage blocks were transcribed by hand and had drifted from the flag sets. Replaced with the verbatim output of interactsh-client -h and interactsh-server -h, with $HOME substituted back into the config paths. This adds the upload flags introduced here -- -fl/-file on the client and the whole UPLOAD group on the server -- and picks up flags that were already shipping but undocumented: -auth, -kai/-keep-alive-interval and -asn on the client, -i as the short form of -ip, and -ru/-rp for the redis backend on the server. Several descriptions and defaults were also stale. The alignment of the client's -asn line is not a typo: its description carries a leading space in the flag definition, so this is what users see. Co-Authored-By: Claude <noreply@anthropic.com>
-upload-directory read as "directory to store uploaded files", which does not warn the operator that interactsh treats part of that directory as its own and deletes from it -- on session end, on -upload-ttl expiry, and at startup, since upload metadata lives only in memory and anything left behind is an orphan. That matters most when the flag points at a directory the operator already uses, or is shared with -ftp-dir, which the feature actively encourages. The flag help now names .interactsh-user-uploads, and the README documents the layout, what is pruned and what is never touched, plus the FTP behaviour that follows from sharing a root: the uploads directory is hidden from listings and refuses to list itself, so the correlation ids with hosted files cannot be enumerated anonymously, while RETR of a known path still works. The -h block in the README is regenerated to match the binary. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
UpdateUploads runs the batch under the correlation id's lock and discards the metadata update when the callback fails, so the metadata side was transactional. The disk side was not: Save reserved quota, wrote a temp file and renamed it into place before returning, so by the time a later file in the same request failed the earlier ones were already committed and already charged. The result was a file on disk holding server-wide quota that nothing could reach -- serveUploadedFile consults the metadata first, so it answered 404 -- and that nothing could reclaim, because the retry read existingSize from the rolled-back metadata, saw 0, and asked for the space a second time. Reproduced with a 1024-byte cap and two 1000-byte files: 507, no metadata, a.bin on disk, 1000 bytes charged, and every later upload on every session refused until the session ended or -upload-ttl expired. The orphan stayed readable over FTP, which serves the filesystem with no metadata check. Two ordinary failures reach that path: the per-session file cap, checked cumulatively inside the callback while pre-validation only checks the request, and the global quota. Filesystem errors do too. Save splits along the seam it already had. Stage validates, reserves and writes the bytes under a temporary name, reachable by nobody. Commit renames a staged file into place, and the batch calls it only once every file has staged. Abort removes a staged file and releases its reservation, deferred so it runs on a panic as well. Save itself becomes Stage plus Commit, for single-file callers with nothing to unwind. Unwinding by deleting what had already been written would not have been enough: for a name that already existed the previous content is gone the moment the rename lands, so a compensating delete turns a leaked file into lost data. Staging avoids the question, since the original stays untouched until the whole batch is ready. One residual is accepted. If a Commit fails after earlier ones in the batch have succeeded, those are published while the metadata is discarded -- but that is a rename failing on a file just written into the same directory, so it means something severe. Abort unwinds what it safely can: a committed file that overwrote nothing is removed, one that replaced an existing file is left with a warning, so the exposure is one rename rather than a whole batch. Everything Abort touches resolves to <sessionsRoot>/<correlationID>/<validated name>, so unwinding one session can never reach another's files. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
-psf is a machine interface: payload hostnames, one per line, exactly -n of them, which is what a wrapper script substituting each line into a payload template relies on. Appending hosted-file URLs to it broke both halves of that contract at once -- the line count stopped matching -n, and "$line" became a full URL, so a template produced http://https://host/f/evil.dtd/ and a resolver lookup simply failed. Nothing reported an error, because the file still parsed as lines. Hosted-file URLs are worth having in a file, so they get their own: -fsf, -file-store-file, empty by default and enabled by being set, as -o is. Each file now holds one record type. Both files are newline-terminated. Previously the last record had no terminator, so wc -l reported one fewer record than the file held and a plain "while read line" loop dropped it -- discarding a payload silently. That predates this branch but the appended URLs changed which record got swallowed, and the files exist to be read by exactly that idiom. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The client mapped 404 and 405 to ErrUploadUnsupported, which the CLI renders as "does not accept file uploads; it must be started with -upload". But the server answers 404 for an unknown correlation id: a session that has been evicted or was never registered, whose remedy is to re-register, on a server that already has -upload and said so in its capabilities a moment earlier. The advice named a flag the operator had already passed. Nor did that mapping serve the case it looks written for. A server predating /upload has no route for it, so the request reaches the catch-all and defaultHandler answers 200 with HTML -- checked against a build of this branch's base commit -- never 404 or 405. So version skew fell through to the JSON decode and failed with `invalid character '<' looking for beginning of value`. Both are now handled where the information actually is. A server that advertised no capabilities at all predates the feature, so UploadFiles refuses before sending anything, with ErrUploadNotAdvertised and a message that says to upgrade the server. 404 and 405 fall through to the default branch, which reports the status and the server's own reason -- "404 Not Found: unknown correlation-id" -- which is true whatever the cause. 501 stays as a backstop for a server that advertised uploads and then refused them, reachable behind a mismatched load balancer. ErrUploadNotAdvertised wraps ErrUploadUnsupported, so a library caller asking only whether hosting is possible is unaffected; pkg/client is consumed that way. The wrap is one-way, which is why the base sentinel is a plain error rather than an errkit one: errkit compares by message, so a wrapped errkit sentinel would satisfy errors.Is in both directions and send a server with -upload merely switched off down the "upgrade the server" path. Absence of capabilities only means "predates the feature" when a registration actually completed. Resuming a session (-sf) re-registers, but the server refuses a duplicate registration while the session is still alive and that error is deliberately ignored, so no capabilities are ever received -- reading that nil as "the server advertised nothing" would make every resume refuse to upload, blaming a server that hosts files perfectly well. capabilitiesKnown records that an answer was received, so an unknown capability set attempts the upload and lets the response speak, while a known-empty one fails closed. The two tests covering the old mapping passed nil capabilities, so after this change they would have short-circuited before sending a request and still passed on the wrapped error, asserting nothing. They now advertise capabilities and check that the request was actually made. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The README's client output and recorded-interaction examples predated two format changes on this branch. The ftp:// payload URL now carries the uploads directory, since hosted files live one level below the FTP root, and the elided body records "delivered of hosted" rather than a single count -- so a conditional fetch reads "0 of 144" and a ranged one reads the bytes the range carried. Both were captured from a real client against a real server built from this branch rather than edited by hand, including the blank lines the client emits between the dumped request and the response. A sentence now explains the two counts, since "144 of 144" is otherwise a puzzle. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Whitespace only -- git diff -w is empty. Both files were already gofmt-unclean before this branch, but adding the upload options to CLIServerOptions and UploadedFile to the storage types widens those struct blocks, so more of the surrounding pre-existing fields fall out of alignment: server_options.go went from 18 gofmt-changed lines to 30. Formatting them keeps the files this branch edits clean without dragging in realignment of files it never touched. Left alone deliberately, since they are unrelated to this change and would only make the diff harder to review: pkg/server/acme/acme_certbot.go, pkg/server/acme/cert_reloader.go, pkg/server/http_server_test.go, pkg/server/responder_server.go, pkg/storage/storage_redis_test.go and pkg/storage/storagedb_test.go. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The library example only demonstrated polling for interactions, so nothing showed a consumer how to reach the feature this branch adds -- or that it has to negotiate for it. It now uploads a file, prints the http:// and ftp:// URLs a target would fetch, fetches one itself, and shows both fetches arriving in the poll callback, which is the whole point of hosting: the retrieval is the evidence. Hosting is optional, and the servers DefaultOptions points at deliberately do not offer it, so the example has to degrade rather than fail. It checks Capabilities() first and handles both refusals from UploadFiles separately, since ErrUploadNotAdvertised means "upgrade the server" while ErrUploadUnsupported means "start it with -upload" -- different remedies, and a consumer that conflates them tells its user the wrong thing. An unknown capability set, as a resumed session has, falls through to attempting the upload rather than assuming either way. The client variable is renamed from client to c so the package remains reachable for those exported errors; it was previously shadowed after assignment. README's library section now says hosting is a negotiated capability and names the four pieces of API involved. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
feat: client file hosting for second-stage OOB verification
feat/file-upload-hosting→ Target:devNote
Stacked on
fix/metrics-race-and-session-count(27faf5c). Merge that first, or review this difffrom
27faf5crather than fromdev.What it does
interactsh proves a callback happened. It cannot prove a second-stage callback, because those need the tester to host a file the target fetches first — an external DTD for XXE, an XSLT include, a JNDI stager. Today
-http-directoryand-ftp-dirare global and operator-managed, with nothing tied to a session.This adds an opt-in
-uploadmode: the client supplies files, the server hosts them against that client's correlation ID, and every fetch is recorded as an interaction, so the second stage shows up in the poll output like any other callback.The target fetches the DTD, that fetch arrives as an HTTP interaction, and the DTD's own callback arrives as a second one. Both land in the same client.
Surface area
Server endpoints
POST /uploadGET /f/<name>Server flags —
-upload(off by default, implies-auth),-ud/-upload-directory(temp dir),-umfs/-upload-max-file-size(1mb),-umf/-upload-max-files(5),-umts/-upload-max-total-size(1gb),-ut/-upload-ttl(24h).Client flags —
-fl/-file(repeatable),-fsf/-file-store-file(write the hosted-file URLs to a file, as-psfdoes for payload hostnames).On-disk layout —
<root>/.interactsh-user-uploads/<correlation-id>/<filename>. Everything under that one directory is interactsh's, deleted on session end, on-upload-ttlexpiry and at startup; the rest of the root is never touched, which is what makes sharing a root with-ftp-dirsafe.Registration now returns a
Capabilitiesblock so a client knows whether to attempt an upload at all. Themessagefield keeps its exact previous value for old clients.Constraints a reviewer should know
-uploadmust not be enabled on the publicoast.*fleet. Anonymous hosting on a domain with a valid wildcard certificate is a malware-staging magnet, and blocklists act on the registrable domain.Off by default, stated in the flag help and the README.
-uploadwith-redis-urlis a startup error. Hosted bytes live on one instance's local disk and the capacity quota is an in-process counter, so with shared state peers would advertise files they do not have. Redis support is a clean follow-up, not a shim.-upload-directoryon real deployments.Security posture, in brief
application/octet-stream+Content-Disposition: attachment+nosniff, so the server never renders client-supplied HTML on its own domain.^[A-Za-z0-9][A-Za-z0-9._-]{0,127}$), applied on upload and on serve; reads resolve through anos.Roothandle.RETRby known path still works.Testing
~140 new cases across
pkg/storage,pkg/server,pkg/clientand bothcmdpackages, plus a scripted end-to-end sweep of 36 checks run against real binaries built from the branch — hosting, FTP serving and listing, quota and file-cap failures, interaction recording for hits and misses, conditional and ranged fetches, and the client's error messages. All passing;-raceclean.Not covered: the DNS path (the harness addresses the server by IP) and TLS (
-saskips ACME).Compatibility
No existing flag changes behaviour. With
-uploadabsent the only observable differences are the extracapabilitieskey in the register response and/uploadanswering501.-psfkeeps its format — hosted-file URLs go to-fsf— and both files are now newline-terminated, which previously cost awhile readconsumer its last record.Follow-ups (not in this PR)
gologger.Warning()is invisible at default verbosity (LevelWarningsorts aboveLevelInfo), which affects every warning the server emits, not just these./s/static hosting persists whole response bodies into interaction records — the amplification this PR's/f/route was shaped to avoid.Misc note
The code was produced by Claude AI. Lots of manual testing went into validating and iterating over the code.