[proposal] protocol extension to support loadable stream filters

Hackers,

I'd like to introduce the concept of (dynamically loaded) stream
filters that would be used to wrap calls to send/recv by the FE/BE
protocol. The initial "StreamFilter" will be a zlib compression
filter. Yeah, I know it could just be added along-side (in the
same way as) the SSL code, in fact, having done just that is
precisely why I think a generic filter API is a good idea. The
SSL code could then be moved out into a loadable filter. At
first blush, this looks like it will entail the following:

1) Define a StreamFilter struct (see below) and a base implementation
that simply wraps send/recv calls. This base StreamFilter source
file will also contain a handful of utility functions to deal
with the loading/managing subsequent filters.

2) Add StreamFilter member to Port and PGconn, initialized with the
base StreamFilter.

3) Add support for a new ProtocolVersion in ProcessStartupPacket
NEGOTIATE_FILTER_CODE PG_PROTOCOL(1234,5680)
In addition, the client would send the name of the filter
for which it is requesting support along with some filter-
specific options...maybe a string like this,
"zlib:required=1;level=7"
where everything up to the ':' is the filter name, and
the remainder would be optional config options.

Q: Would this /require/ a bump of the protocol version?

Q: Is there some chunk of existing code that I could easily
use to parse the options bit of the filter request
string above? I looked at PQconninfoOption, but I'm
not sure that's what I want. All I want is for that
options string to be parsed into a simple structure
from which I can request values by name.

4) Add support to PQconnectPoll (and throughout client...) for
requesting and initializing a StreamFilter.

5) Create a pg_dlsym_ptr() function that returns a void*, instead
of a PGFunction like pg_dlsym.

6) The server will (attempt to) load the filter by name using
pg_dlopen( expand_dynamic_library_name( "sf_" + filterName ) )
or something along those lines. That library's create()
function will return a StreamFilter...

7) add support for StreamFilter(s) to pqsecure_read/write and
secure_read/write. If SSL were factored out as a streamFilter,
fe-secure.c and be-secure.c could really be factored away, and
the calls to [pq]secure_read/write would be replaced by direct
calls to StreamFilter->read/write.

Ok, I think that is a fair high-level description of what I'd
like to do. Comments? Questions? I don't find much time to work
on fun stuff during the week (or even to keep up with fun stuff...),
but I do expect to have time this weekend to wrap up a working
prototype of this. Hopefully, I'll have time knock out the jdbc
zlib client as well.

Thanks.
Brent

typedef struct _StreamFilter {
struct _StreamFilter* next; /* next StreamFilter. NULL iff last filter. */
Port* port;
PGconn conn;
int sock;
int version;
char* name;
void* filterOptions; /* type other than void* ??? */
/* filter-specific data...ptr to some-struct-or-another */
void* filterStateData;

// ...
PostgresPollingStatusType (*connectPoll)(struct _StreamFilter*);
struct _StreamFilter* (*create)(void);
int (*initialize)(struct _StreamFilter*);
int (*destroy)(struct _StreamFilter*);
int (*openServer)(struct _StreamFilter* filter, Port* port);
int (*openClient)(struct _StreamFilter* filter, PGconn* conn);
ssize_t (*read)(struct _StreamFilter* filter, void* ptr, size_t len);
ssize_t (*write)(struct _StreamFilter* filter, void* ptr, size_t len);
int (*close)(struct _StreamFilter* filter);
const char* (*getName)(struct _StreamFilter* filter);
char* (*getInfo)(struct _StreamFilter* filter);

} StreamFilter;

Brent Verner <brent(at)rcfile(dot)org> writes:
> I'd like to introduce the concept of (dynamically loaded) stream
> filters that would be used to wrap calls to send/recv by the FE/BE
> protocol.

I think the "dynamically loaded" part of that is going to be way more
headache than it's worth. You certainly don't get to have any help
from the database, for example, since you're not connected to it
at the time of the connection startup. I also wonder what happens when
the client and server disagree on the meaning of a filter name. It
would seem a lot safer to stick to the existing, low-tech, non dynamic
approach.

regards, tom lane

[2005-04-25 18:34] Tom Lane said:
| Brent Verner <brent(at)rcfile(dot)org> writes:
| > I'd like to introduce the concept of (dynamically loaded) stream
| > filters that would be used to wrap calls to send/recv by the FE/BE
| > protocol.

| You certainly don't get to have any help
| from the database, for example, since you're not connected to it
| at the time of the connection startup.

Right. The list of available filters would controlled at the
server level (in postgresql.conf). A snippet of how I envision
configuring the filters...at the moment, anyway. I suspect my
use of custom_variable_classes might be better done as a specific
enable_stream_filters option, but this is what I'm currently
working with...

#
# Define a custom_variable_class for each filter. A filter,
# $filterName, will be available iff $filterName.enable == true
#
custom_variable_classes = 'ssl, zlib, ...'

# see documentation of ssl filter for available options
ssl.enable = true
ssl.required = false

# see documentation of zlib filter for available options
zlib.enable = true
zlib.required = true
zlib.compression = 7

| I also wonder what happens when
| the client and server disagree on the meaning of a filter name.

How this is any different than saying "...when the client and
server disagree on the meaning of a ProtocolVersion.", which is
how ssl support is currently requested/negotiated? Either way,
client and server must agree on their behaviour. This doesn't
change, AFAICS, when requesting support for some feature/filter
by name. If the filter exists, an attempt will be made to
communicate through it, if that fails, the filter is not installed,
and the client ends up with a 'no support' response (or a disconnect
if the filter is required) and the client goes on without it.

What am I overlooking?

| It
| would seem a lot safer to stick to the existing, low-tech, non dynamic
| approach.

I still don't see what additional problems would be created by
using this StreamFilter API, so I'm going to march on and perhaps
the problems/difficulties will become apparent ;-)

I could see the benefit in having some built-in StreamFilters,
such as SSL (or zlib ;-)) that can't be replaced/overridden by
dlopen'd code, but I think having the ability to provide alternate
stream handling might be useful.

cheers.
brent

Brent Verner <brent(at)rcfile(dot)org> writes:
> | I also wonder what happens when
> | the client and server disagree on the meaning of a filter name.

> How this is any different than saying "...when the client and
> server disagree on the meaning of a ProtocolVersion.", which is
> how ssl support is currently requested/negotiated?

Nonsense. The ProtocolVersion stuff is documented, fixed, and the same
for every Postgres installation that understands a given version at all.
What you are proposing is an installation-dependent meaning of protocol
(because the meaning of any particular filter name is not standardized).
I cannot see any way that that is a good idea.

> What am I overlooking?

Cost/benefit. You have yet to offer even one reason why destandardizing
the protocol is a win.

I am also pretty concerned about the security risks involved. AFAICS
what you are proposing is that a user who hasn't even authenticated yet,
let alone proven himself to be a superuser, can ask the server to load
in code of uncertain provenance. The downsides of this are potentially
enormous, and the upsides are ... well ... you didn't actually offer any.

> I still don't see what additional problems would be created by
> using this StreamFilter API, so I'm going to march on and perhaps
> the problems/difficulties will become apparent ;-)

The stream-filter part is not a bad idea: that would definitely make it
easier to incorporate new capabilities into the standard protocol.
What I'm complaining about is the dynamic-loading part, and the
installation-dependent behavior. I see no real advantage to either of
those aspects, and lots of risks.

regards, tom lane

[2005-04-26 23:00] Tom Lane said:
| Brent Verner <brent(at)rcfile(dot)org> writes:
| > | I also wonder what happens when
| > | the client and server disagree on the meaning of a filter name.
|
| > How this is any different than saying "...when the client and
| > server disagree on the meaning of a ProtocolVersion.", which is
| > how ssl support is currently requested/negotiated?
|
| Nonsense. The ProtocolVersion stuff is documented, fixed, and the same
| for every Postgres installation that understands a given version at all.

Gotcha. Certainly, that is true of clients using libpq. I was
thinking of client libraries that (re)implement the protocol instead
of using libpq. In particular, the jdbc driver has its own idea of
what must be done to setup a connection with the NEGOTIATE_SSL_CODE
ProtocolVersion.

| What you are proposing is an installation-dependent meaning of protocol
| (because the meaning of any particular filter name is not standardized).

In a way, yes, I would like the capabilities of the protocol to be
installation-dependent. I'd like to be able to use a custom/local
filter without having to modify and rebuild my PG installation. The
use of named filters and dynamic loading was the only way I could
see to accomplish that.

| > What am I overlooking?
|
| Cost/benefit. You have yet to offer even one reason why destandardizing
| the protocol is a win.

That one could provide a new filter implementation w/o modifying
the internals of PG is the only benefit. If there's another way
to do this w/o dynamic loading I'd love to hear it.

| I am also pretty concerned about the security risks involved. AFAICS
| what you are proposing is that a user who hasn't even authenticated yet,
| let alone proven himself to be a superuser, can ask the server to load
| in code of uncertain provenance. The downsides of this are potentially
| enormous, and the upsides are ... well ... you didn't actually offer any.

Correct, the use of a filter is not limited to/by user. The admin
would have to enable a filter, which would then be available (or even
required) for any connection. The certainty of provenance seems to
be the same as that for a dynamically loaded PL.

| The stream-filter part is not a bad idea: that would definitely make it
| easier to incorporate new capabilities into the standard protocol.
| What I'm complaining about is the dynamic-loading part, and the
| installation-dependent behavior. I see no real advantage to either of
| those aspects, and lots of risks.

I'll rethink things w/o support for dynamic loading and installation-
dependent behaviour then send an updated proposal.

cheers.
Brent

Brent Verner <brent(at)rcfile(dot)org> writes:
> Would it be sane to recognize a specific PG_PROTOCOL_MAJOR
> to enter the filter-negotiation process? PG_PROTOCOL_MINOR
> would then be used to lookup and call a ptr to the filter's
> create() in CreateStreamFilter...

Looks reasonable enough to me ...

regards, tom lane

[2005-04-28 10:00] Tom Lane said:
| Brent Verner <brent(at)rcfile(dot)org> writes:
| > Would it be sane to recognize a specific PG_PROTOCOL_MAJOR
| > to enter the filter-negotiation process? PG_PROTOCOL_MINOR
| > would then be used to lookup and call a ptr to the filter's
| > create() in CreateStreamFilter...
|
| Looks reasonable enough to me ...

Now, the hard part...where should this code live? I'm thinking a
src/transport directory seems sensible.

StreamFilter.[ch] will contain the base StreamFilter along with
various utility functions. StreamFilter implementations will reside
in their own subdir.

src/include/transport/StreamFilter.h
src/transport/StreamFilter.c
src/transport/zlib/...
src/transport/ssl/...

Comments/ideas appreciated.

cheers.
b

Brent Verner <brent(at)rcfile(dot)org> writes:
> Now, the hard part...where should this code live? I'm thinking a
> src/transport directory seems sensible.

Our experience with trying to write single files to serve both server
and client sides has been, um, painful. I recommend you not try this.
My recommendation would be server-side code in src/backend/libpq/
and client-side code in src/interfaces/libpq/.

regards, tom lane

On Fri, Apr 29, 2005 at 10:17:44AM -0400, Tom Lane wrote:

> Our experience with trying to write single files to serve both server
> and client sides has been, um, painful. I recommend you not try this.

BTW, why not get rid of src/corba?

--
Alvaro Herrera (<alvherre[(at)]dcc(dot)uchile(dot)cl>)
"XML!" Exclaimed C++. "What are you doing here? You're not a programming
language."
"Tell that to the people who use me," said XML.

Alvaro Herrera wrote:
> BTW, why not get rid of src/corba?

Good question; I'll remove it from HEAD tomorrow, barring any objections.

-Neil