On Mon, 2021-07-12 at 18:28 +0200, Magnus Hagander wrote: > Yeah, I have no problem being stricter than necessary, unless that > actually causes any interop problems. It's a lot worse to not be > strict enough..
Agreed. Haven't heard back from the HAProxy mailing list yet, so staying strict seems reasonable in the meantime. That could always be rolled back later. > > I've been thinking more about your earlier comment: > > > > > An interesting thing is what to do about > > > inet_server_addr/inet_server_port. That sort of loops back up to the > > > original question of where/how to expose the information about the > > > proxy in general (since right now it just logs). Right now you can > > > actually use inet_server_port() to see if the connection was proxied > > > (as long as it was over tcp). > > > > IMO these should return the "virtual" dst_addr/port, instead of > > exposing the physical connection information to the client. That way, > > if you intend for your proxy to be transparent, you're not advertising > > your network internals to connected clients. It also means that clients > > can reasonably expect to be able to reconnect to the addr:port that we > > give them, and prevents confusion if the proxy is using an address > > family that the client doesn't even support (e.g. the client is IPv4- > > only but the proxy connects via IPv6). > > That reasoning I think makes a lot of sense, especially with the > comment about being able to connect back to it. > > The question at that point extends to, would we also add extra > functions to get the data on the proxy connection itself? Maybe add a > inet_proxy_addr()/inet_proxy_port()? Better names? What's the intended use case? I have trouble viewing those as anything but information disclosure vectors, but I'm jaded. :) If the goal is to give a last-ditch debugging tool to someone whose proxy isn't behaving properly -- though I'd hope the proxy in question has its own ways to debug its behavior -- maybe they could be locked behind one of the pg_monitor roles, so that they're only available to someone who could get that information anyway? > PFA a patch that fixes the above errors, and changes > inet_server_addr()/inet_server_port(). Does not yet add anything to > receive the actual local port in this case. Looking good in local testing. I'm going to reread the spec with fresh eyes and do a full review pass, but this is shaping up nicely IMO. Something that I haven't thought about very hard yet is proxy authentication, but I think the simple IP authentication will be enough for a first version. For the Unix socket case, it looks like anyone currently relying on peer auth will need to switch to a unix_socket_group/_permissions model. For now, that sounds like a reasonable v1 restriction, though I think not being able to set the proxy socket's permissions separately from the "normal" one might lead to some complications in more advanced setups. --Jacob
