diff --git a/SECURITY.md b/SECURITY.md index 66d7acb..dd0d711 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -7,6 +7,25 @@ | 0.9.8 | :white_check_mark: | | < 0.9.8 | :x: | +## Hardening a deployment + +Configuration is where most of the risk lives. The security recommendations are +kept in [doc/html/securityen.html](doc/html/securityen.html), published at +: how to run the service, what the +ACLs have to cover, and the settings whose defaults are safe only until +something else is enabled alongside them. + +Read it before exposing a service. Recurring points from it: + +- Run unprivileged, never suid, and chroot where the platform allows. +- Name the internal and external interfaces explicitly, and limit sources and + destinations with ACLs rather than relying on defaults. +- Enabling IPv6 makes ACLs written in IPv4 incomplete: the same host is + reachable through an IPv4-mapped address, and the IPv6 loopback is an + address of its own. +- Anything that terminates or intercepts TLS holds key material and sees full + request URLs; both the key and the logs need protecting. + ## Reporting a Vulnerability Report to 3proxy@3proxy.org or via [GitHub security reporting](https://github.com/3proxy/3proxy/security) diff --git a/doc/html/securityen.html b/doc/html/securityen.html index 95451d9..773aa12 100644 --- a/doc/html/securityen.html +++ b/doc/html/securityen.html @@ -19,6 +19,45 @@ authentication is currently available.
  • Always limit connections to the internal network and localhost (to 127.0.0.1 and all interfaces) with ACLs. Be careful, because the BIND command in SOCKS requires the BIND method with the external interface IP address to be allowed. +
  • Services resolve IPv4 only unless told otherwise ('-4' is the default). Enabling +IPv6 with '-6', '-46' or '-64' makes every ACL written in IPv4 incomplete, because the +same host can be asked for in another way. A proxy that denies 127.0.0.1 but has IPv6 +enabled still reaches that host as '::ffff:127.0.0.1', and reaches the machine again as +'::1', which is a different address the IPv4 rule never mentioned. When IPv6 is enabled, +deny the mapped form '::ffff:0:0/96' as well unless it is needed, and deny the IPv6 +addresses that correspond to whatever the IPv4 rules protect: '::1' and '::' for the +local machine, 'fe80::/10' for link-local and 'fc00::/7' for unique local addresses. +Denying the IPv4 spelling alone is not enough. +
  • With '-46' or '-64' a name resolves to either family, so a target ACL that names +only one of a host's addresses does not limit that host. Names are resolved into +separate caches, and a name that resolves to an IPv6 address is only cached when +'nscache6' is configured. +
  • The 'admin' service hands out counters, the list of running services and a way to +trigger a configuration reload. Bind it to an internal interface, and put +authentication and an ACL in front of it. The '-s' option limits what the pages offer +but is not authentication. +
  • The 'echo' and 'data' operations of the 'http' command exist for testing. 'data' +returns a response of whatever size the request asks for, so a listener offering it to +anyone is a traffic amplifier. Do not configure them on a public service. +
  • 'ssl_server_ca_key' is the private key of a certificate authority that clients have +been told to trust. Anyone who obtains it can impersonate any site to those clients, so +protect it as a signing key and use a CA created for this purpose only, never one that +is trusted for anything else. Restrict the 'ssl_certcache' directory as well: it holds +the certificates generated from that key. +
  • Interception ('ssl_mitm') ends the guarantee the client believes it has. The full +URL of every request inside the tunnel, query string included, becomes visible to the +proxy and reaches the log, where a plain CONNECT would have shown only a host and a +port. Treat those logs accordingly. +
  • Certificates generated for interception by a build against wolfSSL carry no key +identifiers, because that library cannot generate certificate extensions, and a client +verifying strictly (OpenSSL 'x509_strict', which recent Python enables by default) +rejects them. Builds against OpenSSL generate them. Where they are missing, turning +verification off in the client removes the protection interception was supposed to +preserve; use an OpenSSL build instead. +
  • Regular expression rules ('pcre', 'pcre_rewrite') are matched without +authentication and do not replace ACLs. A rewrite that would change the method or the +destination of a request is ignored, because the destination was already authorized; +do not rely on one to redirect traffic.
  • Before 3proxy 0.8, always use nserver and nscache under Unix; otherwise, a DoS attack is possible with an unreachable DNS server (because gethostbyname will block other threads).
  • Keep logs in a secure location, because some confidential information from diff --git a/man/3proxy.cfg.5 b/man/3proxy.cfg.5 index 977f419..cf76cdb 100644 --- a/man/3proxy.cfg.5 +++ b/man/3proxy.cfg.5 @@ -132,7 +132,8 @@ change default server port to NUMBER Only resolve IPv6 addresses. IPv4 addresses are packed in IPv6 in IPV6_V6ONLY compatible way. .br .B -4 -Only resolve IPv4 addresses +Only resolve IPv4 addresses. This is the default: a service reaches an IPv6 +address only when told to with \fB-6\fR, \fB-46\fR or \fB-64\fR. .br .B -46 Prefer IPv4. Resolve IPv6 addresses if IPv4 address is not resolvable @@ -521,14 +522,18 @@ If not specified, nserver is used. The syntax is the same as for nserver. .br Cache \fI\fR records for name resolution (\fBnscache\fR for IPv4, \fBnscache6\fR for IPv6). The cache size should usually be large enough -(for example, 65536). +(for example, 65536). The two are separate: a name that resolves to an IPv6 +address, including one given with \fBnsrecord\fR, is only held when +\fBnscache6\fR is configured, and \fBnscache\fR does nothing for it. Both +caches are global rather than per-service. .br .BR nsrecord \fI\fR \fI\fR .br Adds static record to nscache. \fBnscache\fR must be enabled and must come -first, because the record is placed in the table it allocates, and +first, because the record is placed in the table it allocates - \fBnscache6\fR +for a record naming an IPv6 address - and \fBnserver\fR must be set as well: without it the system resolver is used and static records are never consulted. If 0.0.0.0 is used as a hostaddr host will never resolve, it can be used to diff --git a/src/ssllib.c b/src/ssllib.c index 5122e99..f9f5e6c 100644 --- a/src/ssllib.c +++ b/src/ssllib.c @@ -84,7 +84,11 @@ static int copy_ext(X509 *dst_cert, X509 *src_cert, int nid) } #ifndef WITH_WOLFSSL -static int add_ext(X509 *cert, int nid, const char *value) +/* issuer is the certificate the extension should describe as the issuer, + * which matters for an authority key identifier: it names the key that + * signs, not the key being signed. + */ +static int add_ext_issuer(X509 *cert, X509 *issuer, int nid, const char *value) { X509_EXTENSION *ex; X509V3_CTX ctx; @@ -92,10 +96,8 @@ static int add_ext(X509 *cert, int nid, const char *value) /* This sets the 'context' of the extensions. */ /* No configuration database */ X509V3_set_ctx_nodb(&ctx); - /* Issuer and subject certs: both the target since it is self signed, - * no request and no CRL - */ - X509V3_set_ctx(&ctx, cert, cert, NULL, NULL, 0); + /* No request and no CRL */ + X509V3_set_ctx(&ctx, issuer, cert, NULL, NULL, 0); /* value is char * prior to OpenSSL 1.1.0 */ ex = X509V3_EXT_conf_nid(NULL, &ctx, nid, (char *)value); if (!ex) @@ -105,6 +107,12 @@ static int add_ext(X509 *cert, int nid, const char *value) X509_EXTENSION_free(ex); return err > 0; } + +static int add_ext(X509 *cert, int nid, const char *value) +{ + /* Issuer and subject: both the target, for a self signed certificate */ + return add_ext_issuer(cert, cert, nid, value); +} #endif SSL_CERT ssl_copy_cert(SSL_CERT cert, SSL_CONFIG *config) @@ -199,6 +207,16 @@ SSL_CERT ssl_copy_cert(SSL_CERT cert, SSL_CONFIG *config) add_ext(dst_cert, NID_basic_constraints, "critical,CA:FALSE"); if(!copy_ext(dst_cert, src_cert, NID_ext_key_usage)) add_ext(dst_cert, NID_ext_key_usage, "serverAuth"); + /* A verifier following RFC 5280 strictly looks for the issuer through a + * key identifier and refuses a certificate carrying none: OpenSSL does + * with x509_strict, and Python has since 3.13. The identifiers are + * generated rather than copied, so they name the CA signing here + * instead of the one that signed upstream. keyid,issuer keeps working + * when the CA certificate has no subject key identifier of its own. + */ + add_ext(dst_cert, NID_subject_key_identifier, "hash"); + add_ext_issuer(dst_cert, config->CA_cert, NID_authority_key_identifier, + "keyid,issuer"); #else copy_ext(dst_cert, src_cert, NID_basic_constraints); copy_ext(dst_cert, src_cert, NID_ext_key_usage); diff --git a/tests/README.md b/tests/README.md index edee3db..02356a1 100644 --- a/tests/README.md +++ b/tests/README.md @@ -69,7 +69,9 @@ means to stand on its own should start with one - otherwise an earlier ## What is not covered yet -41 of the 112 configuration commands appear in a test. What follows is +41 of the 112 configuration commands appear in a test, and the count says +nothing about service options: the IPv6 case, for instance, exercises -4, +-6, -46, -64 and -i without adding a command to it. What follows is roughly the order worth working through: how much of the product a gap covers, and how much of a fixture it needs. @@ -103,11 +105,12 @@ ranges, time and weekday fields, and operation lists beyond the single `HTTP_CONNECT` used today. `weight` needs several parents and enough requests to see the split. -### IPv6 +### IPv6, what is left of it -Not one test binds or connects over `::1`, though the tree is full of -`#ifndef NOIPV6` and `extip` has an IPv6 CIDR-randomisation path of its own. -Most existing cases would work over IPv6 with the address parameterised. +`tests/cases/ipv6.py` covers listening on `::1`, proxying to and from it, +SOCKS with an IPv6 destination, rules naming an IPv6 address, and which +family each of `-4 -6 -46 -64` will use. Still open: `extip` with an IPv6 CIDR, whose +randomisation path has no coverage. ### Authentication @@ -156,7 +159,6 @@ takes effect, or that services come back. A request rewrite that changes the method or the authority is ignored, and the manual says so; a test that pinned the current behaviour would have to -change when that does. Certificates 3proxy generates for MITM carry no -Authority Key Identifier, so `tests/cases/ssl.py` verifies the chain without -strict checking - if that is fixed, the test should tighten rather than stay -as it is. +change when that does. An intercepted certificate is verified strictly where the build can +generate the key identifiers, and the case skips that one check on a wolfSSL +build, which cannot. If wolfSSL gains the ability, the skip should go. diff --git a/tests/cases/ipv6.py b/tests/cases/ipv6.py new file mode 100644 index 0000000..4879eda --- /dev/null +++ b/tests/cases/ipv6.py @@ -0,0 +1,224 @@ +"""IPv6: listening on it, reaching it, and the rules that mention it. + +A service resolves IPv4 only unless told otherwise, so the proxies that are +meant to reach IPv6 carry a family flag. Names resolving to IPv6 need +nscache6: nscache holds the IPv4 side and nothing else. +""" + + +def run(t): + if not t.has_ipv6(): + t.skip("IPv6 (this machine has no IPv6 loopback)") + return + + origin = t.free_port() + v6proxy = t.free_port() + mixed = t.free_port() + v4only = t.free_port() + socks6 = t.free_port() + + t.start("ipv6", f""" + log + auth iponly + allow * + http * /echo* echo + http * /data data + httpsrv -p{origin} -i::1 + + # reached over IPv6, and allowed to reach IPv6 + flush + auth iponly + allow * + proxy -p{v6proxy} -i::1 -6 + + # reached over IPv4, still able to reach IPv6 + flush + auth iponly + allow * + proxy -p{mixed} -6 + + # asked for IPv4 only, so an IPv6 destination is not for it + flush + auth iponly + allow * + proxy -p{v4only} -4 + + flush + auth iponly + allow * + socks -p{socks6} -6 + """, ports=[("::1", origin), ("::1", v6proxy), mixed, v4only, socks6]) + + url = f"http://[::1]:{origin}/echo" + + # --- listening on IPv6 ------------------------------------------------- + r = t.http(url) + t.eq(200, r.status, "a service bound to ::1 answers over IPv6") + t.contains(r, "peer.addr=::1", "the client is seen as an IPv6 address") + t.contains(r, "path=/echo", "and the request arrives intact") + + # the Host header carries the address in brackets, and a rule matching + # any host still matches it + t.contains(r, "host=[::1]", "the host header keeps its brackets") + + # --- proxying over IPv6 ------------------------------------------------- + r = t.http(url, proxy=f"[::1]:{v6proxy}") + t.eq(200, r.status, "a proxy reached over IPv6 serves an IPv6 destination") + t.contains(r, "peer.addr=::1", "the proxy connects from IPv6 as well") + + t.eq(20000, t.http(f"http://[::1]:{origin}/data?size=20000", + proxy=f"[::1]:{v6proxy}").length, + "a body passes over IPv6") + + t.eq(200, t.http(url, proxy=f"[::1]:{v6proxy}", tunnel=True).status, + "CONNECT works over IPv6") + + # --- across the two families -------------------------------------------- + r = t.http(url, proxy=f"127.0.0.1:{mixed}") + t.eq(200, r.status, "a client on IPv4 can be given an IPv6 destination") + t.contains(r, "peer.addr=::1", "and the far side is still reached over IPv6") + + # a service told to use one family stays in it + t.ne(200, t.http(url, proxy=f"127.0.0.1:{v4only}").status, + "a service asked for IPv4 only refuses an IPv6 destination") + + # --- SOCKS with an IPv6 destination ------------------------------------- + r = t.socks_http(f"127.0.0.1:{socks6}", url) + t.eq(200, r.status, "SOCKS5 carries an IPv6 destination address") + t.contains(r, "peer.addr=::1", "which is reached over IPv6") + + # --- which family a service will use -------------------------------------- + # -46 and -64 both reach either family; -4 and -6 are each restricted to + # one; and nothing said means -46. + v4origin = t.free_port() + flags = {"nothing said": "", "-4": "-4", "-6": "-6", "-46": "-46", "-64": "-64"} + family_ports = {name: t.free_port() for name in flags} + sections = [f""" + flush + auth iponly + allow * + proxy -p{family_ports[name]} {flag}""" for name, flag in flags.items()] + t.start("ipv6_family", f""" + log + auth iponly + allow * + http * /echo* echo + httpsrv -p{v4origin} + {"".join(sections)} + """, ports=[v4origin] + list(family_ports.values())) + + expected = { + "nothing said": (200, None), # -4 is the default + "-4": (200, None), + "-6": (None, 200), + "-46": (200, 200), + "-64": (200, 200), + } + for name, port in family_ports.items(): + want4, want6 = expected[name] + got4 = t.http(f"http://127.0.0.1:{v4origin}/echo", proxy=f"127.0.0.1:{port}").status + got6 = t.http(url, proxy=f"127.0.0.1:{port}").status + if want4 == 200: + t.eq(200, got4, f"{name}: an IPv4 destination is reached") + else: + t.ne(200, got4, f"{name}: an IPv4 destination is refused") + if want6 == 200: + t.eq(200, got6, f"{name}: an IPv6 destination is reached") + else: + t.ne(200, got6, f"{name}: an IPv6 destination is refused") + + # --- a name that resolves to an IPv6 address ------------------------------ + # The two caches are separate, and the record is only kept in the one + # that matches the address family. + # separate processes: the caches belong to the process, not the service, + # so one section configuring nscache6 would answer for the other too + with_cache6 = t.free_port() + without = t.free_port() + t.start("ipv6_names", f""" + log + flush + nserver 127.0.0.1 + nscache6 1024 + nsrecord v6.test ::1 + auth iponly + allow * + proxy -p{with_cache6} -6 + """, ports=[with_cache6]) + t.start("ipv6_names_nocache", f""" + log + flush + nserver 127.0.0.1 + nsrecord v6.test ::1 + auth iponly + allow * + proxy -p{without} -6 + """, ports=[without]) + + t.eq(200, t.http(f"http://v6.test:{origin}/echo", + proxy=f"127.0.0.1:{with_cache6}").status, + "a name kept in nscache6 resolves to its IPv6 address") + t.ne(200, t.http(f"http://v6.test:{origin}/echo", + proxy=f"127.0.0.1:{without}").status, + "the same record without nscache6 is not there to be found") + + # --- an address has more than one spelling -------------------------------- + # Denying the IPv4 form does not deny the same host asked for as an + # IPv4-mapped address, nor the IPv6 loopback, which is why the security + # notes say to deny all of them. Both halves are checked so a change in + # either direction is noticed. + partial = t.free_port() + complete = t.free_port() + t.start("ipv6_deny", f""" + log + flush + auth iponly + deny * * 127.0.0.1 + allow * + proxy -p{partial} -46 + + flush + auth iponly + deny * * 127.0.0.1 + deny * * ::1 + deny * * ::ffff:127.0.0.1 + allow * + proxy -p{complete} -46 + """, ports=[partial, complete]) + + v4url = f"http://127.0.0.1:{v4origin}/echo" + mapped = f"http://[::ffff:127.0.0.1]:{v4origin}/echo" + + t.ne(200, t.http(v4url, proxy=f"127.0.0.1:{partial}").status, + "denying 127.0.0.1 denies the address as written") + t.eq(200, t.http(mapped, proxy=f"127.0.0.1:{partial}").status, + "but the same host asked for as ::ffff:127.0.0.1 is still reached") + t.eq(200, t.http(url, proxy=f"127.0.0.1:{partial}").status, + "and so is ::1, which the rule never mentioned") + + t.ne(200, t.http(v4url, proxy=f"127.0.0.1:{complete}").status, + "naming every spelling denies the plain address") + t.ne(200, t.http(mapped, proxy=f"127.0.0.1:{complete}").status, + "and the mapped one") + t.ne(200, t.http(url, proxy=f"127.0.0.1:{complete}").status, + "and the IPv6 loopback") + + # --- rules that name addresses ------------------------------------------ + allowed = t.free_port() + refused = t.free_port() + t.start("ipv6_rules", f""" + log + flush + auth iponly + allow * ::1 + proxy -p{allowed} -i::1 -6 + + flush + auth iponly + allow * 127.0.0.1 + proxy -p{refused} -i::1 -6 + """, ports=[("::1", allowed), ("::1", refused)]) + + t.eq(200, t.http(url, proxy=f"[::1]:{allowed}").status, + "a rule naming ::1 admits an IPv6 client") + t.ne(200, t.http(url, proxy=f"[::1]:{refused}").status, + "a rule naming only an IPv4 address does not") diff --git a/tests/cases/ssl.py b/tests/cases/ssl.py index 5118999..b4c1d2e 100644 --- a/tests/cases/ssl.py +++ b/tests/cases/ssl.py @@ -162,13 +162,21 @@ def run(t): # what the machine running the tests puts in its hosts file. target = f"https://intercepted.test:{origin}/secret/page" - # The client trusts our CA, which is what signs the spoofed certificate. - # Verification is not strict: 3proxy issues those certificates without an - # Authority Key Identifier, which Python rejects under its 3.13 defaults. - # The spoofed certificate names the upstream host rather than the one - # asked for, so the chain is checked but the name is not. - r = t.https(target, proxy=f"127.0.0.1:{mitm}", ca=certs.ca, strict=False, + # The client trusts our CA, which is what signs the spoofed certificate, + # and checks it the way a current client does. The certificate names the + # upstream host rather than the one asked for, so the chain is verified + # but the name is not. + r = t.https(target, proxy=f"127.0.0.1:{mitm}", ca=certs.ca, verify_name=False) + if r.status is None and "Authority Key Identifier" in (r.error or ""): + # A build against wolfSSL cannot generate certificate extensions, + # so the identifiers a strict verifier looks for are absent there. + t.skip("strict verification of an intercepted certificate " + "(this build cannot generate the key identifiers)") + r = t.https(target, proxy=f"127.0.0.1:{mitm}", ca=certs.ca, + strict=False, verify_name=False) + else: + t.ok("the intercepted certificate satisfies a strict verifier") t.eq(200, r.status, "MITM passes the request through") t.contains(r, "path=/secret/page", "the intercepted request reaches the origin") @@ -186,7 +194,7 @@ def run(t): # Without interception the same request is opaque: the proxy logs the # CONNECT target and nothing from inside the tunnel. before = len(proxies.output()) - r = t.https(target, proxy=f"127.0.0.1:{plain}", ca=certs.ca, strict=False, + r = t.https(target, proxy=f"127.0.0.1:{plain}", ca=certs.ca, verify_name=False) t.eq(200, r.status, "the plain proxy tunnels the same request") tunnelled = t.wait_output(proxies, "intercepted.test", since=before) diff --git a/tests/harness.py b/tests/harness.py index 69235bb..2984f3e 100644 --- a/tests/harness.py +++ b/tests/harness.py @@ -130,6 +130,20 @@ class Tester: # ---- servers ----------------------------------------------------- + def has_ipv6(self): + """Whether this machine can use the IPv6 loopback at all.""" + try: + sock = socket.socket(socket.AF_INET6, socket.SOCK_STREAM) + except OSError: + return False + try: + sock.bind(("::1", 0)) + return True + except OSError: + return False + finally: + sock.close() + def free_port(self): """A port nothing is listening on. Closed again before it is used, which is racy in principle and reliable enough in practice.""" @@ -150,7 +164,11 @@ class Tester: return path def start(self, name, config, ports=()): - """Write a configuration, run it, and wait for its ports to open.""" + """Write a configuration, run it, and wait for its ports to open. + + A port may be given as a number, or as (address, port) for a service + bound somewhere other than 127.0.0.1. + """ path = self.write_config(name, config) logfile = os.path.join(self.tmpdir, name + ".out") with open(logfile, "wb") as out: @@ -159,8 +177,9 @@ class Tester: server = Server(name, path, proc, logfile) self.servers.append(server) - for port in ports: - if not self.wait_port(port): + for entry in ports: + host, port = entry if isinstance(entry, tuple) else ("127.0.0.1", entry) + if not self.wait_port(port, host=host): code = proc.poll() if code is None: died = "the process is still running" @@ -181,11 +200,11 @@ class Tester: stderr=subprocess.STDOUT, timeout=15) return done.stdout.decode("utf-8", "replace") - def wait_port(self, port, timeout=5.0): + def wait_port(self, port, timeout=5.0, host="127.0.0.1"): deadline = time.time() + timeout while time.time() < deadline: try: - with socket.create_connection(("127.0.0.1", port), 0.25): + with socket.create_connection((host, port), 0.25): return True except OSError: time.sleep(0.02) @@ -242,7 +261,10 @@ class Tester: tunnel=tunnel) target = path if proxy and not tunnel: - target = f"http://{host}:{port}{path}" + # an address with colons goes back in brackets, or the + # absolute URI cannot be read + authority = f"[{host}]" if ":" in host else host + target = f"http://{authority}:{port}{path}" if body is not None and not isinstance(body, bytes): body = body.encode() conn.request(method, target, body=body, headers=headers) @@ -479,6 +501,8 @@ class Tester: if remote_dns: target = b"\x03" + bytes([len(host)]) + host.encode() + elif ":" in host: + target = b"\x04" + socket.inet_pton(socket.AF_INET6, host) else: target = b"\x01" + socket.inet_aton(socket.gethostbyname(host)) sock.sendall(b"\x05\x01\x00" + target + struct.pack("!H", port)) @@ -681,7 +705,8 @@ class Tester: try: if body is not None and not isinstance(body, bytes): body = body.encode() - conn.request(method, f"http://{host}:{port}{path}", body=body, + authority = f"[{host}]" if ":" in host else host + conn.request(method, f"http://{authority}:{port}{path}", body=body, headers=headers or {}) reply = conn.getresponse() return Response(reply.status, reply.read(), dict(reply.getheaders())) @@ -740,17 +765,28 @@ class Tester: @staticmethod def _hostport(value): + if value.startswith("["): + host, _, rest = value[1:].partition("]") + return host, int(rest[1:]) host, _, port = value.rpartition(":") return host or "127.0.0.1", int(port) @staticmethod def _split(url, default_port=80): + """Split a URL, understanding an address in brackets. + + The brackets are dropped: they belong to the URL, not to the address + a socket call or a certificate check wants. + """ for prefix in ("http://", "https://"): if url.startswith(prefix): url = url[len(prefix):] break authority, _, path = url.partition("/") - if ":" in authority: + if authority.startswith("["): + host, _, rest = authority[1:].partition("]") + port = rest[1:] if rest.startswith(":") else default_port + elif ":" in authority: host, _, port = authority.rpartition(":") else: host, port = authority, default_port