Thread

  1. libpq compression (part 3)

    Jacob Burroughs <jburroughs@instructure.com> — 2023-12-19T16:40:34Z

    Hello PG developers!
    
    I would like to introduce an updated take on libpq protocol-level
    compression, building off off the work in
    https://www.postgresql.org/message-id/aad16e41-b3f9-e89d-fa57-fb4c694bec25@postgrespro.ru
    and the followon work in
    https://www.postgresql.org/message-id/ABAA09C6-BB95-47A5-890D-90353533F9AC@yandex-team.ru
    along with all of the (nice and detailed) feedback and discussion therein.
    
    The first patch in the stack replaces `secure_read` and `secure_write`
    (and their frontend counterparts) with an "IO stream" abstraction,
    which should address a lot of the concerns from all parties around
    secure_read.  The fundamental idea of an IO stream is that it is a
    linked list of "IoStreamProcessor"s.  The "base" processor is the
    actual socket-backed API, and then SSL, GSSAPI, and compression can
    all add layers on top of that base that add functionality and rely on
    the layer below to read/write data.  This structure makes it easy to
    add compression on top of either plain or encrypted sockets through a
    unified, unconditional API, and also makes it easy for callers to use
    plain, plain-compressed, secure, and secure-compressed communication
    channels equivalently.
    
    The second patch is the refactored implementation of compression
    itself, with ZSTD support merged into the main patch because the
    configuration-level work is now already merged in master.  There was a
    good bit of rebasing, housekeeping, and bugfixing (including fixing
    lz4 by making it now be explicitly buffered inside ZStream), along
    with taking into account a lot of the feedback from this mailing list.
    I reworked the API to use the general compression processing types and
    methods  from `common/compression`.  This change also refactors the
    protocol to require the minimum amount of new message types and
    exchanges possible, while also enabling one-directional compression.
    The compression "handshaking" process now looks as follows:
    1. Client sends startup packet with `_pq_.libpq_compression = alg1;alg2`
    2. At this point, the server can immediately begin compressing packets
    to the client with any of the specified algorithms it supports if it
    so chooses
    3. Server includes `libpq_compression` in the automatically sent
    `ParameterStatus` messages before handshaking
    4. At this point, the client can immediately begin compressing packets
    to the server with any of the supported algorithms
    Both the server and client will prefer to compress using the first
    algorithm in their list that the other side supports, and we
    explicitly support `none` in the algorithm list.  This allows e.g. a
    client to use `none;gzip` and a server to use `zstd;gzip;lz4`, and
    then the client will not compress its data but the server will send
    its data using gzip.  Each side uses its own configured compression
    level (if set), since compression levels affect compression effort
    much more than decompression effort. This change also allows
    connections to succeed if compression was requested but not available
    (most of the time, I imagine that a client would prefer to just not
    use compression if the server doesn't support it; unlike SSL, it's a
    nice to have not an essential.  If a client application actually
    really *needs* compression, that can still be facilitated by
    explicitly checking the negotiated compression methods.)
    
    The third patch adds the traffic monitoring statistics that had been
    in the main patch of the previous series.  I've renamed them and
    changed slightly where to measure the actual raw network bytes and the
    "logical" protocol bytes, which also means this view can measure
    SSL/GSSAPI overhead (not that that's a particularly *important* thing
    to measure, but it's worth nothing what the view will actually
    measure.
    
    The fourth patch adds a TAP test that validates all of the compression
    methods and compression negotiation.  Ideally it would probably be
    part of patch #2, but it uses the monitoring from #3 to be able to
    validate that compression is actually working.
    
    The fifth patch is just a placeholder to allow running the test suite
    with compression maximally enabled to work out any kinks.
    
    I believe this patch series is ready for detailed review/testing, with
    one caveat: as can be seen here
    https://cirrus-ci.com/build/6732518292979712 , the build is passing on
    all platforms and all tests except for the primary SSL test on
    Windows.  After some network-level debugging, it appears that we are
    bumping into a version of the issues seen here
    https://www.postgresql.org/message-id/flat/CA%2BhUKG%2BOeoETZQ%3DQw5Ub5h3tmwQhBmDA%3DnuNO3KG%3DzWfUypFAw%40mail.gmail.com
    , where on Windows some SSL error messages end up getting swallowed up
    by the the process exiting and closing the socket with a RST rather
    than a nice clean shutdown.  I may have the cause/effect wrong here,
    but the issues appear before the compression is actually fully set up
    in the client used and would appear to be a side effect of timing
    differences and/or possibly size differences in the startup packet.
    Any pointers on how to resolve this would be appreciated.  It does
    reproduce on Windows fairly readily, though any one particular test
    still sometimes succeeds, and the relevant SSL connection failure
    message reliably shows up in Wireshark.
    
    Also please let me know if I have made any notable mailing list/patch
    etiquette/format/structure errors.  This is my first time submitting a
    patch to a mailing-list driven open source project and while I have
    tried to carefully review the various wiki guides I'm sure I didn't
    take everything in perfectly.
    
    Thanks,
    Jacob Burroughs
    
  2. Re: libpq compression (part 3)

    Jacob Burroughs <jburroughs@instructure.com> — 2023-12-19T17:02:41Z

    > I believe this patch series is ready for detailed review/testing, with one caveat: as can be seen here https://cirrus-ci.com/build/6732518292979712 , the build is passing on all platforms and all tests except for the primary SSL test on Windows.
    
    One correction: I apparently missed a kerberos timeout failure on
    freebsd with compression enabled (being color blind the checkmark and
    still running colors are awfully similar, and I misread what I saw).
    I haven't yet successfully reproduced that one, so I may or may not
    need some pointers to sort it out, but I think whatever it is the fix
    will be small enough that the patch overall is still reviewable.
    
    
    
    
  3. Re: libpq compression (part 3)

    Jacob Burroughs <jburroughs@instructure.com> — 2023-12-20T19:39:31Z

    > One correction: I apparently missed a kerberos timeout failure on
    > freebsd with compression enabled (being color blind the checkmark and
    > still running colors are awfully similar, and I misread what I saw).
    > I haven't yet successfully reproduced that one, so I may or may not
    > need some pointers to sort it out, but I think whatever it is the fix
    > will be small enough that the patch overall is still reviewable.
    
    I have now sorted out all of the non-Windows build issues (and removed
    my stray misguided attempt at fixing the Windows issue that I hadn't
    intended to post the first time around).  The build that is *actually*
    passing every platform except for the one Windows SSL test mentioned
    in my original message can be seen here:
    https://cirrus-ci.com/build/5924321042890752
    
  4. Re: libpq compression (part 3)

    Robert Haas <robertmhaas@gmail.com> — 2023-12-20T20:49:37Z

    On Tue, Dec 19, 2023 at 11:41 AM Jacob Burroughs
    <jburroughs@instructure.com> wrote:
    > The compression "handshaking" process now looks as follows:
    > 1. Client sends startup packet with `_pq_.libpq_compression = alg1;alg2`
    > 2. At this point, the server can immediately begin compressing packets
    > to the client with any of the specified algorithms it supports if it
    > so chooses
    > 3. Server includes `libpq_compression` in the automatically sent
    > `ParameterStatus` messages before handshaking
    > 4. At this point, the client can immediately begin compressing packets
    > to the server with any of the supported algorithms
    > Both the server and client will prefer to compress using the first
    > algorithm in their list that the other side supports, and we
    > explicitly support `none` in the algorithm list.  This allows e.g. a
    > client to use `none;gzip` and a server to use `zstd;gzip;lz4`, and
    > then the client will not compress its data but the server will send
    > its data using gzip.
    
    I'm having difficulty understanding the details of this handshaking
    algorithm from this description. It seems good that the handshake
    proceeds in each direction somewhat separately from the other, but I
    don't quite understand how the whole thing fits together. If the
    client tells the server that 'none,gzip' is supported, and I elect to
    start using gzip, how does the client know that I picked gzip rather
    than none? Are the compressed packets self-identifying?
    
    It's also slightly odd to me that the same parameter seems to specify
    both what we want to send, and what we're able to receive. I'm not
    really sure we should have separate parameters for those things, but I
    don't quite understand how this works without it. The "none" thing
    seems like a bit of a hack. It lets you say "I'd like to receive
    compressed data but send uncompressed data" ... but what about the
    reverse? How do you say "don't bother compressing what you receive
    from the server, but please lz4 everything you send to the server"? Or
    how do you say "use zstd from server to client, but lz4 from client to
    server"? It seems like you can't really say that kind of thing.
    
    What if we had, on the server side, a GUC saying what compression to
    accept and a GUC saying what compression to be willing to do? And then
    let the client request whatever it wants for each direction.
    
    > Also please let me know if I have made any notable mailing list/patch
    > etiquette/format/structure errors.  This is my first time submitting a
    > patch to a mailing-list driven open source project and while I have
    > tried to carefully review the various wiki guides I'm sure I didn't
    > take everything in perfectly.
    
    Seems fine to me.
    
    -- 
    Robert Haas
    EDB: http://www.enterprisedb.com
    
    
    
    
  5. Re: libpq compression (part 3)

    Jacob Burroughs <jburroughs@instructure.com> — 2023-12-20T21:48:13Z

    > I'm having difficulty understanding the details of this handshaking
    > algorithm from this description. It seems good that the handshake
    > proceeds in each direction somewhat separately from the other, but I
    > don't quite understand how the whole thing fits together. If the
    > client tells the server that 'none,gzip' is supported, and I elect to
    > start using gzip, how does the client know that I picked gzip rather
    > than none? Are the compressed packets self-identifying?
    
    I agree I could have spelled this out more clearly.  I forgot to
    mention that I added a byte to the CompressedMessage message type that
    specifies the chosen algorithm.  So if the server receives
    'none,gzip', it can either keep sending uncompressed regular messages,
    or it can compress them in CompressedMessage packets which now look
    like "z{len}{format}{data}" (format just being a member of the
    pg_compress_algorithm enum, so `1` in the case of gzip).  Overall the
    intention is that both the client and the server can just start
    sending CompressedMessages once they receive the list of ones other
    party supports without any negotiation or agreement needed and without
    an extra message type to first specify the compression algorithm. (One
    byte per message seemed to me like a reasonable overhead for the
    simplicity, but it wouldn't be hard to bring back SetCompressionMethod
    if we prefer.)
    
    > It's also slightly odd to me that the same parameter seems to specify
    > both what we want to send, and what we're able to receive. I'm not
    > really sure we should have separate parameters for those things, but I
    > don't quite understand how this works without it. The "none" thing
    > seems like a bit of a hack. It lets you say "I'd like to receive
    > compressed data but send uncompressed data" ... but what about the
    > reverse? How do you say "don't bother compressing what you receive
    > from the server, but please lz4 everything you send to the server"? Or
    > how do you say "use zstd from server to client, but lz4 from client to
    > server"? It seems like you can't really say that kind of thing.
    
    When I came up with the protocol I was imagining that basically both
    server admins and clients might want a decent bit more control over
    the compression they do rather than the decompression they do, since
    compression is generally much more computationally expensive than
    decompression.  Now that you point it out though, I don't think that
    actually makes that much sense.
    
    > What if we had, on the server side, a GUC saying what compression to
    > accept and a GUC saying what compression to be willing to do? And then
    > let the client request whatever it wants for each direction.
    
    Here's two proposals:
    Option 1:
    GUCs:
    libpq_compression (default "off")
    libpq_decompression (default "auto", which is defined to be equal to
    libpq_compression)
    Connection parameters:
    compression (default "off")
    decompression (default "auto", which is defined to be equal to compression)
    
    I think we would only send the decompression fields over the wire to
    the other side, to be used to filter for the first chosen compression
    field.  We would send the `_pq_.libpq_decompression` protocol
    extension even if only compression was enabled and not decompression
    so that the server knows to enable compression processing for the
    connection (I think this would be the only place we would still use
    `none`, and not as part of a list in this case.)  I think we also
    would want to add libpq functions to allow a client to check the
    last-used compression algorithm in each direction for any
    monitoring/inspection purposes (actually that's probably a good idea
    regardless, so a client application that cares doesn't need to/try to
    implement the intersection and assumption around choosing the first
    algorithm in common).  Also I'm open to better names than "auto", I
    just would like it to avoid unnecessary verbosity for the common case
    of "I just want to enable bidirectional compression with whatever
    algorithms are available with default parameters".
    
    Option 2:
    This one is even cleaner in the common case but a bit worse in the
    uncommon case: just use one parameter and have
    compression/decompression enabling be part of the compression detail
    (e.g. "libpq_compression='gzip:no_decompress;lz4:level=2,no_compress;zstd'"
    or something like that, in which case the "none,gzip" case would
    become "'libpq_compression=gzip:no_compress'").  See
    https://www.postgresql.org/docs/current/app-pgbasebackup.html ,
    specifically the `--compress` flag, for how specifying compression
    algorithms and details works.
    
    I'm actually not sure which of the two I prefer; opinions are welcome :)
    
    Thanks,
    Jacob
    
    
    
    
  6. Re: libpq compression (part 3)

    Jelte Fennema-Nio <postgres@jeltef.nl> — 2023-12-21T00:30:54Z

    Thanks for working on this!
    
    One thing I'm wondering: should it be possible for the client to change the
    compression it wants mid-connection? I can think of some scenarios where
    that would be useful to connection poolers: if a pooler does plain
    forwarding of the compressed messages, then it would need to be able to
    disable/enable compression if it wants to multiplex client connections with
    different compression settings over the same server connection.
    
  7. Re: libpq compression (part 3)

    Andrey Borodin <x4mmm@yandex-team.ru> — 2023-12-29T10:02:48Z

    
    > On 21 Dec 2023, at 05:30, Jelte Fennema-Nio <postgres@jeltef.nl> wrote:
    > 
    > One thing I'm wondering: should it be possible for the client to change the compression it wants mid-connection?
    
    This patchset allows sending CompressionMethod message, which allows to set another codec\level picked from the set of negotiated codec sets (during startup).
    
    
    Best regards, Andrey Borodin.
    
    
    
  8. Re: libpq compression (part 3)

    Jelte Fennema-Nio <postgres@jeltef.nl> — 2023-12-29T16:15:17Z

    On Fri, 29 Dec 2023 at 11:02, Andrey M. Borodin <x4mmm@yandex-team.ru> wrote:
    > This patchset allows sending CompressionMethod message, which allows to set another codec\level picked from the set of negotiated codec sets (during startup).
    
    Did you mean to attach a patchset? I don't see the CompressionMethod
    message in the v2 patchset. Only a CompressedData one.
    
    
    
    
  9. Re: libpq compression (part 3)

    Jacob Burroughs <jburroughs@instructure.com> — 2023-12-31T07:32:19Z

    > One thing I'm wondering: should it be possible for the client to change the compression it wants mid-connection? I can think of some scenarios where that would be useful to connection poolers: if a pooler does plain forwarding of the compressed messages, then it would need to be able to disable/enable compression if it wants to multiplex client connections with different compression settings over the same server connection.
    
    I have reworked this patch series to make it easier to extend to
    restart compression mid-connection once something in the vein of the
    discussion in "Add new protocol message to change GUCs for usage with
    future protocol-only GUCs" [1] happens.  In particular, I have changed
    the `CompressedMessage` protocol message to signal the current
    compression algorithm any time the client should restart its streaming
    decompressor and otherwise implicitly use whatever compression
    algorithm and decompressor was used for previous `CompressedMessage` ,
    which future work can leverage to trigger such a restart on update of
    the client-supported compression algorithms.
    
    > Option 2:
    > This one is even cleaner in the common case but a bit worse in the
    > uncommon case: just use one parameter and have
    > compression/decompression enabling be part of the compression detail
    > (e.g. "libpq_compression='gzip:no_de
    > compress;lz4:level=2,no_compress;zstd'"
    > or something like that, in which case the "none,gzip" case would
    > become "'libpq_compression=gzip:no_compress'").  See
    > https://www.postgresql.org/docs/current/app-pgbasebackup.html ,
    > specifically the `--compress` flag, for how specifying compression
    > algorithms and details works.
    
    I ended up reworking this to use a version of this option in place of
    the `none` hackery, but naming the parameters `compress` and
    `decompress, so to disable compression but allow decompression you
    would specify `libpq_compression=gzip:compress=off`.
    
    Also my windows SSL test failures seem to have resolved themselves
    with either these changes or a rebase, so I think things are truly in
    a reviewable state now.
    
  10. Re: libpq compression (part 3)

    Robert Haas <robertmhaas@gmail.com> — 2024-01-12T20:45:43Z

    On Sun, Dec 31, 2023 at 2:32 AM Jacob Burroughs
    <jburroughs@instructure.com> wrote:
    > I ended up reworking this to use a version of this option in place of
    > the `none` hackery, but naming the parameters `compress` and
    > `decompress, so to disable compression but allow decompression you
    > would specify `libpq_compression=gzip:compress=off`.
    
    I'm still a bit befuddled by this interface.
    libpq_compression=gzip:compress=off looks a lot like it's saying "do
    it, except don't". I guess that you're using "compress" and
    "decompress" to distinguish the two directions - i.e. server to client
    and client to server - but of course in both directions the sender
    compresses and the receiver decompresses, so I don't find that very
    clear.
    
    I wonder if we could use "upstream" and "downstream" to be clearer? Or
    some other terminology?
    
    -- 
    Robert Haas
    EDB: http://www.enterprisedb.com
    
    
    
    
  11. Re: libpq compression (part 3)

    Jacob Burroughs <jburroughs@instructure.com> — 2024-01-12T21:02:47Z

    > I wonder if we could use "upstream" and "downstream" to be clearer? Or
    > some other terminology?
    
    What about `send` and `receive`?
    
    
    
    
  12. Re: libpq compression (part 3)

    Robert Haas <robertmhaas@gmail.com> — 2024-01-12T21:11:19Z

    On Fri, Jan 12, 2024 at 4:02 PM Jacob Burroughs
    <jburroughs@instructure.com> wrote:
    > > I wonder if we could use "upstream" and "downstream" to be clearer? Or
    > > some other terminology?
    >
    > What about `send` and `receive`?
    
    I think that would definitely be better than "compress" and
    "decompress," but I was worried that it might be unclear to the user
    whether the parameter that they specified was from the point of view
    of the client or the server. Perhaps that's a dumb thing to worry
    about, though.
    
    -- 
    Robert Haas
    EDB: http://www.enterprisedb.com
    
    
    
    
  13. Re: libpq compression (part 3)

    Robert Haas <robertmhaas@gmail.com> — 2024-05-14T16:08:40Z

    On Fri, Jan 12, 2024 at 4:11 PM Robert Haas <robertmhaas@gmail.com> wrote:
    > I think that would definitely be better than "compress" and
    > "decompress," but I was worried that it might be unclear to the user
    > whether the parameter that they specified was from the point of view
    > of the client or the server. Perhaps that's a dumb thing to worry
    > about, though.
    
    According to https://commitfest.postgresql.org/48/4746/ this patch set
    needs review, but:
    
    1. Considering that there have been no updates for 5 months, maybe
    it's actually dead?
    
    and
    
    2. I still think it needs to be more clear how the interface is
    supposed to work. I do not want to spend time reviewing a patch to see
    whether it works without understanding how it is intended to work --
    and I also think that reviewing the patch in detail before we've got
    the user interface right makes a whole lot of sense.
    
    -- 
    Robert Haas
    EDB: http://www.enterprisedb.com
    
    
    
    
  14. Re: libpq compression (part 3)

    Jacob Burroughs <jburroughs@instructure.com> — 2024-05-14T16:30:04Z

    On Tue, May 14, 2024 at 11:08 AM Robert Haas <robertmhaas@gmail.com> wrote:
    >
    > According to https://commitfest.postgresql.org/48/4746/ this patch set
    > needs review, but:
    >
    > 1. Considering that there have been no updates for 5 months, maybe
    > it's actually dead?
    
    I've withdrawn this patch from the commitfest.  I had been waiting for
    some resolution on "Add new protocol message to change GUCs for usage
    with future protocol-only GUCs" before I rebased/refactored this one,
    because this would be introducing the first protocol extension so far,
    and that discussion appeared to be working out some meaningful issues
    on how GUCs and protocol parameters should interact.  If you think it
    is worthwhile to proceed here though, I am very happy to do so. (I
    would love to see this feature actually make it into postgres; it
    would almost certainly be a big efficiency and cost savings win for
    how my company deploys postgres internally :) )
    
    > 2. I still think it needs to be more clear how the interface is
    > supposed to work. I do not want to spend time reviewing a patch to see
    > whether it works without understanding how it is intended to work --
    > and I also think that reviewing the patch in detail before we've got
    > the user interface right makes a whole lot of sense.
    
    Regarding the interface, what I had originally gone for was the idea
    that the naming of the options was from the perspective of the side
    you were setting them on.  Therefore, when setting `libpq_compression`
    as a server-level GUC, `compress` would control if the server would
    compress (send compressed data) with the given algorithm, and
    `decompress` would control if the the server would decompress (receive
    compressed data) with the given algorithm.  And likewise on the client
    side, when setting `compression` as a connection config option,
    `compress` would control if the *client* would compress (send
    compressed data) with the given algorithm, and `decompress` would
    control if the the *client* would decompress (receive compressed data)
    with the given algorithm.  So for a client to pick what to send, it
    would choose from the intersection of its own `compress=true` and the
    server's `decompress=true` algorithms sent in the `ParameterStatus`
    message with `libpq_compression`.  And likewise on the server side, it
    would choose from the intersection of the server's `compress=true`
    algorithms and the client's `decompress=true` algorithms sent in the
    `_pq_.libpq_compression` startup option.  If you have a better
    suggestion I am very open to it though.
    
    
    
    -- 
    Jacob Burroughs | Staff Software Engineer
    E: jburroughs@instructure.com
    
    
    
    
  15. Re: libpq compression (part 3)

    Robert Haas <robertmhaas@gmail.com> — 2024-05-14T18:35:20Z

    On Tue, May 14, 2024 at 12:30 PM Jacob Burroughs
    <jburroughs@instructure.com> wrote:
    > I've withdrawn this patch from the commitfest.  I had been waiting for
    > some resolution on "Add new protocol message to change GUCs for usage
    > with future protocol-only GUCs" before I rebased/refactored this one,
    > because this would be introducing the first protocol extension so far,
    > and that discussion appeared to be working out some meaningful issues
    > on how GUCs and protocol parameters should interact.  If you think it
    > is worthwhile to proceed here though, I am very happy to do so. (I
    > would love to see this feature actually make it into postgres; it
    > would almost certainly be a big efficiency and cost savings win for
    > how my company deploys postgres internally :) )
    
    I don't think you should wait for that to be resolved; IMHO, this
    patch needs to inform that discussion more than the other way around.
    
    > > 2. I still think it needs to be more clear how the interface is
    > > supposed to work. I do not want to spend time reviewing a patch to see
    > > whether it works without understanding how it is intended to work --
    > > and I also think that reviewing the patch in detail before we've got
    > > the user interface right makes a whole lot of sense.
    >
    > Regarding the interface, what I had originally gone for was the idea
    > that the naming of the options was from the perspective of the side
    > you were setting them on.  Therefore, when setting `libpq_compression`
    > as a server-level GUC, `compress` would control if the server would
    > compress (send compressed data) with the given algorithm, and
    > `decompress` would control if the the server would decompress (receive
    > compressed data) with the given algorithm.  And likewise on the client
    > side, when setting `compression` as a connection config option,
    > `compress` would control if the *client* would compress (send
    > compressed data) with the given algorithm, and `decompress` would
    > control if the the *client* would decompress (receive compressed data)
    > with the given algorithm.  So for a client to pick what to send, it
    > would choose from the intersection of its own `compress=true` and the
    > server's `decompress=true` algorithms sent in the `ParameterStatus`
    > message with `libpq_compression`.  And likewise on the server side, it
    > would choose from the intersection of the server's `compress=true`
    > algorithms and the client's `decompress=true` algorithms sent in the
    > `_pq_.libpq_compression` startup option.  If you have a better
    > suggestion I am very open to it though.
    
    Well, in my last response before the thread died, I complained that
    libpq_compression=gzip:compress=off was confusing, and I stand by
    that, because "compress" is used both in the name of the parameter and
    as an option within the value of that parameter. I think there's more
    than one acceptable way to resolve that problem, but I think leaving
    it like that is unacceptable.
    
    Even more broadly, I think there have been a couple of versions of
    this patch now where I read the documentation and couldn't understand
    how the feature was supposed to work, and I'm not really willing to
    spend time trying to review a complex patch for conformity with a
    design that I can't understand in the first place. I don't want to
    pretend like I'm the smartest person on this mailing list, and in fact
    I know that I'm definitely not, but I think I'm smart enough and
    experienced enough with PostgreSQL that if I look at the description
    of a parameter and say "I don't understand how the heck this is
    supposed to work", probably a lot of users are going to have the same
    reaction. That lack of understanding on my part my come either from
    the explanation of the parameter not being as good as it needs to be,
    or from the design itself not being as good as it needs to be, or from
    some combination of the two, but whichever is the case, IMHO you or
    somebody else has got to figure out how to fix it.
    
    I do also admit that there is a possibility that everything is totally
    fine and I've just been kinda dumb on the days when I've looked at the
    patch. If a chorus of other hackers shows up and gives me a few whacks
    with the cluestick and after that I look at the proposed options and
    go "oh, yeah, these totally make sense, I was just being stupid," fair
    enough! But right now that's not where I'm at. I don't want you to
    explain to me how it works; I want you to change it in some way so
    that when I or some end user looks at it, they go "I don't need an
    explanation of how that works because it's extremely clear to me
    already," or at least "hmm, this is a bit complicated but after a
    quick glance at the documentation it makes sense".
    
    I would really like to see this patch go forward, but IMHO these UI
    questions are blockers.
    
    -- 
    Robert Haas
    EDB: http://www.enterprisedb.com
    
    
    
    
  16. Re: libpq compression (part 3)

    Jacob Burroughs <jburroughs@instructure.com> — 2024-05-14T19:22:01Z

    On Tue, May 14, 2024 at 1:35 PM Robert Haas <robertmhaas@gmail.com> wrote:
    >
    > Well, in my last response before the thread died, I complained that
    > libpq_compression=gzip:compress=off was confusing, and I stand by
    > that, because "compress" is used both in the name of the parameter and
    > as an option within the value of that parameter. I think there's more
    > than one acceptable way to resolve that problem, but I think leaving
    > it like that is unacceptable.
    
    What if we went with:
    Server side:
    * `libpq_compression=on` (I just want everything the server supports
    available; probably the most common case)
    * `libpq_compression=off` (I don't want any compression ever with this server)
    * `libpq_compression=lzma;gzip` (I only want these algorithms for
    whatever reason)
    * `libpq_compression=lzma:client_to_server=off;gzip:server_to_client=off`
    (I only want to send data with lzma and receive data with gzip)
    Client side:
    *`compression=on` (I just want compression; pick sane defaults
    automatically for me; probably the most common case)
    * `compression=off` (I don't want any compression)
    * `compression=lzma;gzip` (I only want these algorithms for whatever reason)
    * `compression=lzma:client_to_server=off;gzip:server_to_client=off` (I
    only want to receive data with lzma and send data with gzip)
    
    `client_to_server`/`server_to_client` is a bit verbose, but it's very
    explicit in what it means, so you don't need to reason about who is
    sending/receiving/etc in a given context, and a given config string
    applied to the server or the client side has the same effect on the
    connection.
    
    > Even more broadly, I think there have been a couple of versions of
    > this patch now where I read the documentation and couldn't understand
    > how the feature was supposed to work, and I'm not really willing to
    > spend time trying to review a complex patch for conformity with a
    > design that I can't understand in the first place. I don't want to
    > pretend like I'm the smartest person on this mailing list, and in fact
    > I know that I'm definitely not, but I think I'm smart enough and
    > experienced enough with PostgreSQL that if I look at the description
    > of a parameter and say "I don't understand how the heck this is
    > supposed to work", probably a lot of users are going to have the same
    > reaction. That lack of understanding on my part my come either from
    > the explanation of the parameter not being as good as it needs to be,
    > or from the design itself not being as good as it needs to be, or from
    > some combination of the two, but whichever is the case, IMHO you or
    > somebody else has got to figure out how to fix it.
    
    If the above proposal seems better to you I'll both rework the patch
    and then also try to rewrite the relevant bits of documentation to
    separate out "what knobs are there" and "how do I specify the flags to
    turn the knobs", because I think those two being integrated is making
    the parameter documentation less readable/followable.
    
    
    -- 
    Jacob Burroughs | Staff Software Engineer
    E: jburroughs@instructure.com
    
    
    
    
  17. Re: libpq compression (part 3)

    Robert Haas <robertmhaas@gmail.com> — 2024-05-14T20:23:53Z

    On Tue, May 14, 2024 at 3:22 PM Jacob Burroughs
    <jburroughs@instructure.com> wrote:
    > What if we went with:
    > Server side:
    > * `libpq_compression=on` (I just want everything the server supports
    > available; probably the most common case)
    > * `libpq_compression=off` (I don't want any compression ever with this server)
    > * `libpq_compression=lzma;gzip` (I only want these algorithms for
    > whatever reason)
    > * `libpq_compression=lzma:client_to_server=off;gzip:server_to_client=off`
    > (I only want to send data with lzma and receive data with gzip)
    > Client side:
    > *`compression=on` (I just want compression; pick sane defaults
    > automatically for me; probably the most common case)
    > * `compression=off` (I don't want any compression)
    > * `compression=lzma;gzip` (I only want these algorithms for whatever reason)
    > * `compression=lzma:client_to_server=off;gzip:server_to_client=off` (I
    > only want to receive data with lzma and send data with gzip)
    >
    > `client_to_server`/`server_to_client` is a bit verbose, but it's very
    > explicit in what it means, so you don't need to reason about who is
    > sending/receiving/etc in a given context, and a given config string
    > applied to the server or the client side has the same effect on the
    > connection.
    
    IMHO, that's a HUGE improvement. But:
    
    * I would probably change is the name "libpq_compression", because
    even though we have src/backend/libpq, we typically use libpq to refer
    to the client library, not the server's implementation of the wire
    protocol. I think we could call it connection_encryption or
    wire_protocol_encryption or something like that, but I'm not a huge
    fan of libpq_compression.
    
    * I would use commas, not semicolons, to separate items in a list,
    i.e. lzma,gzip not lzma;gzip. I think that convention is nearly
    universal in PostgreSQL, but feel free to point out counterexamples if
    you were modelling this on something.
    
    * libpq_compression=lzma:client_to_server=off;gzip:server_to_client=off
    reads strangely to me. How about making it so that the syntax is like
    this:
    
    libpq_compression=DEFAULT_VALUE_FOR_BOTH_DIRECTIONS:client_to_server=OVERRIDE_FOR_THIS_DIRECTION:servert_to_client=OVERRIDE_FOR_THIS_DIRECTION
    
    With all components being optional. So this example could be written
    in any of these ways:
    
    libpq_compression=lzma;server_to_client=gzip
    libpq_compression=gzip;client_to_server=lzma
    libpq_compression=server_to_client=gzip;client_to_server=lzma
    libpq_compression=client_to_server=lzma;client_to_server=gzip
    
    And if I wrote libpq_compression=server_to_client=gzip that would mean
    send data to the client using gzip and in the other direction use
    whatever the default is.
    
    -- 
    Robert Haas
    EDB: http://www.enterprisedb.com
    
    
    
    
  18. Fwd: libpq compression (part 3)

    Jacob Burroughs <jburroughs@instructure.com> — 2024-05-14T21:21:40Z

    On Tue, May 14, 2024 at 3:24 PM Robert Haas <robertmhaas@gmail.com> wrote:
    >
    > IMHO, that's a HUGE improvement. But:
    >
    > * I would probably change is the name "libpq_compression", because
    > even though we have src/backend/libpq, we typically use libpq to refer
    > to the client library, not the server's implementation of the wire
    > protocol. I think we could call it connection_encryption or
    > wire_protocol_encryption or something like that, but I'm not a huge
    > fan of libpq_compression.
    >
    I think connection_compression would seem like a good name to me.
    
    > * I would use commas, not semicolons, to separate items in a list,
    > i.e. lzma,gzip not lzma;gzip. I think that convention is nearly
    > universal in PostgreSQL, but feel free to point out counterexamples if
    > you were modelling this on something.
    >
    > * libpq_compression=lzma:client_to_server=off;gzip:server_to_client=off
    > reads strangely to me. How about making it so that the syntax is like
    > this:
    >
    > libpq_compression=DEFAULT_VALUE_FOR_BOTH_DIRECTIONS:client_to_server=OVERRIDE_FOR_THIS_DIRECTION:servert_to_client=OVERRIDE_FOR_THIS_DIRECTION
    >
    > With all components being optional. So this example could be written
    > in any of these ways:
    >
    > libpq_compression=lzma;server_to_client=gzip
    > libpq_compression=gzip;client_to_server=lzma
    > libpq_compression=server_to_client=gzip;client_to_server=lzma
    > libpq_compression=client_to_server=lzma;client_to_server=gzip
    >
    > And if I wrote libpq_compression=server_to_client=gzip that would mean
    > send data to the client using gzip and in the other direction use
    > whatever the default is.
    
    The reason for both the semicolons and for not doing this is related
    to using the same specification structure as here:
    https://www.postgresql.org/docs/current/app-pgbasebackup.html
    (specifically the --compress argument). Reusing that specification
    requires that we use commas to separate the flags for a compression
    method, and therefore left me with semicolons as the leftover
    separator character. I think we could go with something like your
    proposal, and in a lot of ways I like it, but there's still the
    possibility of e.g.
    `libpq_compression=client_to_server=zstd:level=10,long=true,gzip;client_to_server=gzip`
    and I'm not quite sure how to make the separator characters work
    coherently if we need to treat `zstd:level=10,long=true` as a unit.
    Alternatively, we could have `connection_compression`,
    `connection_compression_server_to_client`, and
    `connection_compression_client_to_server` as three separate GUCs (and
    on the client side `compression`, `compression_server_to_client`, and
    `compression_client_to_server` as three separate connection
    parameters), where we would treat `connection_compression` as a
    default that could be overridden by an explicit
    client_to_server/server_to_client.  That creates the slightly funky
    case where if you specify all three then the base one ends up unused
    because the two more specific ones are being used instead, but that
    isn't necessarily terrible.  On the server side we *could* go with
    just the server_to_client and client_to_server ones, but I think we
    want it to be easy to use this feature in the simple case with a
    single libpq parameter.
    
    --
    Jacob Burroughs | Staff Software Engineer
    E: jburroughs@instructure.com
    
    
    
    
  19. Re: libpq compression (part 3)

    Robert Haas <robertmhaas@gmail.com> — 2024-05-15T13:38:34Z

    On Tue, May 14, 2024 at 5:21 PM Jacob Burroughs
    <jburroughs@instructure.com> wrote:
    > The reason for both the semicolons and for not doing this is related
    > to using the same specification structure as here:
    > https://www.postgresql.org/docs/current/app-pgbasebackup.html
    > (specifically the --compress argument).
    
    I agree with that goal, but I'm somewhat confused by how your proposal
    achieves it. You had
    libpq_compression=lzma:client_to_server=off;gzip:server_to_client=off,
    so how do we parse that? Is that two completely separate
    specifications, one for lzma and one for gzip, and each of those has
    one option which is set to off? And then they are separated from each
    other by a semicolon? That actually does make sense, and I think it
    may do a better job allowing for compression options than my proposal,
    but it also seems a bit weird, because client_to_server and
    server_to_client are not really compression options at all. They're
    framing when this compression specification applies, rather than what
    it does when it applies. In a way it's a bit like the fact that you
    can prefix a pg_basebackup's --compress option with client- or server-
    to specify where the compression should happen. But we can't quite
    reuse that idea here, because in that case there's no question of
    doing it in both places, whereas here, you might want one thing for
    upstream and another thing for downstream.
    
    > Alternatively, we could have `connection_compression`,
    > `connection_compression_server_to_client`, and
    > `connection_compression_client_to_server` as three separate GUCs (and
    > on the client side `compression`, `compression_server_to_client`, and
    > `compression_client_to_server` as three separate connection
    > parameters), where we would treat `connection_compression` as a
    > default that could be overridden by an explicit
    > client_to_server/server_to_client.  That creates the slightly funky
    > case where if you specify all three then the base one ends up unused
    > because the two more specific ones are being used instead, but that
    > isn't necessarily terrible.  On the server side we *could* go with
    > just the server_to_client and client_to_server ones, but I think we
    > want it to be easy to use this feature in the simple case with a
    > single libpq parameter.
    
    I'm not a fan of three settings; I could go with two settings, one for
    each direction, and if you want both you have to set both. Or, another
    idea, what if we just separated the two directions with a slash,
    SEND/RECEIVE, and if there's no slash, then it applies to both
    directions. So you could say
    connection_compression='gzip:level=9/lzma' or whatever.
    
    But now I'm wondering whether these options should really be symmetric
    on the client and server sides? Isn't it for the server just to
    specify a list of acceptable algorithms, and the client to set the
    compression options? If both sides are trying to set the compression
    level, for example, who wins?
    
    -- 
    Robert Haas
    EDB: http://www.enterprisedb.com
    
    
    
    
  20. Re: libpq compression (part 3)

    Jacob Burroughs <jburroughs@instructure.com> — 2024-05-15T16:24:27Z

    On Wed, May 15, 2024 at 8:38 AM Robert Haas <robertmhaas@gmail.com> wrote:
    >
    > I agree with that goal, but I'm somewhat confused by how your proposal
    > achieves it. You had
    > libpq_compression=lzma:client_to_server=off;gzip:server_to_client=off,
    > so how do we parse that? Is that two completely separate
    > specifications, one for lzma and one for gzip, and each of those has
    > one option which is set to off? And then they are separated from each
    > other by a semicolon? That actually does make sense, and I think it
    > may do a better job allowing for compression options than my proposal,
    > but it also seems a bit weird, because client_to_server and
    > server_to_client are not really compression options at all. They're
    > framing when this compression specification applies, rather than what
    > it does when it applies. In a way it's a bit like the fact that you
    > can prefix a pg_basebackup's --compress option with client- or server-
    > to specify where the compression should happen. But we can't quite
    > reuse that idea here, because in that case there's no question of
    > doing it in both places, whereas here, you might want one thing for
    > upstream and another thing for downstream.
    
    Your interpretation is correct, but I don't disagree that it ends up
    feeling confusing.
    
    > I'm not a fan of three settings; I could go with two settings, one for
    > each direction, and if you want both you have to set both. Or, another
    > idea, what if we just separated the two directions with a slash,
    > SEND/RECEIVE, and if there's no slash, then it applies to both
    > directions. So you could say
    > connection_compression='gzip:level=9/lzma' or whatever.
    >
    > But now I'm wondering whether these options should really be symmetric
    > on the client and server sides? Isn't it for the server just to
    > specify a list of acceptable algorithms, and the client to set the
    > compression options? If both sides are trying to set the compression
    > level, for example, who wins?
    
    Compression options really only ever apply to the side doing the
    compressing, and at least as I had imagined things each party
    (client/server) only used its own level/other compression params.
    That leaves me thinking, maybe we really want two independent GUCs,
    one for "what algorithms are enabled/negotiable" and one for "how
    should I configure my compressors" and then we reduce the dimensions
    we are trying to shove into one GUC and each one ends up with a very
    clear purpose:
    connection_compression=(yes|no|alg1,alg2:server_to_client=alg1,alg2:client_to_server=alg3)
    connection_compression_opts=gzip:level=2
    
    
    -- 
    Jacob Burroughs | Staff Software Engineer
    E: jburroughs@instructure.com
    
    
    
    
  21. Re: libpq compression (part 3)

    Robert Haas <robertmhaas@gmail.com> — 2024-05-15T16:31:29Z

    On Wed, May 15, 2024 at 12:24 PM Jacob Burroughs
    <jburroughs@instructure.com> wrote:
    > > But now I'm wondering whether these options should really be symmetric
    > > on the client and server sides? Isn't it for the server just to
    > > specify a list of acceptable algorithms, and the client to set the
    > > compression options? If both sides are trying to set the compression
    > > level, for example, who wins?
    >
    > Compression options really only ever apply to the side doing the
    > compressing, and at least as I had imagined things each party
    > (client/server) only used its own level/other compression params.
    > That leaves me thinking, maybe we really want two independent GUCs,
    > one for "what algorithms are enabled/negotiable" and one for "how
    > should I configure my compressors" and then we reduce the dimensions
    > we are trying to shove into one GUC and each one ends up with a very
    > clear purpose:
    > connection_compression=(yes|no|alg1,alg2:server_to_client=alg1,alg2:client_to_server=alg3)
    > connection_compression_opts=gzip:level=2
    
    From my point of view, it's the client who knows what it wants to do
    with the connection. If the client plans to read a lot of data, it
    might want the server to compress that data, especially if it knows
    that it's on a slow link. If the client plans to send a lot of data --
    basically COPY, I'm not thinking this is going to matter much
    otherwise -- then it might want to compress that data before sending
    it, again, especially if it knows that it's on a slow link.
    
    But what does the server know, really? If some client connects and
    sends a SELECT query, the server can't guess whether that query is
    going to return 1 row or 100 million rows, so it has no idea of
    whether compression is likely to make sense or not. It is entitled to
    decide, as a matter of policy, that it's not willing to perform
    compression, either because of CPU consumption or security concerns or
    whatever, but it has no knowledge of what the purpose of this
    particular connection is.
    
    -- 
    Robert Haas
    EDB: http://www.enterprisedb.com
    
    
    
    
  22. Re: libpq compression (part 3)

    Jacob Burroughs <jburroughs@instructure.com> — 2024-05-15T16:50:40Z

    On Wed, May 15, 2024 at 11:31 AM Robert Haas <robertmhaas@gmail.com> wrote:
    > From my point of view, it's the client who knows what it wants to do
    > with the connection. If the client plans to read a lot of data, it
    > might want the server to compress that data, especially if it knows
    > that it's on a slow link. If the client plans to send a lot of data --
    > basically COPY, I'm not thinking this is going to matter much
    > otherwise -- then it might want to compress that data before sending
    > it, again, especially if it knows that it's on a slow link.
    >
    > But what does the server know, really? If some client connects and
    > sends a SELECT query, the server can't guess whether that query is
    > going to return 1 row or 100 million rows, so it has no idea of
    > whether compression is likely to make sense or not. It is entitled to
    > decide, as a matter of policy, that it's not willing to perform
    > compression, either because of CPU consumption or security concerns or
    > whatever, but it has no knowledge of what the purpose of this
    > particular connection is.
    
    I think I would agree with that.  That said, I don't think the client
    should be in the business of specifying what configuration of the
    compression algorithm the server should use.  The server administrator
    (or really most of the time, the compression library developer's
    defaults) gets to pick the compression/compute tradeoff for
    compression that runs on the server (which I would imagine would be
    the vast majority of it), and the client gets to pick those same
    parameters for any compression that runs on the client machine
    (probably indeed in practice only for large COPYs).  The *algorithm*
    needs to actually be communicated/negotiated since different
    client/server pairs may be built with support for different
    compression libraries, but I think it is reasonable to say that the
    side that actually has to do the computationally expensive part owns
    the configuration of that part too.  Maybe I'm missing a good reason
    that we want to allow clients to choose compression levels for the
    server though?
    
    
    -- 
    Jacob Burroughs | Staff Software Engineer
    E: jburroughs@instructure.com
    
    
    
    
  23. Re: libpq compression (part 3)

    Robert Haas <robertmhaas@gmail.com> — 2024-05-16T13:28:37Z

    On Wed, May 15, 2024 at 12:50 PM Jacob Burroughs
    <jburroughs@instructure.com> wrote:
    > I think I would agree with that.  That said, I don't think the client
    > should be in the business of specifying what configuration of the
    > compression algorithm the server should use.  The server administrator
    > (or really most of the time, the compression library developer's
    > defaults) gets to pick the compression/compute tradeoff for
    > compression that runs on the server (which I would imagine would be
    > the vast majority of it), and the client gets to pick those same
    > parameters for any compression that runs on the client machine
    > (probably indeed in practice only for large COPYs).  The *algorithm*
    > needs to actually be communicated/negotiated since different
    > client/server pairs may be built with support for different
    > compression libraries, but I think it is reasonable to say that the
    > side that actually has to do the computationally expensive part owns
    > the configuration of that part too.  Maybe I'm missing a good reason
    > that we want to allow clients to choose compression levels for the
    > server though?
    
    Well, I mean, I don't really know what the right answer is here, but
    right now I can say pg_dump --compress=gzip to compress the dump with
    gzip, or pg_dump --compress=gzip:9 to compress with gzip level 9. Now,
    say that instead of compressing the output, I want to compress the
    data sent to me over the connection. So I figure I should be able to
    say pg_dump 'compress=gzip' or pg_dump 'compress=gzip:9'. I think you
    want to let me do the first of those but not the second. But, to turn
    your question on its head, what would be the reasoning behind such a
    restriction?
    
    Note also the precedent of pg_basebackup. I can say pg_basebackup
    --compress=server-gzip:9 to ask the server to compress the backup with
    gzip at level 9. In that case, what I request from the server changes
    the actual output that I get, which is not the case here. Even so, I
    don't really understand what the justification would be for refusing
    to let the client ask for a specific compression level.
    
    And on the flip side, I also don't understand why the server would
    want to mandate a certain compression level. If compression is very
    expensive for a certain algorithm when the level is above some
    threshold X, we could have a GUC to limit the maximum level that the
    client can request. But, given that the gzip compression level
    defaults to 6 in every other context, why would the administrator of a
    particular server want to say, well, the default for my server is 3 or
    9 or whatever?
    
    (This is of course all presuming you want to use gzip at all, which
    you probably don't, because gzip is crazy slow. Use lz4 or zstd! But
    it makes the point.)
    
    -- 
    Robert Haas
    EDB: http://www.enterprisedb.com
    
    
    
    
  24. Re: libpq compression (part 3)

    Jacob Burroughs <jburroughs@instructure.com> — 2024-05-17T20:53:28Z

    On Thu, May 16, 2024 at 3:28 AM Robert Haas <robertmhaas@gmail.com> wrote:
    >
    > Well, I mean, I don't really know what the right answer is here, but
    > right now I can say pg_dump --compress=gzip to compress the dump with
    > gzip, or pg_dump --compress=gzip:9 to compress with gzip level 9. Now,
    > say that instead of compressing the output, I want to compress the
    > data sent to me over the connection. So I figure I should be able to
    > say pg_dump 'compress=gzip' or pg_dump 'compress=gzip:9'. I think you
    > want to let me do the first of those but not the second. But, to turn
    > your question on its head, what would be the reasoning behind such a
    > restriction?
    
    I think I was more thinking that trying to let both parties control
    the parameter seemed like a recipe for confusion and sadness, and so
    the choice that felt most natural to me was to let the sender control
    it, but I'm definitely open to changing that the other way around.
    
    > Note also the precedent of pg_basebackup. I can say pg_basebackup
    > --compress=server-gzip:9 to ask the server to compress the backup with
    > gzip at level 9. In that case, what I request from the server changes
    > the actual output that I get, which is not the case here. Even so, I
    > don't really understand what the justification would be for refusing
    > to let the client ask for a specific compression level.
    >
    > And on the flip side, I also don't understand why the server would
    > want to mandate a certain compression level. If compression is very
    > expensive for a certain algorithm when the level is above some
    > threshold X, we could have a GUC to limit the maximum level that the
    > client can request. But, given that the gzip compression level
    > defaults to 6 in every other context, why would the administrator of a
    > particular server want to say, well, the default for my server is 3 or
    > 9 or whatever?
    >
    > (This is of course all presuming you want to use gzip at all, which
    > you probably don't, because gzip is crazy slow. Use lz4 or zstd! But
    > it makes the point.)
    
    New proposal, predicated on the assumption that if you enable
    compression you are ok with the client picking whatever level they
    want.  At least with the currently enabled algorithms I don't think
    any of them are so insane that they would knock over a server or
    anything, and in general postgres servers are usually connected to by
    clients that the server admin has some channel to talk to (after all
    they somehow had to get access to log in to the server in the first
    place) if they are doing something wasteful, given that a client can
    do a lot worse things than enable aggressive compression by writing
    bad queries.
    
    On the server side, we use slash separated sets of options
    connection_compression=DEFAULT_VALUE_FOR_BOTH_DIRECTIONS/client_to_server=OVERRIDE_FOR_THIS_DIRECTION/server_to_client=OVERRIDE_FOR_THIS_DIRECTION
    with the values being semicolon separated compression algorithms.
    On the client side, you can specify
    compression=<same_specification_as_above>,
    but on the client side you can actually specify compression options,
    which the server will use if provided, and otherwise it will fall back
    to defaults.
    
    If we think we need to, we could let the server specify defaults for
    server-side compression.  My overall thought though is that having an
    excessive number of knobs increases the surface area for testing and
    bugs while also increasing potential user confusion and that allowing
    configuration on *both* sides doesn't seem sufficiently useful to be
    worth adding that complexity.
    
    -- 
    Jacob Burroughs | Staff Software Engineer
    E: jburroughs@instructure.com
    
    
    
    
  25. Re: libpq compression (part 3)

    Jacob Champion <jacob.champion@enterprisedb.com> — 2024-05-17T21:10:36Z

    On Fri, May 17, 2024 at 1:53 PM Jacob Burroughs
    <jburroughs@instructure.com> wrote:
    > New proposal, predicated on the assumption that if you enable
    > compression you are ok with the client picking whatever level they
    > want.  At least with the currently enabled algorithms I don't think
    > any of them are so insane that they would knock over a server or
    > anything, and in general postgres servers are usually connected to by
    > clients that the server admin has some channel to talk to (after all
    > they somehow had to get access to log in to the server in the first
    > place) if they are doing something wasteful, given that a client can
    > do a lot worse things than enable aggressive compression by writing
    > bad queries.
    
    We're talking about a transport-level option, though -- I thought the
    proposal enabled compression before authentication completed? Or has
    that changed?
    
    (I'm suspicious of arguments that begin "well you can already do bad
    things", anyway... It seems like there's a meaningful difference
    between consuming resources running a parsed query and consuming
    resources trying to figure out what the parsed query is. I don't know
    if the solution is locking in a compression level, or something else;
    maybe they're both reasonably mitigated in the same way. I haven't
    really looked into zip bombs much.)
    
    --Jacob
    
    
    
    
  26. Re: libpq compression (part 3)

    Robert Haas <robertmhaas@gmail.com> — 2024-05-17T21:40:44Z

    On Fri, May 17, 2024 at 4:53 PM Jacob Burroughs
    <jburroughs@instructure.com> wrote:
    > I think I was more thinking that trying to let both parties control
    > the parameter seemed like a recipe for confusion and sadness, and so
    > the choice that felt most natural to me was to let the sender control
    > it, but I'm definitely open to changing that the other way around.
    
    To be clear, I am not arguing that it should be the receiver's choice.
    I'm arguing it should be the client's choice, which means the client
    decides what it sends and also tells the server what to send to it.
    I'm open to counter-arguments, but as I've thought about this more,
    I've come to the conclusion that letting the client control the
    behavior is the most likely to be useful and the most consistent with
    existing facilities. I think we're on the same page based on the rest
    of your email: I'm just clarifying.
    
    > On the server side, we use slash separated sets of options
    > connection_compression=DEFAULT_VALUE_FOR_BOTH_DIRECTIONS/client_to_server=OVERRIDE_FOR_THIS_DIRECTION/server_to_client=OVERRIDE_FOR_THIS_DIRECTION
    > with the values being semicolon separated compression algorithms.
    > On the client side, you can specify
    > compression=<same_specification_as_above>,
    > but on the client side you can actually specify compression options,
    > which the server will use if provided, and otherwise it will fall back
    > to defaults.
    
    I have some quibbles with the syntax but I agree with the concept.
    What I'd probably do is separate the server side thing into two GUCs,
    each with a list of algorithms, comma-separated, like we do for other
    lists in postgresql.conf. Maybe make the default 'all' meaning
    "everything this build of the server supports". On the client side,
    I'd allow both things to be specified using a single option, because
    wanting to do the same thing in both directions will be common, and
    you actually have to type in connection strings sometimes, so
    verbosity matters more.
    
    As far as the format of the value for that keyword, what do you think
    about either compression=DO_THIS_BOTH_WAYS or
    compression=DO_THIS_WHEN_SENDING/DO_THIS_WHEN_RECEIVING, with each "do
    this" being a specification of the same form already accepted for
    server-side compression e.g. gzip or gzip:level=9? If you don't like
    that, why do you think the proposal you made above is better, and why
    is that one now punctuated with slashes instead of semicolons?
    
    > If we think we need to, we could let the server specify defaults for
    > server-side compression.  My overall thought though is that having an
    > excessive number of knobs increases the surface area for testing and
    > bugs while also increasing potential user confusion and that allowing
    > configuration on *both* sides doesn't seem sufficiently useful to be
    > worth adding that complexity.
    
    I agree.
    
    -- 
    Robert Haas
    EDB: http://www.enterprisedb.com
    
    
    
    
  27. Re: libpq compression (part 3)

    Jelte Fennema-Nio <postgres@jeltef.nl> — 2024-05-17T22:54:03Z

    On Fri, 17 May 2024 at 23:40, Robert Haas <robertmhaas@gmail.com> wrote:
    > To be clear, I am not arguing that it should be the receiver's choice.
    > I'm arguing it should be the client's choice, which means the client
    > decides what it sends and also tells the server what to send to it.
    > I'm open to counter-arguments, but as I've thought about this more,
    > I've come to the conclusion that letting the client control the
    > behavior is the most likely to be useful and the most consistent with
    > existing facilities. I think we're on the same page based on the rest
    > of your email: I'm just clarifying.
    
    +1
    
    > I have some quibbles with the syntax but I agree with the concept.
    > What I'd probably do is separate the server side thing into two GUCs,
    > each with a list of algorithms, comma-separated, like we do for other
    > lists in postgresql.conf. Maybe make the default 'all' meaning
    > "everything this build of the server supports". On the client side,
    > I'd allow both things to be specified using a single option, because
    > wanting to do the same thing in both directions will be common, and
    > you actually have to type in connection strings sometimes, so
    > verbosity matters more.
    >
    > As far as the format of the value for that keyword, what do you think
    > about either compression=DO_THIS_BOTH_WAYS or
    > compression=DO_THIS_WHEN_SENDING/DO_THIS_WHEN_RECEIVING, with each "do
    > this" being a specification of the same form already accepted for
    > server-side compression e.g. gzip or gzip:level=9? If you don't like
    > that, why do you think the proposal you made above is better, and why
    > is that one now punctuated with slashes instead of semicolons?
    
    +1
    
    
    
    
  28. Re: libpq compression (part 3)

    Jelte Fennema-Nio <postgres@jeltef.nl> — 2024-05-17T23:02:59Z

    On Fri, 17 May 2024 at 23:10, Jacob Champion
    <jacob.champion@enterprisedb.com> wrote:
    > We're talking about a transport-level option, though -- I thought the
    > proposal enabled compression before authentication completed? Or has
    > that changed?
    
    I think it would make sense to only compress messages after
    authentication has completed. The gain of compressing authentication
    related packets seems pretty limited.
    
    > (I'm suspicious of arguments that begin "well you can already do bad
    > things"
    
    Once logged in it's really easy to max out a core of the backend
    you're connected as. There's many trivial queries you can use to do
    that. An example would be:
    SELECT sum(i) from generate_series(1, 1000000000) i;
    
    So I don't think it makes sense to worry about an attacker using a
    high compression level as a means to DoS the server. Sending a few of
    the above queries seems much easier.
    
    
    
    
  29. Re: libpq compression (part 3)

    Jacob Burroughs <jburroughs@instructure.com> — 2024-05-18T05:18:00Z

    On Fri, May 17, 2024 at 11:40 AM Robert Haas <robertmhaas@gmail.com> wrote:
    >
    > To be clear, I am not arguing that it should be the receiver's choice.
    > I'm arguing it should be the client's choice, which means the client
    > decides what it sends and also tells the server what to send to it.
    > I'm open to counter-arguments, but as I've thought about this more,
    > I've come to the conclusion that letting the client control the
    > behavior is the most likely to be useful and the most consistent with
    > existing facilities. I think we're on the same page based on the rest
    > of your email: I'm just clarifying.
    
    This is what I am imagining too
    
    > I have some quibbles with the syntax but I agree with the concept.
    > What I'd probably do is separate the server side thing into two GUCs,
    > each with a list of algorithms, comma-separated, like we do for other
    > lists in postgresql.conf. Maybe make the default 'all' meaning
    > "everything this build of the server supports". On the client side,
    > I'd allow both things to be specified using a single option, because
    > wanting to do the same thing in both directions will be common, and
    > you actually have to type in connection strings sometimes, so
    > verbosity matters more.
    >
    > As far as the format of the value for that keyword, what do you think
    > about either compression=DO_THIS_BOTH_WAYS or
    > compression=DO_THIS_WHEN_SENDING/DO_THIS_WHEN_RECEIVING, with each "do
    > this" being a specification of the same form already accepted for
    > server-side compression e.g. gzip or gzip:level=9? If you don't like
    > that, why do you think the proposal you made above is better, and why
    > is that one now punctuated with slashes instead of semicolons?
    
    I like this more than what I proposed, and will update the patches to
    reflect this proposal. (I've gotten them locally back into a state of
    applying cleanly and dealing with the changes needed to support direct
    SSL connections, so refactoring the protocol layer shouldn't be too
    hard now.)
    
    On Fri, May 17, 2024 at 11:10 AM Jacob Champion
    <jacob.champion@enterprisedb.com> wrote:
    > We're talking about a transport-level option, though -- I thought the
    > proposal enabled compression before authentication completed? Or has
    > that changed?
    On Fri, May 17, 2024 at 1:03 PM Jelte Fennema-Nio <postgres@jeltef.nl> wrote:
    > I think it would make sense to only compress messages after
    > authentication has completed. The gain of compressing authentication
    > related packets seems pretty limited.
    
    At the protocol level, compressed data is a message type that can be
    used to wrap arbitrary data as soon as the startup packet is
    processed.  However, as an implementation detail that clients should
    not rely on but that we can rely on in thinking about the
    implications, the only message types that are compressed (except in
    the 0005 CI patch for test running only) are PqMsg_CopyData,
    PqMsg_DataRow, and PqMsg_Query, all of which aren't sent before
    authentication.
    
    -- 
    Jacob Burroughs | Staff Software Engineer
    E: jburroughs@instructure.com
    
    
    
    
  30. Re: libpq compression (part 3)

    Jacob Champion <jacob.champion@enterprisedb.com> — 2024-05-20T14:14:50Z

    On Fri, May 17, 2024 at 4:03 PM Jelte Fennema-Nio <postgres@jeltef.nl> wrote:
    >
    > On Fri, 17 May 2024 at 23:10, Jacob Champion
    > <jacob.champion@enterprisedb.com> wrote:
    > > We're talking about a transport-level option, though -- I thought the
    > > proposal enabled compression before authentication completed? Or has
    > > that changed?
    >
    > I think it would make sense to only compress messages after
    > authentication has completed. The gain of compressing authentication
    > related packets seems pretty limited.
    
    Okay. But if we're relying on that for its security properties, it
    needs to be enforced by the server.
    
    > > (I'm suspicious of arguments that begin "well you can already do bad
    > > things"
    >
    > Once logged in it's really easy to max out a core of the backend
    > you're connected as. There's many trivial queries you can use to do
    > that. An example would be:
    > SELECT sum(i) from generate_series(1, 1000000000) i;
    
    This is just restating the "you can already do bad things" argument. I
    understand that if your query gets executed, it's going to consume
    resources on the thing that's executing it (for the record, though,
    there are people working on constraining that). But introducing
    disproportionate resource consumption into all traffic-inspecting
    software, like pools and bouncers, seems like a different thing to me.
    Many use cases are going to be fine with it, of course, but I don't
    think it should be hand-waved.
    
    --Jacob
    
    
    
    
  31. Re: libpq compression (part 3)

    Robert Haas <robertmhaas@gmail.com> — 2024-05-20T15:17:30Z

    On Sat, May 18, 2024 at 1:18 AM Jacob Burroughs
    <jburroughs@instructure.com> wrote:
    > I like this more than what I proposed, and will update the patches to
    > reflect this proposal. (I've gotten them locally back into a state of
    > applying cleanly and dealing with the changes needed to support direct
    > SSL connections, so refactoring the protocol layer shouldn't be too
    > hard now.)
    
    Sounds good!
    
    -- 
    Robert Haas
    EDB: http://www.enterprisedb.com
    
    
    
    
  32. Re: libpq compression (part 3)

    Robert Haas <robertmhaas@gmail.com> — 2024-05-20T15:29:07Z

    On Mon, May 20, 2024 at 10:15 AM Jacob Champion
    <jacob.champion@enterprisedb.com> wrote:
    > This is just restating the "you can already do bad things" argument. I
    > understand that if your query gets executed, it's going to consume
    > resources on the thing that's executing it (for the record, though,
    > there are people working on constraining that). But introducing
    > disproportionate resource consumption into all traffic-inspecting
    > software, like pools and bouncers, seems like a different thing to me.
    > Many use cases are going to be fine with it, of course, but I don't
    > think it should be hand-waved.
    
    I can't follow this argument.
    
    I think it's important that the startup message is always sent
    uncompressed, because it's a strange exception to our usual
    message-formatting rules, and because it's so security-critical. I
    don't think we should do anything to allow more variation there,
    because any benefit will be small and the chances of introducing
    security vulnerabilities seems non-trivial.
    
    But if the client says in the startup message that it would like to
    send and receive compressed data and the server is happy with that
    request, I don't see why we need to postpone implementing that request
    until after the authentication exchange is completed. I think that
    will make the code more complicated and I don't see a security
    benefit. If the use of a particular compression algorithm is going to
    impose too much load, the server, or the pooler, is free to refuse it,
    and should. Deferring the use of the compression method until after
    authentication doesn't really solve any problem here, at least not
    that I can see.
    
    It does occur to me that if some compression algorithm has a buffer
    overrun bug, restricting its use until after authentication might
    reduce the score of the resulting CVE, because now you have to be able
    to authenticate to make an exploit work. Perhaps that's an argument
    for imposing a restriction here, but it doesn't seem to be the
    argument that you're making.
    
    -- 
    Robert Haas
    EDB: http://www.enterprisedb.com
    
    
    
    
  33. Re: libpq compression (part 3)

    Jacob Champion <jacob.champion@enterprisedb.com> — 2024-05-20T16:49:46Z

    On Mon, May 20, 2024 at 8:29 AM Robert Haas <robertmhaas@gmail.com> wrote:
    > It does occur to me that if some compression algorithm has a buffer
    > overrun bug, restricting its use until after authentication might
    > reduce the score of the resulting CVE, because now you have to be able
    > to authenticate to make an exploit work. Perhaps that's an argument
    > for imposing a restriction here, but it doesn't seem to be the
    > argument that you're making.
    
    It wasn't my argument; Jacob B said above:
    
    > in general postgres servers are usually connected to by
    > clients that the server admin has some channel to talk to (after all
    > they somehow had to get access to log in to the server in the first
    > place) if they are doing something wasteful, given that a client can
    > do a lot worse things than enable aggressive compression by writing
    > bad queries.
    
    ...and my response was that, no, the proposal doesn't seem to be
    requiring that authentication take place before compression is done.
    (As evidenced by your email. :D) If the claim is that there are no
    security problems with letting unauthenticated clients force
    decompression, then I can try to poke holes in that; or if the claim
    is that we don't need to worry about that at all because we'll wait
    until after authentication, then I can poke holes in that too. My
    request is just that we choose one.
    
    > But if the client says in the startup message that it would like to
    > send and receive compressed data and the server is happy with that
    > request, I don't see why we need to postpone implementing that request
    > until after the authentication exchange is completed. I think that
    > will make the code more complicated and I don't see a security
    > benefit.
    
    I haven't implemented compression bombs before to know lots of
    details, but I think the general idea is to take up resources that are
    vastly disproportionate to the effort expended by the client. The
    systemic risk is then more or less multiplied by the number of
    intermediaries that need to do the decompression. Maybe all three of
    our algorithms are hardened against malicious compression techniques;
    that'd be great. But if we've never had a situation where a completely
    untrusted peer can hand a blob to the server and say "here, decompress
    this for me", maybe we'd better check?
    
    --Jacob
    
    
    
    
  34. Re: libpq compression (part 3)

    Robert Haas <robertmhaas@gmail.com> — 2024-05-20T17:01:38Z

    On Mon, May 20, 2024 at 12:49 PM Jacob Champion
    <jacob.champion@enterprisedb.com> wrote:
    > ...and my response was that, no, the proposal doesn't seem to be
    > requiring that authentication take place before compression is done.
    > (As evidenced by your email. :D) If the claim is that there are no
    > security problems with letting unauthenticated clients force
    > decompression, then I can try to poke holes in that;
    
    I would prefer this approach, so I suggest trying to poke holes here
    first. If you find big enough holes then...
    
    > or if the claim
    > is that we don't need to worry about that at all because we'll wait
    > until after authentication, then I can poke holes in that too. My
    > request is just that we choose one.
    
    ...we can fall back to this and you can try to poke holes here.
    
    I really hope that you can't poke big enough holes to kill the feature
    entirely, though. Because that sounds sad.
    
    -- 
    Robert Haas
    EDB: http://www.enterprisedb.com
    
    
    
    
  35. Re: libpq compression (part 3)

    Jacob Champion <jacob.champion@enterprisedb.com> — 2024-05-20T17:22:55Z

    On Mon, May 20, 2024 at 10:01 AM Robert Haas <robertmhaas@gmail.com> wrote:
    > I really hope that you can't poke big enough holes to kill the feature
    > entirely, though. Because that sounds sad.
    
    Even if there are holes, I don't think the situation's going to be bad
    enough to tank everything; otherwise no one would be able to use
    decompression on the Internet. :D And I expect the authors of the
    newer compression methods to have thought about these things [1].
    
    I hesitate to ask as part of the same email, but what were the plans
    for compression in combination with transport encryption? (Especially
    if you plan to compress the authentication exchange, since mixing your
    LDAP password into the compression context seems like it might be a
    bad idea if you don't want to leak it.)
    
    --Jacob
    
    [1] https://datatracker.ietf.org/doc/html/rfc8878#name-security-considerations
    
    
    
    
  36. Re: libpq compression (part 3)

    Robert Haas <robertmhaas@gmail.com> — 2024-05-20T17:48:30Z

    On Mon, May 20, 2024 at 1:23 PM Jacob Champion
    <jacob.champion@enterprisedb.com> wrote:
    > On Mon, May 20, 2024 at 10:01 AM Robert Haas <robertmhaas@gmail.com> wrote:
    > > I really hope that you can't poke big enough holes to kill the feature
    > > entirely, though. Because that sounds sad.
    >
    > Even if there are holes, I don't think the situation's going to be bad
    > enough to tank everything; otherwise no one would be able to use
    > decompression on the Internet. :D And I expect the authors of the
    > newer compression methods to have thought about these things [1].
    >
    > I hesitate to ask as part of the same email, but what were the plans
    > for compression in combination with transport encryption? (Especially
    > if you plan to compress the authentication exchange, since mixing your
    > LDAP password into the compression context seems like it might be a
    > bad idea if you don't want to leak it.)
    
    So, the data would be compressed first, with framing around that, and
    then transport encryption would happen afterwards. I don't see how
    that would leak your password, but I have a feeling that might be a
    sign that I'm about to learn some unpleasant truths.
    
    -- 
    Robert Haas
    EDB: http://www.enterprisedb.com
    
    
    
    
  37. Re: libpq compression (part 3)

    Andrey Borodin <x4mmm@yandex-team.ru> — 2024-05-20T18:05:22Z

    
    > On 20 May 2024, at 22:48, Robert Haas <robertmhaas@gmail.com> wrote:
    > 
    > On Mon, May 20, 2024 at 1:23 PM Jacob Champion
    > <jacob.champion@enterprisedb.com> wrote:
    >> On Mon, May 20, 2024 at 10:01 AM Robert Haas <robertmhaas@gmail.com> wrote:
    >>> I really hope that you can't poke big enough holes to kill the feature
    >>> entirely, though. Because that sounds sad.
    >> 
    >> Even if there are holes, I don't think the situation's going to be bad
    >> enough to tank everything; otherwise no one would be able to use
    >> decompression on the Internet. :D And I expect the authors of the
    >> newer compression methods to have thought about these things [1].
    >> 
    >> I hesitate to ask as part of the same email, but what were the plans
    >> for compression in combination with transport encryption? (Especially
    >> if you plan to compress the authentication exchange, since mixing your
    >> LDAP password into the compression context seems like it might be a
    >> bad idea if you don't want to leak it.)
    > 
    > So, the data would be compressed first, with framing around that, and
    > then transport encryption would happen afterwards. I don't see how
    > that would leak your password, but I have a feeling that might be a
    > sign that I'm about to learn some unpleasant truths.
    
    Compression defeats encryption. That's why it's not in TLS anymore.
    The thing is compression codecs use data self correlation. And if you mix secret data with user's data, user might guess how correlated they are.
    
    
    Best regards, Andrey Borodin.
    
    
    
  38. Re: libpq compression (part 3)

    Robert Haas <robertmhaas@gmail.com> — 2024-05-20T18:37:14Z

    On Mon, May 20, 2024 at 2:05 PM Andrey M. Borodin <x4mmm@yandex-team.ru> wrote:
    > Compression defeats encryption. That's why it's not in TLS anymore.
    > The thing is compression codecs use data self correlation. And if you mix secret data with user's data, user might guess how correlated they are.
    
    Yeah, I'm aware that there are some problems like this. For example,
    suppose the bad guy can both supply some of the data sent over the
    connection (e.g. by typing search queries into a web page) and also
    observe the traffic between the web application and the database. Then
    they could supply data and try to guess how correlated that is with
    other data sent over the same connection. But if that's a practical
    attack, preventing compression prior to the authentication exchange
    probably isn't good enough: the user could also try to guess what
    queries are being sent on behalf of other users through the same
    pooled connection, or they could try to use the bits of the query that
    they can control to guess what the other bits of the query that they
    can't see look like.
    
    But, does this mean that we should just refuse to offer compression as
    a feature? This kind of attack isn't a threat in every environment,
    and in some environments, compression could be pretty useful. For
    instance, you might need to pull down a lot of data from the database
    over a slow connection. Perhaps you're the only user of the database,
    and you wrote all of the queries yourself in a locked vault, accepting
    no untrusted inputs. In that case, these kinds of attacks aren't
    possible, or at least I don't see how, but you might want both
    compression and encryption. I guess I don't understand why TLS removed
    support for encryption entirely instead of disclaiming its use in some
    appropriate way.
    
    --
    Robert Haas
    EDB: http://www.enterprisedb.com
    
    
    
    
  39. Re: libpq compression (part 3)

    Andrey Borodin <x4mmm@yandex-team.ru> — 2024-05-20T19:09:42Z

    
    
    > On 20 May 2024, at 23:37, Robert Haas <robertmhaas@gmail.com> wrote:
    > 
    >  But if that's a practical
    > attack, preventing compression prior to the authentication exchange
    > probably isn't good enough: the user could also try to guess what
    > queries are being sent on behalf of other users through the same
    > pooled connection, or they could try to use the bits of the query that
    > they can control to guess what the other bits of the query that they
    > can't see look like.
    
    All these attacks can be practically exploited in a controlled environment.
    That's why previous incarnation of this patchset [0] contained a way to reset compression context. And Odyssey AFAIR did it (Dan, coauthor of that patch, implemented the compression in Odyssey).
    But attacking authentication is much more straightforward and viable.
    
    > On 20 May 2024, at 23:37, Robert Haas <robertmhaas@gmail.com> wrote:
    > 
    > But, does this mean that we should just refuse to offer compression as
    > a feature?
    
    No, absolutely, we need the feature.
    
    > I guess I don't understand why TLS removed
    > support for encryption entirely instead of disclaiming its use in some
    > appropriate way.
    
    I think, the scope of TLS is too broad. HTTPS in turn has a compression. But AFAIK it never compress headers.
    IMO we should try to avoid compressing authentication information.
    
    
    Best regards, Andrey Borodin.
    
    [0] https://commitfest.postgresql.org/38/3499/
    
    
    
  40. Re: libpq compression (part 3)

    Jacob Champion <jacob.champion@enterprisedb.com> — 2024-05-20T19:39:49Z

    On Mon, May 20, 2024 at 11:05 AM Andrey M. Borodin <x4mmm@yandex-team.ru> wrote:
    > > So, the data would be compressed first, with framing around that, and
    > > then transport encryption would happen afterwards. I don't see how
    > > that would leak your password, but I have a feeling that might be a
    > > sign that I'm about to learn some unpleasant truths.
    >
    > Compression defeats encryption. That's why it's not in TLS anymore.
    > The thing is compression codecs use data self correlation. And if you mix secret data with user's data, user might guess how correlated they are.
    
    I'm slow on the draw, but I hacked up a sample client to generate
    traffic against the compression-enabled server, to try to illustrate.
    
    If my client sends an LDAP password of "hello", followed by the query
    `SELECT 'world'`, as part of the same gzip stream, I get two encrypted
    packets on the wire: lengths 42 and 49 bytes. If the client instead
    sends the query `SELECT 'hello'`, I get lengths 42 and 46. We lost
    three bytes, and there's only been one packet on the stream before the
    query; if the observer controlled the query, it's pretty obvious that
    the self-similarity has to have come from the PasswordMessage. Rinse
    and repeat.
    
    That doesn't cover the case where the password itself is low-entropy,
    either. "hellohellohellohello" at least has length, but once you
    compress it that collapses. So an attacker can passively monitor for
    shorter password packets and know which user to target first.
    
    --Jacob
    
    
    
    
  41. Re: libpq compression (part 3)

    Jacob Champion <jacob.champion@enterprisedb.com> — 2024-05-20T19:42:13Z

    On Mon, May 20, 2024 at 11:37 AM Robert Haas <robertmhaas@gmail.com> wrote:
    > But if that's a practical
    > attack, preventing compression prior to the authentication exchange
    > probably isn't good enough
    
    I mean... you said it, not me. I'm trying not to rain on the parade
    too much, because compression is clearly very valuable. But it makes
    me really uncomfortable that we're reintroducing the compression
    oracle (especially over the authentication exchange, which is
    generally more secret than the rest of the traffic).
    
    > But, does this mean that we should just refuse to offer compression as
    > a feature? This kind of attack isn't a threat in every environment,
    > and in some environments, compression could be pretty useful. For
    > instance, you might need to pull down a lot of data from the database
    > over a slow connection. Perhaps you're the only user of the database,
    > and you wrote all of the queries yourself in a locked vault, accepting
    > no untrusted inputs. In that case, these kinds of attacks aren't
    > possible, or at least I don't see how, but you might want both
    > compression and encryption.
    
    Right, I think it's reasonable to let a sufficiently
    determined/informed user lift the guardrails, but first we have to
    choose to put guardrails in place... and then we have to somehow
    sufficiently inform the users when it's okay to lift them.
    
    > I guess I don't understand why TLS removed
    > support for encryption entirely instead of disclaiming its use in some
    > appropriate way.
    
    One of the IETF conversations was at [1] (there were dissenters on the
    list, as you might expect). My favorite summary is this one from
    Alyssa Rowan:
    
    > Compression is usually best performed as "high" as possible; transport layer is blind to what's being compressed, which is (as we now know) was definitely too low and was in retrospect a mistake.
    >
    > Any application layer protocol needs to know - if compression is supported - to separate compression contexts for attacker-chosen plaintext and attacker-sought unknown secrets. (As others have stated, HTTPbis covers this.)
    
    But for SQL, where's the dividing line between attacker-chosen and
    attacker-sought? To me, it seems like only the user knows; the server
    has no clue. I think that puts us "lower" in Alyssa's model than HTTP
    is.
    
    As Andrey points out, there was prior work done that started to take
    this into account. I haven't reviewed it to see how good it is -- and
    I think there are probably many use cases in which queries and tables
    contain both private and attacker-controlled information -- but if we
    agree that they have to be separated, then the strategy can at least
    be improved upon.
    
    --Jacob
    
    [1] https://mailarchive.ietf.org/arch/msg/tls/xhMLf8j4pq8W_ZGXUUU1G_m6r1c/
    
    
    
    
  42. Re: libpq compression (part 3)

    Magnus Hagander <magnus@hagander.net> — 2024-05-20T20:11:43Z

    On Mon, May 20, 2024 at 9:09 PM Andrey M. Borodin <x4mmm@yandex-team.ru>
    wrote:
    
    >
    >
    >
    > > On 20 May 2024, at 23:37, Robert Haas <robertmhaas@gmail.com> wrote:
    > >
    > > But, does this mean that we should just refuse to offer compression as
    > > a feature?
    >
    > No, absolutely, we need the feature.
    >
    > > I guess I don't understand why TLS removed
    > > support for encryption entirely instead of disclaiming its use in some
    > > appropriate way.
    >
    > I think, the scope of TLS is too broad. HTTPS in turn has a compression.
    > But AFAIK it never compress headers.
    > IMO we should try to avoid compressing authentication information.
    >
    
    That used to be the case in HTTP/1. But header compression was one of the
    headline features of HTTP/2, which isn't exactly new anymore. But there's a
    special algorithm, HPACK, for it. And then http/3 uses QPACK.
    Cloudflare has a pretty decent blog post explaining why and how:
    https://blog.cloudflare.com/hpack-the-silent-killer-feature-of-http-2/, or
    rfc7541 for all the details.
    
    tl;dr; is yes, let's be careful not to expose headers to a CRIME-style
    attack. And I doubt our connections has as much to gain by compressing
    "header style" fields as http, so we are probably better off just not
    compressing those parts.
    
    -- 
     Magnus Hagander
     Me: https://www.hagander.net/ <http://www.hagander.net/>
     Work: https://www.redpill-linpro.com/ <http://www.redpill-linpro.com/>
    
  43. Re: libpq compression (part 3)

    Robert Haas <robertmhaas@gmail.com> — 2024-05-21T12:32:56Z

    On Mon, May 20, 2024 at 4:12 PM Magnus Hagander <magnus@hagander.net> wrote:
    > That used to be the case in HTTP/1. But header compression was one of the headline features of HTTP/2, which isn't exactly new anymore. But there's a special algorithm, HPACK, for it. And then http/3 uses QPACK. Cloudflare has a pretty decent blog post explaining why and how: https://blog.cloudflare.com/hpack-the-silent-killer-feature-of-http-2/, or rfc7541 for all the details.
    >
    > tl;dr; is yes, let's be careful not to expose headers to a CRIME-style attack. And I doubt our connections has as much to gain by compressing "header style" fields as http, so we are probably better off just not compressing >  Work: https://www.redpill-linpro.com/
    
    What do you think constitutes a header in the context of the
    PostgreSQL wire protocol?
    
    -- 
    Robert Haas
    EDB: http://www.enterprisedb.com
    
    
    
    
  44. Re: libpq compression (part 3)

    Jacob Burroughs <jburroughs@instructure.com> — 2024-05-21T15:23:36Z

    On Mon, May 20, 2024 at 2:42 PM Jacob Champion
    <jacob.champion@enterprisedb.com> wrote:
    >
    > I mean... you said it, not me. I'm trying not to rain on the parade
    > too much, because compression is clearly very valuable. But it makes
    > me really uncomfortable that we're reintroducing the compression
    > oracle (especially over the authentication exchange, which is
    > generally more secret than the rest of the traffic).
    
    As currently implemented, the compression only applies to
    CopyData/DataRow/Query messages, none of which should be involved in
    authentication, unless I've really missed something in my
    understanding.
    
    > Right, I think it's reasonable to let a sufficiently
    > determined/informed user lift the guardrails, but first we have to
    > choose to put guardrails in place... and then we have to somehow
    > sufficiently inform the users when it's okay to lift them.
    
    My thought would be that compression should be opt-in on the client
    side, with documentation around the potential security pitfalls. (I
    could be convinced it should be opt-in on the server side, but overall
    I think opt-in on the client side generally protects against footguns
    without excessively getting in the way and if an attacker controls the
    client, they can just get the information they want directly-they
    don't need compression sidechannels to get that information.)
    
    > But for SQL, where's the dividing line between attacker-chosen and
    > attacker-sought? To me, it seems like only the user knows; the server
    > has no clue. I think that puts us "lower" in Alyssa's model than HTTP
    > is.
    >
    > As Andrey points out, there was prior work done that started to take
    > this into account. I haven't reviewed it to see how good it is -- and
    > I think there are probably many use cases in which queries and tables
    > contain both private and attacker-controlled information -- but if we
    > agree that they have to be separated, then the strategy can at least
    > be improved upon.
    
    Within SQL-level things, I don't think we can reasonably differentiate
    between private and attacker-controlled information at the
    libpq/server level.  We can reasonably differentiate between message
    types that *definitely* are private and ones that could have
    either/both data in them, but that's not nearly as useful.  I think
    not compressing auth-related packets plus giving a mechanism to reset
    the compression stream for clients (plus guidance on the tradeoffs
    involved in turning on compression) is about as good as we can get.
    That said, I *think* the feature is reasonable to be
    reviewed/committed without the reset functionality as long as the
    compressed data already has the mechanism built in (as it does) to
    signal when a decompressor should restart its streaming.  The actual
    signaling protocol mechanism/necessary libpq API can happen in
    followon work.
    
    
    -- 
    Jacob Burroughs | Staff Software Engineer
    E: jburroughs@instructure.com
    
    
    
    
  45. Re: libpq compression (part 3)

    Jelte Fennema-Nio <postgres@jeltef.nl> — 2024-05-21T15:42:19Z

    On Mon, 20 May 2024 at 21:42, Jacob Champion
    <jacob.champion@enterprisedb.com> wrote:
    > As Andrey points out, there was prior work done that started to take
    > this into account. I haven't reviewed it to see how good it is -- and
    > I think there are probably many use cases in which queries and tables
    > contain both private and attacker-controlled information -- but if we
    > agree that they have to be separated, then the strategy can at least
    > be improved upon.
    
    
    To help get everyone on the same page I wanted to list all the
    security concerns in one place:
    
    1. Triggering excessive CPU usage before authentication, by asking for
    very high compression levels
    2. Triggering excessive memory/CPU usage before authentication, by
    sending a client sending a zipbomb
    3. Triggering excessive CPU after authentication, by asking for a very
    high compression level
    4. Triggering excessive memory/CPU after authentication due to
    zipbombs (i.e. small amount of data extracting to lots of data)
    5. CRIME style leakage of information about encrypted data
    
    1 & 2 can easily be solved by not allowing any authentication packets
    to be compressed. This also has benefits for 5.
    
    3 & 4 are less of a concern than 1&2 imho. Once authenticated a client
    deserves some level of trust. But having knobs to limit impact
    definitely seems useful.
    
    3 can be solved in two ways afaict:
    a. Allow the server to choose the maximum compression level for each
    compression method (using some GUC), and downgrade the level
    transparently when a higher level is requested
    b. Don't allow the client to choose the compression level that the server uses.
    
    I'd prefer option a
    
    4 would require some safety limits on the amount of data that a
    (small) compressed message can be decompressed to, and stop
    decompression of that message once that limit is hit. What that limit
    should be seems hard to choose though. A few ideas:
    a. The size of the message reported by the uncompressed header. This
    would mean that at most the 4GB will be uncompressed, since maximum
    message length is 4GB (limited by 32bit message length field)
    b. Allow servers to specify maximum client decompressed message length
    lower than this 4GB, e.g. messages of more than 100MB of uncompressed
    size should not be allowed.
    
    I think 5 is the most complicated to deal with, especially as it
    depends on the actual usage to know what is safe. I believe we should
    let users have the freedom to make their own security tradeoffs, but
    we should protect them against some of the most glaring issues
    (especially ones that benefit little from compression anyway). As
    already shown by Andrey, sending LDAP passwords in a compressed way
    seems extremely dangerous. So I think we should disallow compressing
    any authentication related packets. To reduce similar risks further we
    can choose to compress only the message types that we expect to
    benefit most from compression. IMHO those are the following (marked
    with (B)ackend or (F)rontend to show who sends them):
    - Query (F)
    - Parse (F)
    - Describe (F)
    - Bind (F)
    - RowDescription (B)
    - DataRow (B)
    - CopyData (B/F)
    
    Then I think we should let users choose how they want to compress and
    where they want their compression stream to restart. Something like
    this:
    a. compression_restart=query: Restart the stream after every query.
    Recommended if queries across the same connection are triggered by
    different end-users. I think this would be a sane default
    b. compression_restart=message: Restart the stream for every message.
    Recommended if the amount of correlation between rows of the same
    query is a security concern.
    c. compression_restart=manual: Don't restart the stream automatically,
    but only when the client user calls a specific function. Recommended
    only if the user can make trade-offs, or if no encryption is used
    anyway.
    
    
    
    
  46. Re: libpq compression (part 3)

    Jacob Burroughs <jburroughs@instructure.com> — 2024-05-21T16:13:57Z

    On Tue, May 21, 2024 at 10:43 AM Jelte Fennema-Nio <postgres@jeltef.nl> wrote:
    >
    > To help get everyone on the same page I wanted to list all the
    > security concerns in one place:
    >
    > 1. Triggering excessive CPU usage before authentication, by asking for
    > very high compression levels
    > 2. Triggering excessive memory/CPU usage before authentication, by
    > sending a client sending a zipbomb
    > 3. Triggering excessive CPU after authentication, by asking for a very
    > high compression level
    > 4. Triggering excessive memory/CPU after authentication due to
    > zipbombs (i.e. small amount of data extracting to lots of data)
    > 5. CRIME style leakage of information about encrypted data
    >
    > 1 & 2 can easily be solved by not allowing any authentication packets
    > to be compressed. This also has benefits for 5.
    
    This is already addressed by only compressing certain message types.
    If we think it is important that the server reject compressed packets
    of other types I can add that, but it seemed reasonable to just make
    the client never send such packets compressed.
    
    > 3 & 4 are less of a concern than 1&2 imho. Once authenticated a client
    > deserves some level of trust. But having knobs to limit impact
    > definitely seems useful.
    >
    > 3 can be solved in two ways afaict:
    > a. Allow the server to choose the maximum compression level for each
    > compression method (using some GUC), and downgrade the level
    > transparently when a higher level is requested
    > b. Don't allow the client to choose the compression level that the server uses.
    >
    > I'd prefer option a
    
    3a would seem preferable given discussion upthread. It would probably
    be worth doing some measurement to check how much of an actual
    difference in compute effort the max vs the default for all 3
    algorithms adds, because I would really prefer to avoid needing to add
    even more configuration knobs if the max compression level for the
    streaming data usecase is sufficiently performant.
    
    > 4 would require some safety limits on the amount of data that a
    > (small) compressed message can be decompressed to, and stop
    > decompression of that message once that limit is hit. What that limit
    > should be seems hard to choose though. A few ideas:
    > a. The size of the message reported by the uncompressed header. This
    > would mean that at most the 4GB will be uncompressed, since maximum
    > message length is 4GB (limited by 32bit message length field)
    > b. Allow servers to specify maximum client decompressed message length
    > lower than this 4GB, e.g. messages of more than 100MB of uncompressed
    > size should not be allowed.
    
    Because we are using streaming decompression, this is much less of an
    issue than for things that decompress wholesale onto disk/into memory.
    We only read PQ_RECV_BUFFER_SIZE (8k) bytes off the stream at once,
    and when reading a packet we already have a `maxmsglen` that is
    PQ_LARGE_MESSAGE_LIMIT (1gb) already, and "We abort the connection (by
    returning EOF) if client tries to send more than that.)".  Therefore,
    we effectively already have a limit of 1gb that applies to regular
    messages too, and I think we should rely on this mechanism for
    compressed data too (if we really think we need to make that number
    configurable we probably could, but again the fewer new knobs we need
    to add the better.
    
    
    > I think 5 is the most complicated to deal with, especially as it
    > depends on the actual usage to know what is safe. I believe we should
    > let users have the freedom to make their own security tradeoffs, but
    > we should protect them against some of the most glaring issues
    > (especially ones that benefit little from compression anyway). As
    > already shown by Andrey, sending LDAP passwords in a compressed way
    > seems extremely dangerous. So I think we should disallow compressing
    > any authentication related packets. To reduce similar risks further we
    > can choose to compress only the message types that we expect to
    > benefit most from compression. IMHO those are the following (marked
    > with (B)ackend or (F)rontend to show who sends them):
    > - Query (F)
    > - Parse (F)
    > - Describe (F)
    > - Bind (F)
    > - RowDescription (B)
    > - DataRow (B)
    > - CopyData (B/F)
    
    That seems like a reasonable list (current implementation is just
    CopyData/DataRow/Query, but I really just copied that fairly blindly
    from the previous incarnation of this effort.) See also my comment
    below 1&2 for if we think we need to block decompressing them too.
    
    > Then I think we should let users choose how they want to compress and
    > where they want their compression stream to restart. Something like
    > this:
    > a. compression_restart=query: Restart the stream after every query.
    > Recommended if queries across the same connection are triggered by
    > different end-users. I think this would be a sane default
    > b. compression_restart=message: Restart the stream for every message.
    > Recommended if the amount of correlation between rows of the same
    > query is a security concern.
    > c. compression_restart=manual: Don't restart the stream automatically,
    > but only when the client user calls a specific function. Recommended
    > only if the user can make trade-offs, or if no encryption is used
    > anyway.
    
    I reasonably like this idea, though I think maybe we should also
    (instead of query?) add per-transaction on the backend side.  I'm
    curious what other people think of this.
    
    
    -- 
    Jacob Burroughs | Staff Software Engineer
    E: jburroughs@instructure.com
    
    
    
    
  47. Re: libpq compression (part 3)

    Jacob Champion <jacob.champion@enterprisedb.com> — 2024-05-21T18:23:52Z

    On Tue, May 21, 2024 at 8:23 AM Jacob Burroughs
    <jburroughs@instructure.com> wrote:
    > As currently implemented, the compression only applies to
    > CopyData/DataRow/Query messages, none of which should be involved in
    > authentication, unless I've really missed something in my
    > understanding.
    
    Right, but Robert has argued that we should compress it all, and I'm
    responding to that proposal.
    
    Sorry for introducing threads within threads. But I think it's
    valuable to pin down both 1) the desired behavior, and 2) how the
    current proposal behaves, as two separate things. I'll try to do a
    better job of communicating which I'm talking about.
    
    > > Right, I think it's reasonable to let a sufficiently
    > > determined/informed user lift the guardrails, but first we have to
    > > choose to put guardrails in place... and then we have to somehow
    > > sufficiently inform the users when it's okay to lift them.
    >
    > My thought would be that compression should be opt-in on the client
    > side, with documentation around the potential security pitfalls. (I
    > could be convinced it should be opt-in on the server side, but overall
    > I think opt-in on the client side generally protects against footguns
    > without excessively getting in the way
    
    We absolutely have to document the risks and allow clients to be
    written safely. But I think server-side controls on risky behavior
    have proven to be generally more valuable, because the server
    administrator is often in a better spot to see the overall risks to
    the system. ("No, you will not use deprecated ciphersuites. No, you
    will not access this URL over plaintext. No, I will not compress this
    response containing customer credit card numbers, no matter how nicely
    you ask.") There are many more clients than servers, so it's less
    risky for the server to enforce safety than to hope that every client
    is safe.
    
    Does your database and access pattern regularly mingle secrets with
    public data? Would auditing correct client use of compression be a
    logistical nightmare? Do your app developers keep indicating in
    conversations that they don't understand the risks at all? Cool, just
    set `encrypted_compression = nope_nope_nope` on the server and sleep
    soundly at night. (Ideally we would default to that.)
    
    > and if an attacker controls the
    > client, they can just get the information they want directly-they
    > don't need compression sidechannels to get that information.)
    
    Sure, but I don't think that's relevant to the threats being discussed.
    
    > Within SQL-level things, I don't think we can reasonably differentiate
    > between private and attacker-controlled information at the
    > libpq/server level.
    
    And by the IETF line of argument -- or at least the argument I quoted
    above -- that implies that we really have no business introducing
    compression when confidentiality is requested. A stronger approach
    would require us to prove, or the user to indicate, safety before
    compressing.
    
    Take a look at the security notes for QPACK [1] -- keeping in mind
    that they know _more_ about what's going on at the protocol level than
    we do, due to the header design. And they still say things like "an
    encoder might choose not to index values with low entropy" and "these
    criteria ... will evolve over time as new attacks are discovered." A
    huge amount is left as an exercise for the reader. This stuff is
    really hard.
    
    > We can reasonably differentiate between message
    > types that *definitely* are private and ones that could have
    > either/both data in them, but that's not nearly as useful.  I think
    > not compressing auth-related packets plus giving a mechanism to reset
    > the compression stream for clients (plus guidance on the tradeoffs
    > involved in turning on compression) is about as good as we can get.
    
    The concept of stream reset seems necessary but insufficient at the
    application level, which bleeds over into Jelte's compression_restart
    proposal. (At the protocol level, I think it may be sufficient?)
    
    If I write a query where one of the WHERE clauses is
    attacker-controlled and the other is a secret, I would really like to
    not compress that query on the client side. If I join a table of user
    IDs against a table of user-provided addresses and a table of
    application tokens for that user, compressing even a single row leaks
    information about those tokens -- at a _very_ granular level -- and I
    would really like the server not to do that.
    
    So if I'm building sand castles... I think maybe it'd be nice to mark
    tables (and/or individual columns?) as safe for compression under
    encryption, whether by row or in aggregate. And maybe libpq and psql
    should be able to turn outgoing compression on and off at will.
    
    And I understand those would balloon the scope of the feature. I'm
    worried I'm doing the security-person thing and sucking all the air
    out of the room. I know not everybody uses transport encryption; for
    those people, compress-it-all is probably a pretty winning strategy,
    and there's no need to reset the compression context ever. And the
    pg_dump-style, "give me everything" use case seems like it could maybe
    be okay, but I really don't know how to assess the risk there, at all.
    
    > That said, I *think* the feature is reasonable to be
    > reviewed/committed without the reset functionality as long as the
    > compressed data already has the mechanism built in (as it does) to
    > signal when a decompressor should restart its streaming.  The actual
    > signaling protocol mechanism/necessary libpq API can happen in
    > followon work.
    
    Well... working out the security minutiae _after_ changing the
    protocol is not historically a winning strategy, I think. Better to do
    it as a vertical stack.
    
    Thanks,
    --Jacob
    
    [1] https://www.rfc-editor.org/rfc/rfc9204.html#name-security-considerations
    
    
    
    
  48. Re: libpq compression (part 3)

    Jacob Champion <jacob.champion@enterprisedb.com> — 2024-05-21T18:38:51Z

    On Tue, May 21, 2024 at 9:14 AM Jacob Burroughs
    <jburroughs@instructure.com> wrote:
    > On Tue, May 21, 2024 at 10:43 AM Jelte Fennema-Nio <postgres@jeltef.nl> wrote:
    > > To help get everyone on the same page I wanted to list all the
    > > security concerns in one place:
    > >
    > > 1. Triggering excessive CPU usage before authentication, by asking for
    > > very high compression levels
    > > 2. Triggering excessive memory/CPU usage before authentication, by
    > > sending a client sending a zipbomb
    > > 3. Triggering excessive CPU after authentication, by asking for a very
    > > high compression level
    > > 4. Triggering excessive memory/CPU after authentication due to
    > > zipbombs (i.e. small amount of data extracting to lots of data)
    > > 5. CRIME style leakage of information about encrypted data
    > >
    > > 1 & 2 can easily be solved by not allowing any authentication packets
    > > to be compressed. This also has benefits for 5.
    >
    > This is already addressed by only compressing certain message types.
    > If we think it is important that the server reject compressed packets
    > of other types I can add that, but it seemed reasonable to just make
    > the client never send such packets compressed.
    
    If the server doesn't reject compressed packets pre-authentication,
    then case 2 isn't mitigated. (I haven't proven how risky that case is
    yet, to be clear.) In other words: if the threat model is that a
    client can attack us, we shouldn't assume that it will attack us
    politely.
    
    > > 4 would require some safety limits on the amount of data that a
    > > (small) compressed message can be decompressed to, and stop
    > > decompression of that message once that limit is hit. What that limit
    > > should be seems hard to choose though. A few ideas:
    > > a. The size of the message reported by the uncompressed header. This
    > > would mean that at most the 4GB will be uncompressed, since maximum
    > > message length is 4GB (limited by 32bit message length field)
    > > b. Allow servers to specify maximum client decompressed message length
    > > lower than this 4GB, e.g. messages of more than 100MB of uncompressed
    > > size should not be allowed.
    >
    > Because we are using streaming decompression, this is much less of an
    > issue than for things that decompress wholesale onto disk/into memory.
    
    (I agree in general, but since you're designing a protocol extension,
    IMO it's not enough that your implementation happens to mitigate
    risks. We more or less have to bake those mitigations into the
    specification of the extension, because things that aren't servers
    have to decompress now. Similar to RFC security considerations.)
    
    --Jacob
    
    
    
    
  49. Re: libpq compression (part 3)

    Jacob Burroughs <jburroughs@instructure.com> — 2024-05-21T19:08:37Z

    On Tue, May 21, 2024 at 1:39 PM Jacob Champion
    <jacob.champion@enterprisedb.com> wrote:
    >
    > If the server doesn't reject compressed packets pre-authentication,
    > then case 2 isn't mitigated. (I haven't proven how risky that case is
    > yet, to be clear.) In other words: if the threat model is that a
    > client can attack us, we shouldn't assume that it will attack us
    > politely.
    
    I think I thought I was writing about something else when I wrote that
    :sigh:.  I think what I really should have written was a version of
    the part below, which is that we use streaming decompression, only
    decompress 8kb at a time, and for pre-auth messages limit them to
    `PG_MAX_AUTH_TOKEN_LENGTH` (65535 bytes), which isn't really enough
    data to actually cause any real-world pain by needing to decompress vs
    the equivalent pain of sending invalid uncompressed auth packets.
    
    > > Because we are using streaming decompression, this is much less of an
    > > issue than for things that decompress wholesale onto disk/into memory.
    >
    > (I agree in general, but since you're designing a protocol extension,
    > IMO it's not enough that your implementation happens to mitigate
    > risks. We more or less have to bake those mitigations into the
    > specification of the extension, because things that aren't servers
    > have to decompress now. Similar to RFC security considerations.)
    
    We own both the canonical client and server, so those are both covered
    here.  I would think it would be the responsibility of any other
    system that maintains its own implementation of the postgres protocol
    and chooses to support the compression protocol to perform its own
    mitigations against potential compression security issues.  Should we
    put the fixed message size limits (that have de facto been part of the
    protocol since 2021, even if they weren't documented as such) into the
    protocol documentation?  That would give implementers of the protocol
    numbers that they could actually rely on when implementing the
    appropriate safeguards because they would be able to actually have
    explicit guarantees about the size of messages. I think it would make
    more sense to put the limits on the underlying messages rather than
    adding an additional limit that only applies to compressed messages.
    ( I don't really see how one could implement other tooling that used
    pg compression without using streaming compression, as the protocol
    never hands over a standalone blob of compressed data: all compressed
    data is always part of a stream, but even with streaming decompression
    you still need some kind of limits or you will just chew up memory.)
    
    -- 
    Jacob Burroughs | Staff Software Engineer
    E: jburroughs@instructure.com
    
    
    
    
  50. Re: libpq compression (part 3)

    Jacob Burroughs <jburroughs@instructure.com> — 2024-05-21T19:26:32Z

    On Tue, May 21, 2024 at 1:24 PM Jacob Champion
    <jacob.champion@enterprisedb.com> wrote:
    >
    > We absolutely have to document the risks and allow clients to be
    > written safely. But I think server-side controls on risky behavior
    > have proven to be generally more valuable, because the server
    > administrator is often in a better spot to see the overall risks to
    > the system. ("No, you will not use deprecated ciphersuites. No, you
    > will not access this URL over plaintext. No, I will not compress this
    > response containing customer credit card numbers, no matter how nicely
    > you ask.") There are many more clients than servers, so it's less
    > risky for the server to enforce safety than to hope that every client
    > is safe.
    >
    > Does your database and access pattern regularly mingle secrets with
    > public data? Would auditing correct client use of compression be a
    > logistical nightmare? Do your app developers keep indicating in
    > conversations that they don't understand the risks at all? Cool, just
    > set `encrypted_compression = nope_nope_nope` on the server and sleep
    > soundly at night. (Ideally we would default to that.)
    
    Thinking about this more (and adding a encrypted_compression GUC or
    whatever), I think my inclination would on the server-side default
    enable compression for insecure connections but default disable for
    encrypted connections, but both would be config parameters that can be
    changed as desired.
    
    > The concept of stream reset seems necessary but insufficient at the
    > application level, which bleeds over into Jelte's compression_restart
    > proposal. (At the protocol level, I think it may be sufficient?)
    >
    > If I write a query where one of the WHERE clauses is
    > attacker-controlled and the other is a secret, I would really like to
    > not compress that query on the client side. If I join a table of user
    > IDs against a table of user-provided addresses and a table of
    > application tokens for that user, compressing even a single row leaks
    > information about those tokens -- at a _very_ granular level -- and I
    > would really like the server not to do that.
    >
    > So if I'm building sand castles... I think maybe it'd be nice to mark
    > tables (and/or individual columns?) as safe for compression under
    > encryption, whether by row or in aggregate. And maybe libpq and psql
    > should be able to turn outgoing compression on and off at will.
    >
    > And I understand those would balloon the scope of the feature. I'm
    > worried I'm doing the security-person thing and sucking all the air
    > out of the room. I know not everybody uses transport encryption; for
    > those people, compress-it-all is probably a pretty winning strategy,
    > and there's no need to reset the compression context ever. And the
    > pg_dump-style, "give me everything" use case seems like it could maybe
    > be okay, but I really don't know how to assess the risk there, at all.
    
    I would imagine that a large volume of uses of postgres are in
    contexts (e.g. internal networks) where either no encryption is used
    or even when encryption is used the benefit of compression vs the risk
    of someone being a position to perform a BREACH-style sidechannel
    attack against DB traffic is sufficiently high that compress-it-all
    would be be quite useful in many cases.  Would some sort of
    per-table/column marking be useful for some cases?  Probably, but that
    doesn't seem to me like it needs to be in v1 of this feature as long
    as the protocol layer itself is designed such that parties can
    arbitrarily alternate between transmitting compressed and uncompressed
    data.  Then if we build such a feature down the road we just add logic
    around *when* we compress but the protocol layer doesn't change.
    
    > Well... working out the security minutiae _after_ changing the
    > protocol is not historically a winning strategy, I think. Better to do
    > it as a vertical stack.
    
    Thinking about it more, I agree that we probably should work out the
    protocol level mechanism for resetting compression
    context/enabling/disabling/reconfiguring compression as part of this
    work.  I don't think that we need to have all the ways that the
    application layer might choose to use such things done here, but we
    should have all the necessary primitives worked out.
    
    -- 
    Jacob Burroughs | Staff Software Engineer
    E: jburroughs@instructure.com
    
    
    
    
  51. Re: libpq compression (part 3)

    Jacob Champion <jacob.champion@enterprisedb.com> — 2024-05-21T19:42:57Z

    On Tue, May 21, 2024 at 12:08 PM Jacob Burroughs
    <jburroughs@instructure.com> wrote:
    > I think I thought I was writing about something else when I wrote that
    > :sigh:.  I think what I really should have written was a version of
    > the part below, which is that we use streaming decompression, only
    > decompress 8kb at a time, and for pre-auth messages limit them to
    > `PG_MAX_AUTH_TOKEN_LENGTH` (65535 bytes), which isn't really enough
    > data to actually cause any real-world pain by needing to decompress vs
    > the equivalent pain of sending invalid uncompressed auth packets.
    
    Okay. So it sounds like your position is similar to Robert's from
    earlier: prefer allowing unauthenticated compressed packets for
    simplicity, as long as we think it's safe for the server. (Personally
    I still think a client that compresses its password packets is doing
    it wrong, and we could help them out by refusing that.)
    
    > We own both the canonical client and server, so those are both covered
    > here.  I would think it would be the responsibility of any other
    > system that maintains its own implementation of the postgres protocol
    > and chooses to support the compression protocol to perform its own
    > mitigations against potential compression security issues.
    
    Sure, but if our official documentation is "here's an extremely
    security-sensitive feature, figure it out!" then we've done a
    disservice to the community.
    
    > Should we
    > put the fixed message size limits (that have de facto been part of the
    > protocol since 2021, even if they weren't documented as such) into the
    > protocol documentation?
    
    Possibly? I don't know if the other PG-compatible implementations use
    the same limits. It might be better to say "limits must exist".
    
    > ( I don't really see how one could implement other tooling that used
    > pg compression without using streaming compression, as the protocol
    > never hands over a standalone blob of compressed data: all compressed
    > data is always part of a stream, but even with streaming decompression
    > you still need some kind of limits or you will just chew up memory.)
    
    Well, that's a good point; I wasn't thinking about the streaming APIs
    themselves. If the easiest way to implement decompression requires the
    use of an API that shouts "hey, give me guardrails!", then that helps
    quite a bit. I really need to look into the attack surface of the
    three algorithms.
    
    --Jacob