From b377791be7e65fd2f2ca84710cea815ad679a69f Mon Sep 17 00:00:00 2001 From: ed Date: Thu, 14 Mar 2024 19:07:35 +0100 Subject: [PATCH] support cidr notation for `--xff-src`, `--ipa`, `--*-ipa` the old `10.88.` syntax is still supported, translating to `10.88.0.0/16` also fix `--tftp-ipa` when optimizations are enabled --- copyparty/__main__.py | 8 +-- copyparty/ftpd.py | 2 +- copyparty/httpcli.py | 29 +++++----- copyparty/httpconn.py | 4 +- copyparty/httpsrv.py | 7 ++- copyparty/multicast.py | 2 +- copyparty/svchub.py | 7 +-- copyparty/tftpd.py | 16 +++-- copyparty/util.py | 58 ++++++++++++++++--- .../idp-authelia-traefik/cpp/copyparty.conf | 4 +- docs/examples/docker/idp/copyparty.conf | 5 +- docs/idp.md | 2 +- tests/util.py | 3 +- 13 files changed, 103 insertions(+), 44 deletions(-) diff --git a/copyparty/__main__.py b/copyparty/__main__.py index ae4b0e60..b7bd8e8c 100755 --- a/copyparty/__main__.py +++ b/copyparty/__main__.py @@ -907,8 +907,8 @@ def add_network(ap): ap2.add_argument("--ll", action="store_true", help="include link-local IPv4/IPv6 in mDNS replies, even if the NIC has routable IPs (breaks some mDNS clients)") ap2.add_argument("--rproxy", metavar="DEPTH", type=int, default=1, help="which ip to associate clients with; [\033[32m0\033[0m]=tcp, [\033[32m1\033[0m]=origin (first x-fwd, unsafe), [\033[32m2\033[0m]=outermost-proxy, [\033[32m3\033[0m]=second-proxy, [\033[32m-1\033[0m]=closest-proxy") ap2.add_argument("--xff-hdr", metavar="NAME", type=u, default="x-forwarded-for", help="if reverse-proxied, which http header to read the client's real ip from") - ap2.add_argument("--xff-src", metavar="IP", type=u, default="127., ::1", help="comma-separated list of trusted reverse-proxy IPs; only accept the real-ip header (\033[33m--xff-hdr\033[0m) if the incoming connection is from an IP starting with either of these. Can be disabled with [\033[32many\033[0m] if you are behind cloudflare (or similar) and are using \033[32m--xff-hdr=cf-connecting-ip\033[0m (or similar)") - ap2.add_argument("--ipa", metavar="PREFIX", type=u, default="", help="only accept connections from IP-addresses starting with \033[33mPREFIX\033[0m; example: [\033[32m127., 10.89., 192.168.\033[0m]") + ap2.add_argument("--xff-src", metavar="CIDR", type=u, default="127.0.0.0/8, ::1/128", help="comma-separated list of trusted reverse-proxy CIDRs; only accept the real-ip header (\033[33m--xff-hdr\033[0m) and IdP headers if the incoming connection is from an IP within either of these subnets. Specify [\033[32mlan\033[0m] to allow all LAN / private / non-internet IPs. Can be disabled with [\033[32many\033[0m] if you are behind cloudflare (or similar) and are using \033[32m--xff-hdr=cf-connecting-ip\033[0m (or similar)") + ap2.add_argument("--ipa", metavar="CIDR", type=u, default="", help="only accept connections from IP-addresses inside \033[33mCIDR\033[0m; examples: [\033[32mlan\033[0m], [\033[32m10.89.0.0/16, 192.168.33.0/24\033[0m]") ap2.add_argument("--rp-loc", metavar="PATH", type=u, default="", help="if reverse-proxying on a location instead of a dedicated domain/subdomain, provide the base location here; example: [\033[32m/foo/bar\033[0m]") if ANYWIN: ap2.add_argument("--reuseaddr", action="store_true", help="set reuseaddr on listening sockets on windows; allows rapid restart of copyparty at the expense of being able to accidentally start multiple instances") @@ -1006,7 +1006,7 @@ def add_ftp(ap): ap2.add_argument("--ftps", metavar="PORT", type=int, help="enable FTPS server on \033[33mPORT\033[0m, for example \033[32m3990") ap2.add_argument("--ftpv", action="store_true", help="verbose") ap2.add_argument("--ftp4", action="store_true", help="only listen on IPv4") - ap2.add_argument("--ftp-ipa", metavar="PFX", type=u, default="", help="only accept connections from IP-addresses starting with \033[33mPFX\033[0m; specify [\033[32many\033[0m] to disable inheriting \033[33m--ipa\033[0m. Example: [\033[32m127., 10.89., 192.168.\033[0m]") + ap2.add_argument("--ftp-ipa", metavar="CIDR", type=u, default="", help="only accept connections from IP-addresses inside \033[33mCIDR\033[0m; specify [\033[32many\033[0m] to disable inheriting \033[33m--ipa\033[0m. Examples: [\033[32mlan\033[0m] or [\033[32m10.89.0.0/16, 192.168.33.0/24\033[0m]") ap2.add_argument("--ftp-wt", metavar="SEC", type=int, default=7, help="grace period for resuming interrupted uploads (any client can write to any file last-modified more recently than \033[33mSEC\033[0m seconds ago)") ap2.add_argument("--ftp-nat", metavar="ADDR", type=u, help="the NAT address to use for passive connections") ap2.add_argument("--ftp-pr", metavar="P-P", type=u, help="the range of TCP ports to use for passive connections, for example \033[32m12000-13000") @@ -1029,7 +1029,7 @@ def add_tftp(ap): ap2.add_argument("--tftp-no-fast", action="store_true", help="debug: disable optimizations") ap2.add_argument("--tftp-lsf", metavar="PTN", type=u, default="\\.?(dir|ls)(\\.txt)?", help="return a directory listing if a file with this name is requested and it does not exist; defaults matches .ls, dir, .dir.txt, ls.txt, ...") ap2.add_argument("--tftp-nols", action="store_true", help="if someone tries to download a directory, return an error instead of showing its directory listing") - ap2.add_argument("--tftp-ipa", metavar="PFX", type=u, default="", help="only accept connections from IP-addresses starting with \033[33mPFX\033[0m; specify [\033[32many\033[0m] to disable inheriting \033[33m--ipa\033[0m. Example: [\033[32m127., 10.89., 192.168.\033[0m]") + ap2.add_argument("--tftp-ipa", metavar="CIDR", type=u, default="", help="only accept connections from IP-addresses inside \033[33mCIDR\033[0m; specify [\033[32many\033[0m] to disable inheriting \033[33m--ipa\033[0m. Examples: [\033[32mlan\033[0m] or [\033[32m10.89.0.0/16, 192.168.33.0/24\033[0m]") ap2.add_argument("--tftp-pr", metavar="P-P", type=u, help="the range of UDP ports to use for data transfer, for example \033[32m12000-13000") diff --git a/copyparty/ftpd.py b/copyparty/ftpd.py index 4d9c4879..4e349401 100644 --- a/copyparty/ftpd.py +++ b/copyparty/ftpd.py @@ -410,7 +410,7 @@ def __init__(self, conn: Any, server: Any, ioloop: Any = None) -> None: if cip.startswith("::ffff:"): cip = cip[7:] - if self.args.ftp_ipa_re and not self.args.ftp_ipa_re.match(cip): + if self.args.ftp_ipa_nm and not self.args.ftp_ipa_nm.map(cip): logging.warning("client rejected (--ftp-ipa): %s", cip) self.connected = False conn.close() diff --git a/copyparty/httpcli.py b/copyparty/httpcli.py index 15dd030d..48a41cb8 100644 --- a/copyparty/httpcli.py +++ b/copyparty/httpcli.py @@ -231,7 +231,7 @@ def run(self) -> bool: if self.is_banned(): return False - if self.args.ipa_re and not self.args.ipa_re.match(self.conn.addr[0]): + if self.conn.ipa_nm and not self.conn.ipa_nm.map(self.conn.addr[0]): self.log("client rejected (--ipa)", 3) self.terse_reply(b"", 500) return False @@ -311,8 +311,9 @@ def run(self) -> bool: self.log(t.format(self.args.rproxy, zso), c=3) pip = self.conn.addr[0] - if self.args.xff_re and not self.args.xff_re.match(pip): - t = 'got header "%s" from untrusted source "%s" claiming the true client ip is "%s" (raw value: "%s"); if you trust this, you must allowlist this proxy with "--xff-src=%s"' + xffs = self.conn.xff_nm + if xffs and not xffs.map(pip): + t = 'got header "%s" from untrusted source "%s" claiming the true client ip is "%s" (raw value: "%s"); if you trust this, you must allowlist this proxy with "--xff-src=%s"%s' if self.headers.get("cf-connecting-ip"): t += ' Note: if you are behind cloudflare, then this default header is not a good choice; please first make sure your local reverse-proxy (if any) does not allow non-cloudflare IPs from providing cf-* headers, and then add this additional global setting: "--xff-hdr=cf-connecting-ip"' else: @@ -321,8 +322,9 @@ def run(self) -> bool: ".".join(pip.split(".")[:2]) + "." if "." in pip else ":".join(pip.split(":")[:4]) + ":" - ) - self.log(t % (self.args.xff_hdr, pip, cli_ip, zso, zs), 3) + ) + "0.0/16" + zs2 = ' or "--xff-src=lan"' if self.conn.hsrv.xff_lan.map(pip) else "" + self.log(t % (self.args.xff_hdr, pip, cli_ip, zso, zs, zs2), 3) else: self.ip = cli_ip self.is_vproxied = bool(self.args.R) @@ -466,24 +468,22 @@ def run(self) -> bool: if not trusted_xff: pip = self.conn.addr[0] - trusted_xff = self.args.xff_re and self.args.xff_re.match(pip) - - # always require --xff-src with idp, but check against original (xff_src) rather than computed value (xff_re) to allow 'any' - trusted_xff_strict = trusted_xff and self.args.xff_src + xffs = self.conn.xff_nm + trusted_xff = xffs and xffs.map(pip) trusted_key = ( not self.args.idp_h_key ) or self.args.idp_h_key in self.headers - if trusted_key and trusted_xff_strict: + if trusted_key and trusted_xff: self.asrv.idp_checkin(self.conn.hsrv.broker, idp_usr, idp_grp) else: if not trusted_key: t = 'the idp-h-key header ("%s") is not present in the request; will NOT trust the other headers saying that the client\'s username is "%s" and group is "%s"' self.log(t % (self.args.idp_h_key, idp_usr, idp_grp), 3) - if not trusted_xff_strict: - t = 'got IdP headers from untrusted source "%s" claiming the client\'s username is "%s" and group is "%s"; if you trust this, you must allowlist this proxy with "--xff-src=%s"' + if not trusted_xff: + t = 'got IdP headers from untrusted source "%s" claiming the client\'s username is "%s" and group is "%s"; if you trust this, you must allowlist this proxy with "--xff-src=%s"%s' if not self.args.idp_h_key: t += " Note: you probably also want to specify --idp-h-key for additional security" @@ -492,8 +492,9 @@ def run(self) -> bool: ".".join(pip.split(".")[:2]) + "." if "." in pip else ":".join(pip.split(":")[:4]) + ":" - ) - self.log(t % (pip, idp_usr, idp_grp, zs), 3) + ) + "0.0/16" + zs2 = ' or "--xff-src=lan"' if self.conn.hsrv.xff_lan.map(pip) else "" + self.log(t % (pip, idp_usr, idp_grp, zs, zs2), 3) idp_usr = "*" idp_grp = "" diff --git a/copyparty/httpconn.py b/copyparty/httpconn.py index bf3690ab..543994ae 100644 --- a/copyparty/httpconn.py +++ b/copyparty/httpconn.py @@ -23,7 +23,7 @@ from .th_cli import ThumbCli from .th_srv import HAVE_PIL, HAVE_VIPS from .u2idx import U2idx -from .util import HMaccas, shut_socket +from .util import HMaccas, NetMap, shut_socket if True: # pylint: disable=using-constant-test from typing import Optional, Pattern, Union @@ -55,6 +55,8 @@ def __init__( self.E: EnvParams = self.args.E self.asrv: AuthSrv = hsrv.asrv # mypy404 self.u2fh: Util.FHC = hsrv.u2fh # mypy404 + self.ipa_nm: NetMap = hsrv.ipa_nm + self.xff_nm: NetMap = hsrv.xff_nm self.iphash: HMaccas = hsrv.broker.iphash self.bans: dict[str, int] = hsrv.bans self.aclose: dict[str, int] = hsrv.aclose diff --git a/copyparty/httpsrv.py b/copyparty/httpsrv.py index cade2df7..fd0854df 100644 --- a/copyparty/httpsrv.py +++ b/copyparty/httpsrv.py @@ -67,6 +67,7 @@ Netdev, NetMap, absreal, + build_netmap, ipnorm, min_ex, shut_socket, @@ -150,6 +151,10 @@ def __init__(self, broker: "BrokerCli", nid: Optional[int]) -> None: zs = os.path.join(self.E.mod, "web", "deps", "prism.js.gz") self.prism = os.path.exists(zs) + self.ipa_nm = build_netmap(self.args.ipa) + self.xff_nm = build_netmap(self.args.xff_src) + self.xff_lan = build_netmap("lan") + self.statics: set[str] = set() self._build_statics() @@ -199,7 +204,7 @@ def set_netdevs(self, netdevs: dict[str, Netdev]) -> None: for ip, _ in self.bound: ips.add(ip) - self.nm = NetMap(list(ips), netdevs) + self.nm = NetMap(list(ips), list(netdevs)) def start_threads(self, n: int) -> None: self.tp_nthr += n diff --git a/copyparty/multicast.py b/copyparty/multicast.py index 8547b1e6..483f6426 100644 --- a/copyparty/multicast.py +++ b/copyparty/multicast.py @@ -110,7 +110,7 @@ def create_servers(self) -> list[str]: ) ips = [x for x in ips if x not in ("::1", "127.0.0.1")] - ips = find_prefix(ips, netdevs) + ips = find_prefix(ips, list(netdevs)) on = self.on[:] off = self.off[:] diff --git a/copyparty/svchub.py b/copyparty/svchub.py index fdd43f99..e8c14a72 100644 --- a/copyparty/svchub.py +++ b/copyparty/svchub.py @@ -49,6 +49,7 @@ ODict, alltrace, ansi_re, + build_netmap, min_ex, mp, odfusion, @@ -481,10 +482,8 @@ def _process_config(self) -> bool: al.idp_h_grp = al.idp_h_grp.lower() al.idp_h_key = al.idp_h_key.lower() - al.xff_re = self._ipa2re(al.xff_src) - al.ipa_re = self._ipa2re(al.ipa) - al.ftp_ipa_re = self._ipa2re(al.ftp_ipa or al.ipa) - al.tftp_ipa_re = self._ipa2re(al.tftp_ipa or al.ipa) + al.ftp_ipa_nm = build_netmap(al.ftp_ipa or al.ipa) + al.tftp_ipa_nm = build_netmap(al.tftp_ipa or al.ipa) mte = ODict.fromkeys(DEF_MTE.split(","), True) al.mte = odfusion(mte, al.mte) diff --git a/copyparty/tftpd.py b/copyparty/tftpd.py index 7b09533a..191a0793 100644 --- a/copyparty/tftpd.py +++ b/copyparty/tftpd.py @@ -55,17 +55,16 @@ def noop(*a, **ka) -> None: def _serverInitial(self, pkt: Any, raddress: str, rport: int) -> bool: info("connection from %s:%s", raddress, rport) - ret = _orig_serverInitial(self, pkt, raddress, rport) - ptn = _hub[0].args.tftp_ipa_re - if ptn and not ptn.match(raddress): + ret = _sinitial[0](self, pkt, raddress, rport) + nm = _hub[0].args.tftp_ipa_nm + if nm and not nm.map(raddress): yeet("client rejected (--tftp-ipa): %s" % (raddress,)) return ret -# patch ipa-check into partftpd +# patch ipa-check into partftpd (part 1/2) _hub: list["SvcHub"] = [] -_orig_serverInitial = TftpStates.TftpServerState.serverInitial -TftpStates.TftpServerState.serverInitial = _serverInitial +_sinitial: list[Any] = [] class Tftpd(object): @@ -116,6 +115,11 @@ def __init__(self, hub: "SvcHub") -> None: for C in Cs: C.log.debug = noop + # patch ipa-check into partftpd (part 2/2) + _sinitial[:] = [] + _sinitial.append(TftpStates.TftpServerState.serverInitial) + TftpStates.TftpServerState.serverInitial = _serverInitial + # patch vfs into partftpy TftpContexts.open = self._open TftpStates.open = self._open diff --git a/copyparty/util.py b/copyparty/util.py index 890fee05..f9401790 100644 --- a/copyparty/util.py +++ b/copyparty/util.py @@ -559,20 +559,26 @@ def emit(self, record: logging.LogRecord) -> None: class NetMap(object): - def __init__(self, ips: list[str], netdevs: dict[str, Netdev]) -> None: + def __init__(self, ips: list[str], cidrs: list[str], keep_lo=False) -> None: + """ + ips: list of plain ipv4/ipv6 IPs, not cidr + cidrs: list of cidr-notation IPs (ip/prefix) + """ if "::" in ips: ips = [x for x in ips if x != "::"] + list( - [x.split("/")[0] for x in netdevs if ":" in x] + [x.split("/")[0] for x in cidrs if ":" in x] ) ips.append("0.0.0.0") if "0.0.0.0" in ips: ips = [x for x in ips if x != "0.0.0.0"] + list( - [x.split("/")[0] for x in netdevs if ":" not in x] + [x.split("/")[0] for x in cidrs if ":" not in x] ) - ips = [x for x in ips if x not in ("::1", "127.0.0.1")] - ips = find_prefix(ips, netdevs) + if not keep_lo: + ips = [x for x in ips if x not in ("::1", "127.0.0.1")] + + ips = find_prefix(ips, cidrs) self.cache: dict[str, str] = {} self.b2sip: dict[bytes, str] = {} @@ -589,6 +595,9 @@ def __init__(self, ips: list[str], netdevs: dict[str, Netdev]) -> None: self.bip.sort(reverse=True) def map(self, ip: str) -> str: + if ip.startswith("::ffff:"): + ip = ip[7:] + try: return self.cache[ip] except: @@ -1920,10 +1929,10 @@ def ipnorm(ip: str) -> str: return ip -def find_prefix(ips: list[str], netdevs: dict[str, Netdev]) -> list[str]: +def find_prefix(ips: list[str], cidrs: list[str]) -> list[str]: ret = [] for ip in ips: - hit = next((x for x in netdevs if x.startswith(ip + "/")), None) + hit = next((x for x in cidrs if x.startswith(ip + "/") or ip == x), None) if hit: ret.append(hit) return ret @@ -2317,6 +2326,41 @@ def list_ips() -> list[str]: return list(ret) +def build_netmap(csv: str): + csv = csv.lower().strip() + + if csv in ("any", "all", "no", ",", ""): + return None + + if csv in ("lan", "local", "private", "prvt"): + csv = "10.0.0.0/8, 172.16.0.0/12, 192.168.0.0/16, fd00::/8" # lan + csv += ", 169.254.0.0/16, fe80::/10" # link-local + csv += ", 127.0.0.0/8, ::1/128" # loopback + + srcs = [x.strip() for x in csv.split(",") if x.strip()] + cidrs = [] + for zs in srcs: + if not zs.endswith("."): + cidrs.append(zs) + continue + + # translate old syntax "172.19." => "172.19.0.0/16" + words = len(zs.rstrip(".").split(".")) + if words == 1: + zs += "0.0.0/8" + elif words == 2: + zs += "0.0/16" + elif words == 3: + zs += "0/24" + else: + raise Exception("invalid config value [%s]" % (zs,)) + + cidrs.append(zs) + + ips = [x.split("/")[0] for x in cidrs] + return NetMap(ips, cidrs, True) + + def yieldfile(fn: str) -> Generator[bytes, None, None]: with open(fsenc(fn), "rb", 512 * 1024) as f: while True: diff --git a/docs/examples/docker/idp-authelia-traefik/cpp/copyparty.conf b/docs/examples/docker/idp-authelia-traefik/cpp/copyparty.conf index ab253f36..6a0d6e44 100644 --- a/docs/examples/docker/idp-authelia-traefik/cpp/copyparty.conf +++ b/docs/examples/docker/idp-authelia-traefik/cpp/copyparty.conf @@ -24,8 +24,8 @@ # if we are confident that we got the docker-network config correct # (meaning copyparty is only accessible through traefik, and # traefik makes sure that all requests go through authelia), - # then disable the reverse-proxy source-ip safety check like this: - xff-src: any + # then accept X-Forwarded-For and IdP headers from any private IP: + xff-src: lan # enable IdP support by expecting username/groupname in # http-headers provided by the reverse-proxy; header "X-IdP-User" diff --git a/docs/examples/docker/idp/copyparty.conf b/docs/examples/docker/idp/copyparty.conf index 2670560a..35dd809d 100644 --- a/docs/examples/docker/idp/copyparty.conf +++ b/docs/examples/docker/idp/copyparty.conf @@ -31,7 +31,10 @@ # preventing malicious users from pretending to be the proxy; # pay attention to the warning message in the logs and then # adjust the following config option accordingly: - xff-src: 192.168. + xff-src: 192.168.0.0/16 + + # or just allow all LAN / private IPs (probably good enough): + xff-src: lan # an additional, optional security measure is to expect a # secret header name from the reverse-proxy; you can enable diff --git a/docs/idp.md b/docs/idp.md index c45ea830..6d27411b 100644 --- a/docs/idp.md +++ b/docs/idp.md @@ -4,4 +4,4 @@ to configure IdP from scratch, you must place copyparty behind a reverse-proxy w in the copyparty `[global]` config, specify which headers to read client info from; username is required (`idp-h-usr: X-Authooley-User`), group(s) are optional (`idp-h-grp: X-Authooley-Groups`) -* it is also required to specify the subnet that legit requests will be coming from, for example `--xff-src=10.88.` to allow 10.88.x.x, and it is recommended to configure the reverseproxy to include a secret header as proof that the other headers are also legit (and not smuggled in by a malicious client), telling copyparty the headername to expect with `idp-h-key: shangala-bangala` +* it is also required to specify the subnet that legit requests will be coming from, for example `--xff-src=10.88.0.0/24` to allow 10.88.x.x (or `--xff-src=lan` for all private IPs), and it is recommended to configure the reverseproxy to include a secret header as proof that the other headers are also legit (and not smuggled in by a malicious client), telling copyparty the headername to expect with `idp-h-key: shangala-bangala` diff --git a/tests/util.py b/tests/util.py index 3230b629..0fd52c0a 100644 --- a/tests/util.py +++ b/tests/util.py @@ -116,7 +116,7 @@ def __init__(self, a=None, v=None, c=None, **ka0): ex = "dotpart dotsrch no_dhash no_fastboot no_rescan no_sendfile no_voldump re_dhash plain_ip" ka.update(**{k: True for k in ex.split()}) - ex = "ah_cli ah_gen css_browser hist ipa_re js_browser no_forget no_hash no_idx nonsus_urls" + ex = "ah_cli ah_gen css_browser hist js_browser no_forget no_hash no_idx nonsus_urls" ka.update(**{k: None for k in ex.split()}) ex = "hash_mt srch_time u2abort u2j" @@ -242,6 +242,7 @@ def __init__(self, args, asrv, log, buf): self.freshen_pwd = 0.0 self.hsrv = VHttpSrv(args, asrv, log) self.ico = None + self.ipa_nm = None self.lf_url = None self.log_func = log self.log_src = "a"