Обсуждение: Re: GSS Auth issue when user member of lots of AD groups
[ pgsql-committers is completely inappropriate, redirecting to -bugs ] Chris Gooch <cgooch@bamfunds.com> writes: > GSS authentication is working for users with small number of AD > groups but getting below error when a user has larger number of > groups. I believe it might to token size related, but they don't > have issues when authenticating with Kerberos/GSS to other > applications, only with Postgres. > failed: GSSAPI context establishment error: The routine must be called again to complete its function: Unknown error Hmm. That must be coming from this bit in libpq: /* Must have output.length > 0 */ if (output.length > PQ_GSS_SEND_BUFFER_SIZE - sizeof(uint32)) { pg_GSS_error(libpq_gettext("GSSAPI context establishment error"), conn, major, minor); gss_release_buffer(&minor, &output); return PGRES_POLLING_FAILED; } which makes it look like gss_init_sec_context wants us to send a packet larger than PQ_GSS_SEND_BUFFER_SIZE, which perhaps is a plausible thing to happen if the user belongs to enough groups. Unfortunately, elsewhere in the same file: * NOTE: The client and server have to agree on the max packet size, * because we have to pass an entire packet to GSSAPI at a time and we * don't want the other side to send arbitrarily huge packets as we * would have to allocate memory for them to then pass them to GSSAPI. * * Therefore, these two #define's are effectively part of the protocol * spec and can't ever be changed. */ #define PQ_GSS_SEND_BUFFER_SIZE 16384 #define PQ_GSS_RECV_BUFFER_SIZE 16384 Not sure where to go from here. Unfortunately the person who was mostly responsible for this code has left the project... regards, tom lane
On Thu, May 22, 2025 at 8:46 AM Tom Lane <tgl@sss.pgh.pa.us> wrote: > Hmm. That must be coming from this bit in libpq: > > /* Must have output.length > 0 */ > if (output.length > PQ_GSS_SEND_BUFFER_SIZE - sizeof(uint32)) > { > pg_GSS_error(libpq_gettext("GSSAPI context establishment error"), > conn, major, minor); > gss_release_buffer(&minor, &output); > return PGRES_POLLING_FAILED; > } > > which makes it look like gss_init_sec_context wants us to send a > packet larger than PQ_GSS_SEND_BUFFER_SIZE, which perhaps is a > plausible thing to happen if the user belongs to enough groups. Yeah, it seems like we need to be able to handle up to PG_MAX_AUTH_TOKEN_LENGTH (64k) for that initial ticket, at least? > * Therefore, these two #define's are effectively part of the protocol > * spec and can't ever be changed. > */ > #define PQ_GSS_SEND_BUFFER_SIZE 16384 > #define PQ_GSS_RECV_BUFFER_SIZE 16384 We can't increase our send buffer size without risking breakage, but a peer could choose to receive larger initial packets without issue. Then it comes down to deciding when to flip the sender into that extended mode. Unfortunately this happens prior to feature negotiation, and I don't see any obvious extension points yet. (Other than introducing a completely new negotiation code, which would make the existing fallback logic even worse than it is today.) Maybe the user could just opt in for a few releases. But also, the current behavior is just to fail hard, so if the client tries to do something extra that also sometimes fails hard, it may not really be a regression... --Jacob
Jacob Champion <jacob.champion@enterprisedb.com> writes: > On Thu, May 22, 2025 at 8:46 AM Tom Lane <tgl@sss.pgh.pa.us> wrote: >> Hmm. That must be coming from this bit in libpq: >> ... >> which makes it look like gss_init_sec_context wants us to send a >> packet larger than PQ_GSS_SEND_BUFFER_SIZE, which perhaps is a >> plausible thing to happen if the user belongs to enough groups. > Yeah, it seems like we need to be able to handle up to > PG_MAX_AUTH_TOKEN_LENGTH (64k) for that initial ticket, at least? Hmm, unfortunate that that was chosen independent of the GSS limits. > But also, the current behavior is just to fail hard, so if the client > tries to do something extra that also sometimes fails hard, it may not > really be a regression... Yeah, that's a good point. If we simply allowed the initial packet to be bigger, that would extend the set of cases that work, and if the recipient complains (because it predates that change) then it's a case that would have failed anyway, so we've not made anybody's life worse. I'm wondering though if this isn't just pushing the problem out a little further. Is there a good reason to think 64K is enough? regards, tom lane
Thanks both.
It now makes sense to me. I believe the KDC will not allow tokens larger than 65535 bytes, so feel it is safe from a GSS perspective.
Thanks
Chris
From: Tom Lane <tgl@sss.pgh.pa.us>
Sent: Thursday, May 22, 2025 5:57:14 PM
To: Jacob Champion <jacob.champion@enterprisedb.com>
Cc: Chris Gooch <cgooch@bamfunds.com>; pgsql-bugs@lists.postgresql.org <pgsql-bugs@lists.postgresql.org>
Subject: [EXT] Re: GSS Auth issue when user member of lots of AD groups
Sent: Thursday, May 22, 2025 5:57:14 PM
To: Jacob Champion <jacob.champion@enterprisedb.com>
Cc: Chris Gooch <cgooch@bamfunds.com>; pgsql-bugs@lists.postgresql.org <pgsql-bugs@lists.postgresql.org>
Subject: [EXT] Re: GSS Auth issue when user member of lots of AD groups
Jacob Champion <jacob.champion@enterprisedb.com> writes:
> On Thu, May 22, 2025 at 8:46 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Hmm. That must be coming from this bit in libpq:
>> ...
>> which makes it look like gss_init_sec_context wants us to send a
>> packet larger than PQ_GSS_SEND_BUFFER_SIZE, which perhaps is a
>> plausible thing to happen if the user belongs to enough groups.
> Yeah, it seems like we need to be able to handle up to
> PG_MAX_AUTH_TOKEN_LENGTH (64k) for that initial ticket, at least?
Hmm, unfortunate that that was chosen independent of the GSS limits.
> But also, the current behavior is just to fail hard, so if the client
> tries to do something extra that also sometimes fails hard, it may not
> really be a regression...
Yeah, that's a good point. If we simply allowed the initial packet
to be bigger, that would extend the set of cases that work, and if the
recipient complains (because it predates that change) then it's a case
that would have failed anyway, so we've not made anybody's life worse.
I'm wondering though if this isn't just pushing the problem out a
little further. Is there a good reason to think 64K is enough?
regards, tom lane
> On Thu, May 22, 2025 at 8:46 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Hmm. That must be coming from this bit in libpq:
>> ...
>> which makes it look like gss_init_sec_context wants us to send a
>> packet larger than PQ_GSS_SEND_BUFFER_SIZE, which perhaps is a
>> plausible thing to happen if the user belongs to enough groups.
> Yeah, it seems like we need to be able to handle up to
> PG_MAX_AUTH_TOKEN_LENGTH (64k) for that initial ticket, at least?
Hmm, unfortunate that that was chosen independent of the GSS limits.
> But also, the current behavior is just to fail hard, so if the client
> tries to do something extra that also sometimes fails hard, it may not
> really be a regression...
Yeah, that's a good point. If we simply allowed the initial packet
to be bigger, that would extend the set of cases that work, and if the
recipient complains (because it predates that change) then it's a case
that would have failed anyway, so we've not made anybody's life worse.
I'm wondering though if this isn't just pushing the problem out a
little further. Is there a good reason to think 64K is enough?
regards, tom lane
This email and any attachments should not be construed as an offer or recommendation to sell or buy or a solicitation of an offer to sell or buy any specific security, fund or instrument or to participate in any particular investment strategy. The information contained herein is given as of a certain date and does not purport to give information as of any other date. Although the information presented herein has been obtained from sources we believe to be reliable, no representation or warranty, expressed or implied, is made as to the accuracy or completeness of that information. Past performance is not indicative of future results.
CONFIDENTIALITY NOTICE: This message and any attachment are confidential. If you are not the intended recipient, please telephone or email the sender and delete this message and any attachment from your system. If you are not the intended recipient you must not copy this message or attachment or disclose the contents to any other persons.
Balyasny Asset Management (UK) LLP is authorised and regulated by the Financial Conduct Authority in the UK. Balyasny Asset Management LP is registered as an Investment Advisor with the Securities and Exchange Commission in the USA.
BAM prohibits all personnel from having any business related communications over text message or other unapproved communication applications. Unless pre-approved, BAM employees are only permitted to communicate over email, Bloomberg and BAM telephone lines.
CONFIDENTIALITY NOTICE: This message and any attachment are confidential. If you are not the intended recipient, please telephone or email the sender and delete this message and any attachment from your system. If you are not the intended recipient you must not copy this message or attachment or disclose the contents to any other persons.
Balyasny Asset Management (UK) LLP is authorised and regulated by the Financial Conduct Authority in the UK. Balyasny Asset Management LP is registered as an Investment Advisor with the Securities and Exchange Commission in the USA.
BAM prohibits all personnel from having any business related communications over text message or other unapproved communication applications. Unless pre-approved, BAM employees are only permitted to communicate over email, Bloomberg and BAM telephone lines.
On Thu, May 22, 2025 at 9:57 AM Tom Lane <tgl@sss.pgh.pa.us> wrote: > I'm wondering though if this isn't just pushing the problem out a > little further. Is there a good reason to think 64K is enough? Microsoft docs [1] seem to imply that there are still a bunch of existing problems if you try to go much higher, though it is possible to do so with registry tweaks. Looks like they default to 48k. Maybe we should consider making the max incoming ticket size configurable, so users that really need a bigger one can deal with the DoS risk without it affecting everyone else. (A limit on outgoing tickets probably doesn't make too much sense; I imagine you're going to use the ticket that GSSAPI hands you, no matter how big it is, because it's not as if you have a choice.) --Jacob [1] https://learn.microsoft.com/en-us/troubleshoot/windows-server/windows-security/kerberos-authentication-problems-if-user-belongs-to-groups#known-issues-that-affect-maxtokensize
Jacob Champion <jacob.champion@enterprisedb.com> writes: > On Thu, May 22, 2025 at 9:57 AM Tom Lane <tgl@sss.pgh.pa.us> wrote: >> I'm wondering though if this isn't just pushing the problem out a >> little further. Is there a good reason to think 64K is enough? > Microsoft docs [1] seem to imply that there are still a bunch of > existing problems if you try to go much higher, though it is possible > to do so with registry tweaks. Looks like they default to 48k. > Maybe we should consider making the max incoming ticket size > configurable, so users that really need a bigger one can deal with the > DoS risk without it affecting everyone else. (A limit on outgoing > tickets probably doesn't make too much sense; I imagine you're going > to use the ticket that GSSAPI hands you, no matter how big it is, > because it's not as if you have a choice.) Yeah, but we don't want to change the packet size used after the initial exchange, because that would create compatibility issues in cases that aren't failing today. I didn't look at the code to see if we can easily use a different buffer size during the authentication exchange. If we can, I'd be inclined to goose it up to 128K or so. Given Chris' point that should be plenty, so I don't feel a need to expose a knob. regards, tom lane
Hi Both, Will anyone be working on a fix for this bug?
Thanks
Chris
From: Tom Lane <tgl@sss.pgh.pa.us>
Sent: Thursday, May 22, 2025 6:58:33 PM
To: Jacob Champion <jacob.champion@enterprisedb.com>
Cc: Chris Gooch <cgooch@bamfunds.com>; pgsql-bugs@lists.postgresql.org <pgsql-bugs@lists.postgresql.org>
Subject: [EXT] Re: GSS Auth issue when user member of lots of AD groups
Sent: Thursday, May 22, 2025 6:58:33 PM
To: Jacob Champion <jacob.champion@enterprisedb.com>
Cc: Chris Gooch <cgooch@bamfunds.com>; pgsql-bugs@lists.postgresql.org <pgsql-bugs@lists.postgresql.org>
Subject: [EXT] Re: GSS Auth issue when user member of lots of AD groups
Jacob Champion <jacob.champion@enterprisedb.com> writes:
> On Thu, May 22, 2025 at 9:57 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> I'm wondering though if this isn't just pushing the problem out a
>> little further. Is there a good reason to think 64K is enough?
> Microsoft docs [1] seem to imply that there are still a bunch of
> existing problems if you try to go much higher, though it is possible
> to do so with registry tweaks. Looks like they default to 48k.
> Maybe we should consider making the max incoming ticket size
> configurable, so users that really need a bigger one can deal with the
> DoS risk without it affecting everyone else. (A limit on outgoing
> tickets probably doesn't make too much sense; I imagine you're going
> to use the ticket that GSSAPI hands you, no matter how big it is,
> because it's not as if you have a choice.)
Yeah, but we don't want to change the packet size used after the
initial exchange, because that would create compatibility issues
in cases that aren't failing today. I didn't look at the code
to see if we can easily use a different buffer size during
the authentication exchange. If we can, I'd be inclined to goose
it up to 128K or so. Given Chris' point that should be plenty,
so I don't feel a need to expose a knob.
regards, tom lane
> On Thu, May 22, 2025 at 9:57 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> I'm wondering though if this isn't just pushing the problem out a
>> little further. Is there a good reason to think 64K is enough?
> Microsoft docs [1] seem to imply that there are still a bunch of
> existing problems if you try to go much higher, though it is possible
> to do so with registry tweaks. Looks like they default to 48k.
> Maybe we should consider making the max incoming ticket size
> configurable, so users that really need a bigger one can deal with the
> DoS risk without it affecting everyone else. (A limit on outgoing
> tickets probably doesn't make too much sense; I imagine you're going
> to use the ticket that GSSAPI hands you, no matter how big it is,
> because it's not as if you have a choice.)
Yeah, but we don't want to change the packet size used after the
initial exchange, because that would create compatibility issues
in cases that aren't failing today. I didn't look at the code
to see if we can easily use a different buffer size during
the authentication exchange. If we can, I'd be inclined to goose
it up to 128K or so. Given Chris' point that should be plenty,
so I don't feel a need to expose a knob.
regards, tom lane
This email and any attachments should not be construed as an offer or recommendation to sell or buy or a solicitation of an offer to sell or buy any specific security, fund or instrument or to participate in any particular investment strategy. The information contained herein is given as of a certain date and does not purport to give information as of any other date. Although the information presented herein has been obtained from sources we believe to be reliable, no representation or warranty, expressed or implied, is made as to the accuracy or completeness of that information. Past performance is not indicative of future results.
CONFIDENTIALITY NOTICE: This message and any attachment are confidential. If you are not the intended recipient, please telephone or email the sender and delete this message and any attachment from your system. If you are not the intended recipient you must not copy this message or attachment or disclose the contents to any other persons.
Balyasny Asset Management (UK) LLP is authorised and regulated by the Financial Conduct Authority in the UK. Balyasny Asset Management LP is registered as an Investment Advisor with the Securities and Exchange Commission in the USA.
BAM prohibits all personnel from having any business related communications over text message or other unapproved communication applications. Unless pre-approved, BAM employees are only permitted to communicate over email, Bloomberg and BAM telephone lines.
CONFIDENTIALITY NOTICE: This message and any attachment are confidential. If you are not the intended recipient, please telephone or email the sender and delete this message and any attachment from your system. If you are not the intended recipient you must not copy this message or attachment or disclose the contents to any other persons.
Balyasny Asset Management (UK) LLP is authorised and regulated by the Financial Conduct Authority in the UK. Balyasny Asset Management LP is registered as an Investment Advisor with the Securities and Exchange Commission in the USA.
BAM prohibits all personnel from having any business related communications over text message or other unapproved communication applications. Unless pre-approved, BAM employees are only permitted to communicate over email, Bloomberg and BAM telephone lines.
Chris Gooch <cgooch@bamfunds.com> writes: > Hi Both, Will anyone be working on a fix for this bug? Yeah, I will pick it up (unless Jacob's already on it?) I believe we've agreed that it'd be sufficient if we allow the packets exchanged during the auth phase to be up to 64K or so, but once we reach the point where we're able to split the data on arbitrary boundaries, keep the packet size at 16K for cross-version compatibility. I did look at the code briefly, and this seems like it'd require releasing and re-alloc'ing the buffers, but that's surely doable. regards, tom lane
I wrote: > I believe we've agreed that it'd be sufficient if we allow the > packets exchanged during the auth phase to be up to 64K or so, > but once we reach the point where we're able to split the > data on arbitrary boundaries, keep the packet size at 16K > for cross-version compatibility. OK, here's a set of draft patches for that. (The HEAD one works on v16 and v17 too, the v15 one works on v14 too. They are all basically the same, but we kept revising libpq's internal convention for error reports ...) I am not in a great position to test these with a setup that actually needs larger auth messages; I wonder if Chris can test? Some notes: * Is 128kB unreasonably large? I think we may want some daylight above 64kB, but I'm not sure how much. * I concluded that the error report that's being given for the case is just flat-out bogus. The GSSAPI library has not given us an error report so asking it for info is useless, which leads to the very unhelpful error message Chris showed. We should just report "client tried to send oversize GSSAPI packet" as we do elsewhere. * It seems pretty silly to have separate symbols for PQ_GSS_SEND_BUFFER_SIZE and PQ_GSS_RECV_BUFFER_SIZE when we're requiring those to be the same, so I merged them into one symbol PQ_GSS_MAX_PACKET_SIZE. * The backend's secure_open_gssapi allowed received initialization packets to be up to buffer-size long, whereas libpq will choke sending them if they're more than buffer-size minus sizeof(uint32). This isn't actually a bug, since the buffer management is handled in such a way that it's safe, but it seems very inconsistent. I changed the limit to subtract off sizeof(uint32) in all cases, which incidentally allowed removing one variant of the translatable message string. regards, tom lane
I wrote: > OK, here's a set of draft patches for that. Sigh, it'd help if I actually attached the patches ... regards, tom lane diff --git a/src/backend/libpq/be-secure-gssapi.c b/src/backend/libpq/be-secure-gssapi.c index 717ba9824f9..910c7f22711 100644 --- a/src/backend/libpq/be-secure-gssapi.c +++ b/src/backend/libpq/be-secure-gssapi.c @@ -46,11 +46,19 @@ * don't want the other side to send arbitrarily huge packets as we * would have to allocate memory for them to then pass them to GSSAPI. * - * Therefore, these two #define's are effectively part of the protocol + * Therefore, this #define is effectively part of the protocol * spec and can't ever be changed. */ -#define PQ_GSS_SEND_BUFFER_SIZE 16384 -#define PQ_GSS_RECV_BUFFER_SIZE 16384 +#define PQ_GSS_MAX_PACKET_SIZE 16384 + +/* + * However, during the authentication exchange we must cope with whatever + * message size the GSSAPI library wants to send (because our protocol + * doesn't support splitting those messages). Depending on configuration + * those messages might be as much as 64kB. To provide some safety margin + * we use 128kB buffers during transport negotiation. + */ +#define PQ_GSS_AUTH_BUFFER_SIZE 131072 /* * Since we manage at most one GSS-encrypted connection per backend, @@ -210,12 +218,12 @@ be_gssapi_write(Port *port, const void *ptr, size_t len) errno = ECONNRESET; return -1; } - if (output.length > PQ_GSS_SEND_BUFFER_SIZE - sizeof(uint32)) + if (output.length > PQ_GSS_MAX_PACKET_SIZE - sizeof(uint32)) { ereport(COMMERROR, (errmsg("server tried to send oversize GSSAPI packet (%zu > %zu)", (size_t) output.length, - PQ_GSS_SEND_BUFFER_SIZE - sizeof(uint32)))); + PQ_GSS_MAX_PACKET_SIZE - sizeof(uint32)))); errno = ECONNRESET; return -1; } @@ -346,12 +354,12 @@ be_gssapi_read(Port *port, void *ptr, size_t len) /* Decode the packet length and check for overlength packet */ input.length = pg_ntoh32(*(uint32 *) PqGSSRecvBuffer); - if (input.length > PQ_GSS_RECV_BUFFER_SIZE - sizeof(uint32)) + if (input.length > PQ_GSS_MAX_PACKET_SIZE - sizeof(uint32)) { ereport(COMMERROR, (errmsg("oversize GSSAPI packet sent by the client (%zu > %zu)", (size_t) input.length, - PQ_GSS_RECV_BUFFER_SIZE - sizeof(uint32)))); + PQ_GSS_MAX_PACKET_SIZE - sizeof(uint32)))); errno = ECONNRESET; return -1; } @@ -517,10 +525,13 @@ secure_open_gssapi(Port *port) * that will never use them, and we ensure that the buffers are * sufficiently aligned for the length-word accesses that we do in some * places in this file. + * + * We'll use PQ_GSS_AUTH_BUFFER_SIZE-sized buffers until transport + * negotiation is complete, then switch to PQ_GSS_MAX_PACKET_SIZE. */ - PqGSSSendBuffer = malloc(PQ_GSS_SEND_BUFFER_SIZE); - PqGSSRecvBuffer = malloc(PQ_GSS_RECV_BUFFER_SIZE); - PqGSSResultBuffer = malloc(PQ_GSS_RECV_BUFFER_SIZE); + PqGSSSendBuffer = malloc(PQ_GSS_AUTH_BUFFER_SIZE); + PqGSSRecvBuffer = malloc(PQ_GSS_AUTH_BUFFER_SIZE); + PqGSSResultBuffer = malloc(PQ_GSS_AUTH_BUFFER_SIZE); if (!PqGSSSendBuffer || !PqGSSRecvBuffer || !PqGSSResultBuffer) ereport(FATAL, (errcode(ERRCODE_OUT_OF_MEMORY), @@ -568,16 +579,16 @@ secure_open_gssapi(Port *port) /* * During initialization, packets are always fully consumed and - * shouldn't ever be over PQ_GSS_RECV_BUFFER_SIZE in length. + * shouldn't ever be over PQ_GSS_AUTH_BUFFER_SIZE in total length. * * Verify on our side that the client doesn't do something funny. */ - if (input.length > PQ_GSS_RECV_BUFFER_SIZE) + if (input.length > PQ_GSS_AUTH_BUFFER_SIZE - sizeof(uint32)) { ereport(COMMERROR, - (errmsg("oversize GSSAPI packet sent by the client (%zu > %d)", + (errmsg("oversize GSSAPI packet sent by the client (%zu > %zu)", (size_t) input.length, - PQ_GSS_RECV_BUFFER_SIZE))); + PQ_GSS_AUTH_BUFFER_SIZE - sizeof(uint32)))); return -1; } @@ -631,12 +642,12 @@ secure_open_gssapi(Port *port) { uint32 netlen = pg_hton32(output.length); - if (output.length > PQ_GSS_SEND_BUFFER_SIZE - sizeof(uint32)) + if (output.length > PQ_GSS_AUTH_BUFFER_SIZE - sizeof(uint32)) { ereport(COMMERROR, (errmsg("server tried to send oversize GSSAPI packet (%zu > %zu)", (size_t) output.length, - PQ_GSS_SEND_BUFFER_SIZE - sizeof(uint32)))); + PQ_GSS_AUTH_BUFFER_SIZE - sizeof(uint32)))); gss_release_buffer(&minor, &output); return -1; } @@ -691,12 +702,29 @@ secure_open_gssapi(Port *port) break; } + /* + * Release the large authentication buffers and allocate the ones we want + * for normal operation. + */ + free(PqGSSSendBuffer); + free(PqGSSRecvBuffer); + free(PqGSSResultBuffer); + PqGSSSendBuffer = malloc(PQ_GSS_MAX_PACKET_SIZE); + PqGSSRecvBuffer = malloc(PQ_GSS_MAX_PACKET_SIZE); + PqGSSResultBuffer = malloc(PQ_GSS_MAX_PACKET_SIZE); + if (!PqGSSSendBuffer || !PqGSSRecvBuffer || !PqGSSResultBuffer) + ereport(FATAL, + (errcode(ERRCODE_OUT_OF_MEMORY), + errmsg("out of memory"))); + PqGSSSendLength = PqGSSSendNext = PqGSSSendConsumed = 0; + PqGSSRecvLength = PqGSSResultLength = PqGSSResultNext = 0; + /* * Determine the max packet size which will fit in our buffer, after * accounting for the length. be_gssapi_write will need this. */ major = gss_wrap_size_limit(&minor, port->gss->ctx, 1, GSS_C_QOP_DEFAULT, - PQ_GSS_SEND_BUFFER_SIZE - sizeof(uint32), + PQ_GSS_MAX_PACKET_SIZE - sizeof(uint32), &PqGSSMaxPktSize); if (GSS_ERROR(major)) diff --git a/src/interfaces/libpq/fe-secure-gssapi.c b/src/interfaces/libpq/fe-secure-gssapi.c index ce183bc04b4..9bdf0192f1e 100644 --- a/src/interfaces/libpq/fe-secure-gssapi.c +++ b/src/interfaces/libpq/fe-secure-gssapi.c @@ -47,11 +47,19 @@ * don't want the other side to send arbitrarily huge packets as we * would have to allocate memory for them to then pass them to GSSAPI. * - * Therefore, these two #define's are effectively part of the protocol + * Therefore, this #define is effectively part of the protocol * spec and can't ever be changed. */ -#define PQ_GSS_SEND_BUFFER_SIZE 16384 -#define PQ_GSS_RECV_BUFFER_SIZE 16384 +#define PQ_GSS_MAX_PACKET_SIZE 16384 + +/* + * However, during the authentication exchange we must cope with whatever + * message size the GSSAPI library wants to send (because our protocol + * doesn't support splitting those messages). Depending on configuration + * those messages might be as much as 64kB. To provide some safety margin + * we use 128kB buffers during transport negotiation. + */ +#define PQ_GSS_AUTH_BUFFER_SIZE 131072 /* * We need these state variables per-connection. To allow the functions @@ -203,11 +211,11 @@ pg_GSS_write(PGconn *conn, const void *ptr, size_t len) goto cleanup; } - if (output.length > PQ_GSS_SEND_BUFFER_SIZE - sizeof(uint32)) + if (output.length > PQ_GSS_MAX_PACKET_SIZE - sizeof(uint32)) { libpq_append_conn_error(conn, "client tried to send oversize GSSAPI packet (%zu > %zu)", (size_t) output.length, - PQ_GSS_SEND_BUFFER_SIZE - sizeof(uint32)); + PQ_GSS_MAX_PACKET_SIZE - sizeof(uint32)); errno = EIO; /* for lack of a better idea */ goto cleanup; } @@ -342,11 +350,11 @@ pg_GSS_read(PGconn *conn, void *ptr, size_t len) /* Decode the packet length and check for overlength packet */ input.length = pg_ntoh32(*(uint32 *) PqGSSRecvBuffer); - if (input.length > PQ_GSS_RECV_BUFFER_SIZE - sizeof(uint32)) + if (input.length > PQ_GSS_MAX_PACKET_SIZE - sizeof(uint32)) { libpq_append_conn_error(conn, "oversize GSSAPI packet sent by the server (%zu > %zu)", (size_t) input.length, - PQ_GSS_RECV_BUFFER_SIZE - sizeof(uint32)); + PQ_GSS_MAX_PACKET_SIZE - sizeof(uint32)); errno = EIO; /* for lack of a better idea */ return -1; } @@ -485,12 +493,15 @@ pqsecure_open_gss(PGconn *conn) * initialize state variables. By malloc'ing the buffers separately, we * ensure that they are sufficiently aligned for the length-word accesses * that we do in some places in this file. + * + * We'll use PQ_GSS_AUTH_BUFFER_SIZE-sized buffers until transport + * negotiation is complete, then switch to PQ_GSS_MAX_PACKET_SIZE. */ if (PqGSSSendBuffer == NULL) { - PqGSSSendBuffer = malloc(PQ_GSS_SEND_BUFFER_SIZE); - PqGSSRecvBuffer = malloc(PQ_GSS_RECV_BUFFER_SIZE); - PqGSSResultBuffer = malloc(PQ_GSS_RECV_BUFFER_SIZE); + PqGSSSendBuffer = malloc(PQ_GSS_AUTH_BUFFER_SIZE); + PqGSSRecvBuffer = malloc(PQ_GSS_AUTH_BUFFER_SIZE); + PqGSSResultBuffer = malloc(PQ_GSS_AUTH_BUFFER_SIZE); if (!PqGSSSendBuffer || !PqGSSRecvBuffer || !PqGSSResultBuffer) { libpq_append_conn_error(conn, "out of memory"); @@ -564,13 +575,13 @@ pqsecure_open_gss(PGconn *conn) * so leave a spot at the end for a NULL byte too) and report that * back to the caller. */ - result = gss_read(conn, PqGSSRecvBuffer + PqGSSRecvLength, PQ_GSS_RECV_BUFFER_SIZE - PqGSSRecvLength - 1, &ret); + result = gss_read(conn, PqGSSRecvBuffer + PqGSSRecvLength, PQ_GSS_AUTH_BUFFER_SIZE - PqGSSRecvLength - 1, &ret); if (result != PGRES_POLLING_OK) return result; PqGSSRecvLength += ret; - Assert(PqGSSRecvLength < PQ_GSS_RECV_BUFFER_SIZE); + Assert(PqGSSRecvLength < PQ_GSS_AUTH_BUFFER_SIZE); PqGSSRecvBuffer[PqGSSRecvLength] = '\0'; appendPQExpBuffer(&conn->errorMessage, "%s\n", PqGSSRecvBuffer + 1); @@ -584,11 +595,11 @@ pqsecure_open_gss(PGconn *conn) /* Get the length and check for over-length packet */ input.length = pg_ntoh32(*(uint32 *) PqGSSRecvBuffer); - if (input.length > PQ_GSS_RECV_BUFFER_SIZE - sizeof(uint32)) + if (input.length > PQ_GSS_AUTH_BUFFER_SIZE - sizeof(uint32)) { libpq_append_conn_error(conn, "oversize GSSAPI packet sent by the server (%zu > %zu)", (size_t) input.length, - PQ_GSS_RECV_BUFFER_SIZE - sizeof(uint32)); + PQ_GSS_AUTH_BUFFER_SIZE - sizeof(uint32)); return PGRES_POLLING_FAILED; } @@ -668,12 +679,33 @@ pqsecure_open_gss(PGconn *conn) conn->gcred = GSS_C_NO_CREDENTIAL; gss_release_buffer(&minor, &output); + /* + * Release the large authentication buffers and allocate the ones we + * want for normal operation. (This maneuver is safe only because + * pqDropConnection will drop the buffers; otherwise, during a + * reconnection we'd be at risk of using undersized buffers during + * negotiation.) + */ + free(PqGSSSendBuffer); + free(PqGSSRecvBuffer); + free(PqGSSResultBuffer); + PqGSSSendBuffer = malloc(PQ_GSS_MAX_PACKET_SIZE); + PqGSSRecvBuffer = malloc(PQ_GSS_MAX_PACKET_SIZE); + PqGSSResultBuffer = malloc(PQ_GSS_MAX_PACKET_SIZE); + if (!PqGSSSendBuffer || !PqGSSRecvBuffer || !PqGSSResultBuffer) + { + libpq_append_conn_error(conn, "out of memory"); + return PGRES_POLLING_FAILED; + } + PqGSSSendLength = PqGSSSendNext = PqGSSSendConsumed = 0; + PqGSSRecvLength = PqGSSResultLength = PqGSSResultNext = 0; + /* * Determine the max packet size which will fit in our buffer, after * accounting for the length. pg_GSS_write will need this. */ major = gss_wrap_size_limit(&minor, conn->gctx, 1, GSS_C_QOP_DEFAULT, - PQ_GSS_SEND_BUFFER_SIZE - sizeof(uint32), + PQ_GSS_MAX_PACKET_SIZE - sizeof(uint32), &PqGSSMaxPktSize); if (GSS_ERROR(major)) @@ -687,10 +719,11 @@ pqsecure_open_gss(PGconn *conn) } /* Must have output.length > 0 */ - if (output.length > PQ_GSS_SEND_BUFFER_SIZE - sizeof(uint32)) + if (output.length > PQ_GSS_AUTH_BUFFER_SIZE - sizeof(uint32)) { - pg_GSS_error(libpq_gettext("GSSAPI context establishment error"), - conn, major, minor); + libpq_append_conn_error(conn, "client tried to send oversize GSSAPI packet (%zu > %zu)", + (size_t) output.length, + PQ_GSS_AUTH_BUFFER_SIZE - sizeof(uint32)); gss_release_buffer(&minor, &output); return PGRES_POLLING_FAILED; } diff --git a/src/backend/libpq/be-secure-gssapi.c b/src/backend/libpq/be-secure-gssapi.c index d3337a3d1c4..4aa828bef08 100644 --- a/src/backend/libpq/be-secure-gssapi.c +++ b/src/backend/libpq/be-secure-gssapi.c @@ -45,11 +45,19 @@ * don't want the other side to send arbitrarily huge packets as we * would have to allocate memory for them to then pass them to GSSAPI. * - * Therefore, these two #define's are effectively part of the protocol + * Therefore, this #define is effectively part of the protocol * spec and can't ever be changed. */ -#define PQ_GSS_SEND_BUFFER_SIZE 16384 -#define PQ_GSS_RECV_BUFFER_SIZE 16384 +#define PQ_GSS_MAX_PACKET_SIZE 16384 + +/* + * However, during the authentication exchange we must cope with whatever + * message size the GSSAPI library wants to send (because our protocol + * doesn't support splitting those messages). Depending on configuration + * those messages might be as much as 64kB. To provide some safety margin + * we use 128kB buffers during transport negotiation. + */ +#define PQ_GSS_AUTH_BUFFER_SIZE 131072 /* * Since we manage at most one GSS-encrypted connection per backend, @@ -209,12 +217,12 @@ be_gssapi_write(Port *port, void *ptr, size_t len) errno = ECONNRESET; return -1; } - if (output.length > PQ_GSS_SEND_BUFFER_SIZE - sizeof(uint32)) + if (output.length > PQ_GSS_MAX_PACKET_SIZE - sizeof(uint32)) { ereport(COMMERROR, (errmsg("server tried to send oversize GSSAPI packet (%zu > %zu)", (size_t) output.length, - PQ_GSS_SEND_BUFFER_SIZE - sizeof(uint32)))); + PQ_GSS_MAX_PACKET_SIZE - sizeof(uint32)))); errno = ECONNRESET; return -1; } @@ -345,12 +353,12 @@ be_gssapi_read(Port *port, void *ptr, size_t len) /* Decode the packet length and check for overlength packet */ input.length = pg_ntoh32(*(uint32 *) PqGSSRecvBuffer); - if (input.length > PQ_GSS_RECV_BUFFER_SIZE - sizeof(uint32)) + if (input.length > PQ_GSS_MAX_PACKET_SIZE - sizeof(uint32)) { ereport(COMMERROR, (errmsg("oversize GSSAPI packet sent by the client (%zu > %zu)", (size_t) input.length, - PQ_GSS_RECV_BUFFER_SIZE - sizeof(uint32)))); + PQ_GSS_MAX_PACKET_SIZE - sizeof(uint32)))); errno = ECONNRESET; return -1; } @@ -510,10 +518,13 @@ secure_open_gssapi(Port *port) * that will never use them, and we ensure that the buffers are * sufficiently aligned for the length-word accesses that we do in some * places in this file. + * + * We'll use PQ_GSS_AUTH_BUFFER_SIZE-sized buffers until transport + * negotiation is complete, then switch to PQ_GSS_MAX_PACKET_SIZE. */ - PqGSSSendBuffer = malloc(PQ_GSS_SEND_BUFFER_SIZE); - PqGSSRecvBuffer = malloc(PQ_GSS_RECV_BUFFER_SIZE); - PqGSSResultBuffer = malloc(PQ_GSS_RECV_BUFFER_SIZE); + PqGSSSendBuffer = malloc(PQ_GSS_AUTH_BUFFER_SIZE); + PqGSSRecvBuffer = malloc(PQ_GSS_AUTH_BUFFER_SIZE); + PqGSSResultBuffer = malloc(PQ_GSS_AUTH_BUFFER_SIZE); if (!PqGSSSendBuffer || !PqGSSRecvBuffer || !PqGSSResultBuffer) ereport(FATAL, (errcode(ERRCODE_OUT_OF_MEMORY), @@ -560,16 +571,16 @@ secure_open_gssapi(Port *port) /* * During initialization, packets are always fully consumed and - * shouldn't ever be over PQ_GSS_RECV_BUFFER_SIZE in length. + * shouldn't ever be over PQ_GSS_AUTH_BUFFER_SIZE in total length. * * Verify on our side that the client doesn't do something funny. */ - if (input.length > PQ_GSS_RECV_BUFFER_SIZE) + if (input.length > PQ_GSS_AUTH_BUFFER_SIZE - sizeof(uint32)) { ereport(COMMERROR, - (errmsg("oversize GSSAPI packet sent by the client (%zu > %d)", + (errmsg("oversize GSSAPI packet sent by the client (%zu > %zu)", (size_t) input.length, - PQ_GSS_RECV_BUFFER_SIZE))); + PQ_GSS_AUTH_BUFFER_SIZE - sizeof(uint32)))); return -1; } @@ -616,12 +627,12 @@ secure_open_gssapi(Port *port) { uint32 netlen = pg_hton32(output.length); - if (output.length > PQ_GSS_SEND_BUFFER_SIZE - sizeof(uint32)) + if (output.length > PQ_GSS_AUTH_BUFFER_SIZE - sizeof(uint32)) { ereport(COMMERROR, (errmsg("server tried to send oversize GSSAPI packet (%zu > %zu)", (size_t) output.length, - PQ_GSS_SEND_BUFFER_SIZE - sizeof(uint32)))); + PQ_GSS_AUTH_BUFFER_SIZE - sizeof(uint32)))); gss_release_buffer(&minor, &output); return -1; } @@ -676,12 +687,29 @@ secure_open_gssapi(Port *port) break; } + /* + * Release the large authentication buffers and allocate the ones we want + * for normal operation. + */ + free(PqGSSSendBuffer); + free(PqGSSRecvBuffer); + free(PqGSSResultBuffer); + PqGSSSendBuffer = malloc(PQ_GSS_MAX_PACKET_SIZE); + PqGSSRecvBuffer = malloc(PQ_GSS_MAX_PACKET_SIZE); + PqGSSResultBuffer = malloc(PQ_GSS_MAX_PACKET_SIZE); + if (!PqGSSSendBuffer || !PqGSSRecvBuffer || !PqGSSResultBuffer) + ereport(FATAL, + (errcode(ERRCODE_OUT_OF_MEMORY), + errmsg("out of memory"))); + PqGSSSendLength = PqGSSSendNext = PqGSSSendConsumed = 0; + PqGSSRecvLength = PqGSSResultLength = PqGSSResultNext = 0; + /* * Determine the max packet size which will fit in our buffer, after * accounting for the length. be_gssapi_write will need this. */ major = gss_wrap_size_limit(&minor, port->gss->ctx, 1, GSS_C_QOP_DEFAULT, - PQ_GSS_SEND_BUFFER_SIZE - sizeof(uint32), + PQ_GSS_MAX_PACKET_SIZE - sizeof(uint32), &PqGSSMaxPktSize); if (GSS_ERROR(major)) diff --git a/src/interfaces/libpq/fe-secure-gssapi.c b/src/interfaces/libpq/fe-secure-gssapi.c index a3768cd70a3..eb23731645f 100644 --- a/src/interfaces/libpq/fe-secure-gssapi.c +++ b/src/interfaces/libpq/fe-secure-gssapi.c @@ -47,11 +47,19 @@ * don't want the other side to send arbitrarily huge packets as we * would have to allocate memory for them to then pass them to GSSAPI. * - * Therefore, these two #define's are effectively part of the protocol + * Therefore, this #define is effectively part of the protocol * spec and can't ever be changed. */ -#define PQ_GSS_SEND_BUFFER_SIZE 16384 -#define PQ_GSS_RECV_BUFFER_SIZE 16384 +#define PQ_GSS_MAX_PACKET_SIZE 16384 + +/* + * However, during the authentication exchange we must cope with whatever + * message size the GSSAPI library wants to send (because our protocol + * doesn't support splitting those messages). Depending on configuration + * those messages might be as much as 64kB. To provide some safety margin + * we use 128kB buffers during transport negotiation. + */ +#define PQ_GSS_AUTH_BUFFER_SIZE 131072 /* * We need these state variables per-connection. To allow the functions @@ -204,12 +212,12 @@ pg_GSS_write(PGconn *conn, const void *ptr, size_t len) goto cleanup; } - if (output.length > PQ_GSS_SEND_BUFFER_SIZE - sizeof(uint32)) + if (output.length > PQ_GSS_MAX_PACKET_SIZE - sizeof(uint32)) { appendPQExpBuffer(&conn->errorMessage, libpq_gettext("client tried to send oversize GSSAPI packet (%zu > %zu)\n"), (size_t) output.length, - PQ_GSS_SEND_BUFFER_SIZE - sizeof(uint32)); + PQ_GSS_MAX_PACKET_SIZE - sizeof(uint32)); errno = EIO; /* for lack of a better idea */ goto cleanup; } @@ -344,12 +352,12 @@ pg_GSS_read(PGconn *conn, void *ptr, size_t len) /* Decode the packet length and check for overlength packet */ input.length = pg_ntoh32(*(uint32 *) PqGSSRecvBuffer); - if (input.length > PQ_GSS_RECV_BUFFER_SIZE - sizeof(uint32)) + if (input.length > PQ_GSS_MAX_PACKET_SIZE - sizeof(uint32)) { appendPQExpBuffer(&conn->errorMessage, libpq_gettext("oversize GSSAPI packet sent by the server (%zu > %zu)\n"), (size_t) input.length, - PQ_GSS_RECV_BUFFER_SIZE - sizeof(uint32)); + PQ_GSS_MAX_PACKET_SIZE - sizeof(uint32)); errno = EIO; /* for lack of a better idea */ return -1; } @@ -488,12 +496,15 @@ pqsecure_open_gss(PGconn *conn) * initialize state variables. By malloc'ing the buffers separately, we * ensure that they are sufficiently aligned for the length-word accesses * that we do in some places in this file. + * + * We'll use PQ_GSS_AUTH_BUFFER_SIZE-sized buffers until transport + * negotiation is complete, then switch to PQ_GSS_MAX_PACKET_SIZE. */ if (PqGSSSendBuffer == NULL) { - PqGSSSendBuffer = malloc(PQ_GSS_SEND_BUFFER_SIZE); - PqGSSRecvBuffer = malloc(PQ_GSS_RECV_BUFFER_SIZE); - PqGSSResultBuffer = malloc(PQ_GSS_RECV_BUFFER_SIZE); + PqGSSSendBuffer = malloc(PQ_GSS_AUTH_BUFFER_SIZE); + PqGSSRecvBuffer = malloc(PQ_GSS_AUTH_BUFFER_SIZE); + PqGSSResultBuffer = malloc(PQ_GSS_AUTH_BUFFER_SIZE); if (!PqGSSSendBuffer || !PqGSSRecvBuffer || !PqGSSResultBuffer) { appendPQExpBufferStr(&conn->errorMessage, @@ -568,13 +579,13 @@ pqsecure_open_gss(PGconn *conn) * so leave a spot at the end for a NULL byte too) and report that * back to the caller. */ - result = gss_read(conn, PqGSSRecvBuffer + PqGSSRecvLength, PQ_GSS_RECV_BUFFER_SIZE - PqGSSRecvLength - 1, &ret); + result = gss_read(conn, PqGSSRecvBuffer + PqGSSRecvLength, PQ_GSS_AUTH_BUFFER_SIZE - PqGSSRecvLength - 1, &ret); if (result != PGRES_POLLING_OK) return result; PqGSSRecvLength += ret; - Assert(PqGSSRecvLength < PQ_GSS_RECV_BUFFER_SIZE); + Assert(PqGSSRecvLength < PQ_GSS_AUTH_BUFFER_SIZE); PqGSSRecvBuffer[PqGSSRecvLength] = '\0'; appendPQExpBuffer(&conn->errorMessage, "%s\n", PqGSSRecvBuffer + 1); @@ -588,12 +599,12 @@ pqsecure_open_gss(PGconn *conn) /* Get the length and check for over-length packet */ input.length = pg_ntoh32(*(uint32 *) PqGSSRecvBuffer); - if (input.length > PQ_GSS_RECV_BUFFER_SIZE - sizeof(uint32)) + if (input.length > PQ_GSS_AUTH_BUFFER_SIZE - sizeof(uint32)) { appendPQExpBuffer(&conn->errorMessage, libpq_gettext("oversize GSSAPI packet sent by the server (%zu > %zu)\n"), (size_t) input.length, - PQ_GSS_RECV_BUFFER_SIZE - sizeof(uint32)); + PQ_GSS_AUTH_BUFFER_SIZE - sizeof(uint32)); return PGRES_POLLING_FAILED; } @@ -655,12 +666,34 @@ pqsecure_open_gss(PGconn *conn) conn->gcred = GSS_C_NO_CREDENTIAL; gss_release_buffer(&minor, &output); + /* + * Release the large authentication buffers and allocate the ones we + * want for normal operation. (This maneuver is safe only because + * pqDropConnection will drop the buffers; otherwise, during a + * reconnection we'd be at risk of using undersized buffers during + * negotiation.) + */ + free(PqGSSSendBuffer); + free(PqGSSRecvBuffer); + free(PqGSSResultBuffer); + PqGSSSendBuffer = malloc(PQ_GSS_MAX_PACKET_SIZE); + PqGSSRecvBuffer = malloc(PQ_GSS_MAX_PACKET_SIZE); + PqGSSResultBuffer = malloc(PQ_GSS_MAX_PACKET_SIZE); + if (!PqGSSSendBuffer || !PqGSSRecvBuffer || !PqGSSResultBuffer) + { + appendPQExpBufferStr(&conn->errorMessage, + libpq_gettext("out of memory\n")); + return PGRES_POLLING_FAILED; + } + PqGSSSendLength = PqGSSSendNext = PqGSSSendConsumed = 0; + PqGSSRecvLength = PqGSSResultLength = PqGSSResultNext = 0; + /* * Determine the max packet size which will fit in our buffer, after * accounting for the length. pg_GSS_write will need this. */ major = gss_wrap_size_limit(&minor, conn->gctx, 1, GSS_C_QOP_DEFAULT, - PQ_GSS_SEND_BUFFER_SIZE - sizeof(uint32), + PQ_GSS_MAX_PACKET_SIZE - sizeof(uint32), &PqGSSMaxPktSize); if (GSS_ERROR(major)) @@ -674,10 +707,12 @@ pqsecure_open_gss(PGconn *conn) } /* Must have output.length > 0 */ - if (output.length > PQ_GSS_SEND_BUFFER_SIZE - sizeof(uint32)) + if (output.length > PQ_GSS_AUTH_BUFFER_SIZE - sizeof(uint32)) { - pg_GSS_error(libpq_gettext("GSSAPI context establishment error"), - conn, major, minor); + appendPQExpBuffer(&conn->errorMessage, + libpq_gettext("client tried to send oversize GSSAPI packet (%zu > %zu)\n"), + (size_t) output.length, + PQ_GSS_AUTH_BUFFER_SIZE - sizeof(uint32)); gss_release_buffer(&minor, &output); return PGRES_POLLING_FAILED; } diff --git a/src/backend/libpq/be-secure-gssapi.c b/src/backend/libpq/be-secure-gssapi.c index 0fc7d565e2a..4992ff5bd20 100644 --- a/src/backend/libpq/be-secure-gssapi.c +++ b/src/backend/libpq/be-secure-gssapi.c @@ -45,11 +45,19 @@ * don't want the other side to send arbitrarily huge packets as we * would have to allocate memory for them to then pass them to GSSAPI. * - * Therefore, these two #define's are effectively part of the protocol + * Therefore, this #define is effectively part of the protocol * spec and can't ever be changed. */ -#define PQ_GSS_SEND_BUFFER_SIZE 16384 -#define PQ_GSS_RECV_BUFFER_SIZE 16384 +#define PQ_GSS_MAX_PACKET_SIZE 16384 + +/* + * However, during the authentication exchange we must cope with whatever + * message size the GSSAPI library wants to send (because our protocol + * doesn't support splitting those messages). Depending on configuration + * those messages might be as much as 64kB. To provide some safety margin + * we use 128kB buffers during transport negotiation. + */ +#define PQ_GSS_AUTH_BUFFER_SIZE 131072 /* * Since we manage at most one GSS-encrypted connection per backend, @@ -209,12 +217,12 @@ be_gssapi_write(Port *port, void *ptr, size_t len) errno = ECONNRESET; return -1; } - if (output.length > PQ_GSS_SEND_BUFFER_SIZE - sizeof(uint32)) + if (output.length > PQ_GSS_MAX_PACKET_SIZE - sizeof(uint32)) { ereport(COMMERROR, (errmsg("server tried to send oversize GSSAPI packet (%zu > %zu)", (size_t) output.length, - PQ_GSS_SEND_BUFFER_SIZE - sizeof(uint32)))); + PQ_GSS_MAX_PACKET_SIZE - sizeof(uint32)))); errno = ECONNRESET; return -1; } @@ -345,12 +353,12 @@ be_gssapi_read(Port *port, void *ptr, size_t len) /* Decode the packet length and check for overlength packet */ input.length = ntohl(*(uint32 *) PqGSSRecvBuffer); - if (input.length > PQ_GSS_RECV_BUFFER_SIZE - sizeof(uint32)) + if (input.length > PQ_GSS_MAX_PACKET_SIZE - sizeof(uint32)) { ereport(COMMERROR, (errmsg("oversize GSSAPI packet sent by the client (%zu > %zu)", (size_t) input.length, - PQ_GSS_RECV_BUFFER_SIZE - sizeof(uint32)))); + PQ_GSS_MAX_PACKET_SIZE - sizeof(uint32)))); errno = ECONNRESET; return -1; } @@ -510,10 +518,13 @@ secure_open_gssapi(Port *port) * that will never use them, and we ensure that the buffers are * sufficiently aligned for the length-word accesses that we do in some * places in this file. + * + * We'll use PQ_GSS_AUTH_BUFFER_SIZE-sized buffers until transport + * negotiation is complete, then switch to PQ_GSS_MAX_PACKET_SIZE. */ - PqGSSSendBuffer = malloc(PQ_GSS_SEND_BUFFER_SIZE); - PqGSSRecvBuffer = malloc(PQ_GSS_RECV_BUFFER_SIZE); - PqGSSResultBuffer = malloc(PQ_GSS_RECV_BUFFER_SIZE); + PqGSSSendBuffer = malloc(PQ_GSS_AUTH_BUFFER_SIZE); + PqGSSRecvBuffer = malloc(PQ_GSS_AUTH_BUFFER_SIZE); + PqGSSResultBuffer = malloc(PQ_GSS_AUTH_BUFFER_SIZE); if (!PqGSSSendBuffer || !PqGSSRecvBuffer || !PqGSSResultBuffer) ereport(FATAL, (errcode(ERRCODE_OUT_OF_MEMORY), @@ -560,16 +571,16 @@ secure_open_gssapi(Port *port) /* * During initialization, packets are always fully consumed and - * shouldn't ever be over PQ_GSS_RECV_BUFFER_SIZE in length. + * shouldn't ever be over PQ_GSS_AUTH_BUFFER_SIZE in total length. * * Verify on our side that the client doesn't do something funny. */ - if (input.length > PQ_GSS_RECV_BUFFER_SIZE) + if (input.length > PQ_GSS_AUTH_BUFFER_SIZE - sizeof(uint32)) { ereport(COMMERROR, - (errmsg("oversize GSSAPI packet sent by the client (%zu > %d)", + (errmsg("oversize GSSAPI packet sent by the client (%zu > %zu)", (size_t) input.length, - PQ_GSS_RECV_BUFFER_SIZE))); + PQ_GSS_AUTH_BUFFER_SIZE - sizeof(uint32)))); return -1; } @@ -616,12 +627,12 @@ secure_open_gssapi(Port *port) { uint32 netlen = htonl(output.length); - if (output.length > PQ_GSS_SEND_BUFFER_SIZE - sizeof(uint32)) + if (output.length > PQ_GSS_AUTH_BUFFER_SIZE - sizeof(uint32)) { ereport(COMMERROR, (errmsg("server tried to send oversize GSSAPI packet (%zu > %zu)", (size_t) output.length, - PQ_GSS_SEND_BUFFER_SIZE - sizeof(uint32)))); + PQ_GSS_AUTH_BUFFER_SIZE - sizeof(uint32)))); gss_release_buffer(&minor, &output); return -1; } @@ -676,12 +687,29 @@ secure_open_gssapi(Port *port) break; } + /* + * Release the large authentication buffers and allocate the ones we want + * for normal operation. + */ + free(PqGSSSendBuffer); + free(PqGSSRecvBuffer); + free(PqGSSResultBuffer); + PqGSSSendBuffer = malloc(PQ_GSS_MAX_PACKET_SIZE); + PqGSSRecvBuffer = malloc(PQ_GSS_MAX_PACKET_SIZE); + PqGSSResultBuffer = malloc(PQ_GSS_MAX_PACKET_SIZE); + if (!PqGSSSendBuffer || !PqGSSRecvBuffer || !PqGSSResultBuffer) + ereport(FATAL, + (errcode(ERRCODE_OUT_OF_MEMORY), + errmsg("out of memory"))); + PqGSSSendLength = PqGSSSendNext = PqGSSSendConsumed = 0; + PqGSSRecvLength = PqGSSResultLength = PqGSSResultNext = 0; + /* * Determine the max packet size which will fit in our buffer, after * accounting for the length. be_gssapi_write will need this. */ major = gss_wrap_size_limit(&minor, port->gss->ctx, 1, GSS_C_QOP_DEFAULT, - PQ_GSS_SEND_BUFFER_SIZE - sizeof(uint32), + PQ_GSS_MAX_PACKET_SIZE - sizeof(uint32), &PqGSSMaxPktSize); if (GSS_ERROR(major)) diff --git a/src/interfaces/libpq/fe-secure-gssapi.c b/src/interfaces/libpq/fe-secure-gssapi.c index ac2a8c8563f..abc07fe5f04 100644 --- a/src/interfaces/libpq/fe-secure-gssapi.c +++ b/src/interfaces/libpq/fe-secure-gssapi.c @@ -47,11 +47,19 @@ * don't want the other side to send arbitrarily huge packets as we * would have to allocate memory for them to then pass them to GSSAPI. * - * Therefore, these two #define's are effectively part of the protocol + * Therefore, this #define is effectively part of the protocol * spec and can't ever be changed. */ -#define PQ_GSS_SEND_BUFFER_SIZE 16384 -#define PQ_GSS_RECV_BUFFER_SIZE 16384 +#define PQ_GSS_MAX_PACKET_SIZE 16384 + +/* + * However, during the authentication exchange we must cope with whatever + * message size the GSSAPI library wants to send (because our protocol + * doesn't support splitting those messages). Depending on configuration + * those messages might be as much as 64kB. To provide some safety margin + * we use 128kB buffers during transport negotiation. + */ +#define PQ_GSS_AUTH_BUFFER_SIZE 131072 /* * We need these state variables per-connection. To allow the functions @@ -204,12 +212,12 @@ pg_GSS_write(PGconn *conn, const void *ptr, size_t len) goto cleanup; } - if (output.length > PQ_GSS_SEND_BUFFER_SIZE - sizeof(uint32)) + if (output.length > PQ_GSS_MAX_PACKET_SIZE - sizeof(uint32)) { printfPQExpBuffer(&conn->errorMessage, libpq_gettext("client tried to send oversize GSSAPI packet (%zu > %zu)\n"), (size_t) output.length, - PQ_GSS_SEND_BUFFER_SIZE - sizeof(uint32)); + PQ_GSS_MAX_PACKET_SIZE - sizeof(uint32)); errno = EIO; /* for lack of a better idea */ goto cleanup; } @@ -344,12 +352,12 @@ pg_GSS_read(PGconn *conn, void *ptr, size_t len) /* Decode the packet length and check for overlength packet */ input.length = ntohl(*(uint32 *) PqGSSRecvBuffer); - if (input.length > PQ_GSS_RECV_BUFFER_SIZE - sizeof(uint32)) + if (input.length > PQ_GSS_MAX_PACKET_SIZE - sizeof(uint32)) { printfPQExpBuffer(&conn->errorMessage, libpq_gettext("oversize GSSAPI packet sent by the server (%zu > %zu)\n"), (size_t) input.length, - PQ_GSS_RECV_BUFFER_SIZE - sizeof(uint32)); + PQ_GSS_MAX_PACKET_SIZE - sizeof(uint32)); errno = EIO; /* for lack of a better idea */ return -1; } @@ -488,12 +496,15 @@ pqsecure_open_gss(PGconn *conn) * initialize state variables. By malloc'ing the buffers separately, we * ensure that they are sufficiently aligned for the length-word accesses * that we do in some places in this file. + * + * We'll use PQ_GSS_AUTH_BUFFER_SIZE-sized buffers until transport + * negotiation is complete, then switch to PQ_GSS_MAX_PACKET_SIZE. */ if (PqGSSSendBuffer == NULL) { - PqGSSSendBuffer = malloc(PQ_GSS_SEND_BUFFER_SIZE); - PqGSSRecvBuffer = malloc(PQ_GSS_RECV_BUFFER_SIZE); - PqGSSResultBuffer = malloc(PQ_GSS_RECV_BUFFER_SIZE); + PqGSSSendBuffer = malloc(PQ_GSS_AUTH_BUFFER_SIZE); + PqGSSRecvBuffer = malloc(PQ_GSS_AUTH_BUFFER_SIZE); + PqGSSResultBuffer = malloc(PQ_GSS_AUTH_BUFFER_SIZE); if (!PqGSSSendBuffer || !PqGSSRecvBuffer || !PqGSSResultBuffer) { printfPQExpBuffer(&conn->errorMessage, @@ -568,13 +579,13 @@ pqsecure_open_gss(PGconn *conn) * so leave a spot at the end for a NULL byte too) and report that * back to the caller. */ - result = gss_read(conn, PqGSSRecvBuffer + PqGSSRecvLength, PQ_GSS_RECV_BUFFER_SIZE - PqGSSRecvLength - 1, &ret); + result = gss_read(conn, PqGSSRecvBuffer + PqGSSRecvLength, PQ_GSS_AUTH_BUFFER_SIZE - PqGSSRecvLength - 1, &ret); if (result != PGRES_POLLING_OK) return result; PqGSSRecvLength += ret; - Assert(PqGSSRecvLength < PQ_GSS_RECV_BUFFER_SIZE); + Assert(PqGSSRecvLength < PQ_GSS_AUTH_BUFFER_SIZE); PqGSSRecvBuffer[PqGSSRecvLength] = '\0'; printfPQExpBuffer(&conn->errorMessage, "%s\n", PqGSSRecvBuffer + 1); @@ -588,12 +599,12 @@ pqsecure_open_gss(PGconn *conn) /* Get the length and check for over-length packet */ input.length = ntohl(*(uint32 *) PqGSSRecvBuffer); - if (input.length > PQ_GSS_RECV_BUFFER_SIZE - sizeof(uint32)) + if (input.length > PQ_GSS_AUTH_BUFFER_SIZE - sizeof(uint32)) { printfPQExpBuffer(&conn->errorMessage, libpq_gettext("oversize GSSAPI packet sent by the server (%zu > %zu)\n"), (size_t) input.length, - PQ_GSS_RECV_BUFFER_SIZE - sizeof(uint32)); + PQ_GSS_AUTH_BUFFER_SIZE - sizeof(uint32)); return PGRES_POLLING_FAILED; } @@ -655,12 +666,34 @@ pqsecure_open_gss(PGconn *conn) conn->gcred = GSS_C_NO_CREDENTIAL; gss_release_buffer(&minor, &output); + /* + * Release the large authentication buffers and allocate the ones we + * want for normal operation. (This maneuver is safe only because + * pqDropConnection will drop the buffers; otherwise, during a + * reconnection we'd be at risk of using undersized buffers during + * negotiation.) + */ + free(PqGSSSendBuffer); + free(PqGSSRecvBuffer); + free(PqGSSResultBuffer); + PqGSSSendBuffer = malloc(PQ_GSS_MAX_PACKET_SIZE); + PqGSSRecvBuffer = malloc(PQ_GSS_MAX_PACKET_SIZE); + PqGSSResultBuffer = malloc(PQ_GSS_MAX_PACKET_SIZE); + if (!PqGSSSendBuffer || !PqGSSRecvBuffer || !PqGSSResultBuffer) + { + printfPQExpBuffer(&conn->errorMessage, + libpq_gettext("out of memory\n")); + return PGRES_POLLING_FAILED; + } + PqGSSSendLength = PqGSSSendNext = PqGSSSendConsumed = 0; + PqGSSRecvLength = PqGSSResultLength = PqGSSResultNext = 0; + /* * Determine the max packet size which will fit in our buffer, after * accounting for the length. pg_GSS_write will need this. */ major = gss_wrap_size_limit(&minor, conn->gctx, 1, GSS_C_QOP_DEFAULT, - PQ_GSS_SEND_BUFFER_SIZE - sizeof(uint32), + PQ_GSS_MAX_PACKET_SIZE - sizeof(uint32), &PqGSSMaxPktSize); if (GSS_ERROR(major)) @@ -674,10 +707,12 @@ pqsecure_open_gss(PGconn *conn) } /* Must have output.length > 0 */ - if (output.length > PQ_GSS_SEND_BUFFER_SIZE - sizeof(uint32)) + if (output.length > PQ_GSS_AUTH_BUFFER_SIZE - sizeof(uint32)) { - pg_GSS_error(libpq_gettext("GSSAPI context establishment error"), - conn, major, minor); + printfPQExpBuffer(&conn->errorMessage, + libpq_gettext("client tried to send oversize GSSAPI packet (%zu > %zu)\n"), + (size_t) output.length, + PQ_GSS_AUTH_BUFFER_SIZE - sizeof(uint32)); gss_release_buffer(&minor, &output); return PGRES_POLLING_FAILED; }
On Thu, May 22, 2025 at 05:04:32PM +0000, Chris Gooch wrote: > It now makes sense to me. I believe the KDC will not allow tokens > larger than 65535 bytes, so feel it is safe from a GSS perspective. The KDC protocol over TCP uses 32-bit unsigned PDU lengths in network byte order, of which the high bit is reserved and must be zero. ASN.1 supports much larger lengths still. The protocol easily supports very large tickets, therefore very large initial security context tokens. The architecture of having the user's SIDs be included in the user's service tickets was very useful as an optimization for a long time, but as the number of SIDs increases this optimization becomes more of an albatross. Nico --
Thanks Tom. More than happy to give these patches a test as I can easily reproduce the bug.
Are there specific test cases you would like me to try?
Thanks,
Chris
Chris
From: Tom Lane <tgl@sss.pgh.pa.us>
Sent: Saturday, May 24, 2025 8:38 pm
To: Chris Gooch <cgooch@bamfunds.com>
Cc: Jacob Champion <jacob.champion@enterprisedb.com>; pgsql-bugs@lists.postgresql.org <pgsql-bugs@lists.postgresql.org>
Subject: Re: [EXT] Re: GSS Auth issue when user member of lots of AD groups
Sent: Saturday, May 24, 2025 8:38 pm
To: Chris Gooch <cgooch@bamfunds.com>
Cc: Jacob Champion <jacob.champion@enterprisedb.com>; pgsql-bugs@lists.postgresql.org <pgsql-bugs@lists.postgresql.org>
Subject: Re: [EXT] Re: GSS Auth issue when user member of lots of AD groups
I wrote:
> OK, here's a set of draft patches for that.
Sigh, it'd help if I actually attached the patches ...
regards, tom lane
> OK, here's a set of draft patches for that.
Sigh, it'd help if I actually attached the patches ...
regards, tom lane
This email and any attachments should not be construed as an offer or recommendation to sell or buy or a solicitation of an offer to sell or buy any specific security, fund or instrument or to participate in any particular investment strategy. The information contained herein is given as of a certain date and does not purport to give information as of any other date. Although the information presented herein has been obtained from sources we believe to be reliable, no representation or warranty, expressed or implied, is made as to the accuracy or completeness of that information. Past performance is not indicative of future results.
CONFIDENTIALITY NOTICE: This message and any attachment are confidential. If you are not the intended recipient, please telephone or email the sender and delete this message and any attachment from your system. If you are not the intended recipient you must not copy this message or attachment or disclose the contents to any other persons.
Balyasny Asset Management (UK) LLP is authorised and regulated by the Financial Conduct Authority in the UK. Balyasny Asset Management LP is registered as an Investment Advisor with the Securities and Exchange Commission in the USA.
BAM prohibits all personnel from having any business related communications over text message or other unapproved communication applications. Unless pre-approved, BAM employees are only permitted to communicate over email, Bloomberg and BAM telephone lines.
CONFIDENTIALITY NOTICE: This message and any attachment are confidential. If you are not the intended recipient, please telephone or email the sender and delete this message and any attachment from your system. If you are not the intended recipient you must not copy this message or attachment or disclose the contents to any other persons.
Balyasny Asset Management (UK) LLP is authorised and regulated by the Financial Conduct Authority in the UK. Balyasny Asset Management LP is registered as an Investment Advisor with the Securities and Exchange Commission in the USA.
BAM prohibits all personnel from having any business related communications over text message or other unapproved communication applications. Unless pre-approved, BAM employees are only permitted to communicate over email, Bloomberg and BAM telephone lines.
Chris Gooch <cgooch@bamfunds.com> writes: > Thanks Tom. More than happy to give these patches a test as I can easily reproduce the bug. Great! > Are there specific test cases you would like me to try? Obviously, check if it works for the user with lots of AD groups. But beyond that, just check it in everyday use. All I've done is to run our "kerberos" regression test, and I don't have a huge amount of faith in the coverage of that. regards, tom lane
I wrote: > Chris Gooch <cgooch@bamfunds.com> writes: >> Are there specific test cases you would like me to try? > Obviously, check if it works for the user with lots of AD groups. > But beyond that, just check it in everyday use. Oh --- one specific scenario that would be good to check is to verify that patched libpq can still use GSS with unpatched server and vice versa, so long as the ticket doesn't exceed 16K. regards, tom lane
Thanks again for sharing the patches Tom. I have just tested the following combinations on v17 and all working as expected, both with tickets less thank 16k and greaterthan 16k. 1. Patched Client to Unpatched Server 2. Unpatched Client to Patched Server 3. Patched Client to Patched Server Thanks, Chris -----Original Message----- From: Tom Lane <tgl@sss.pgh.pa.us> Sent: Sunday, May 25, 2025 4:51 PM To: Chris Gooch <cgooch@bamfunds.com> Cc: Jacob Champion <jacob.champion@enterprisedb.com>; pgsql-bugs@lists.postgresql.org Subject: Re: [EXT] Re: GSS Auth issue when user member of lots of AD groups I wrote: > Chris Gooch <cgooch@bamfunds.com> writes: >> Are there specific test cases you would like me to try? > Obviously, check if it works for the user with lots of AD groups. > But beyond that, just check it in everyday use. Oh --- one specific scenario that would be good to check is to verify that patched libpq can still use GSS with unpatchedserver and vice versa, so long as the ticket doesn't exceed 16K. regards, tom lane This email and any attachments should not be construed as an offer or recommendation to sell or buy or a solicitation ofan offer to sell or buy any specific security, fund or instrument or to participate in any particular investment strategy.The information contained herein is given as of a certain date and does not purport to give information as of anyother date. Although the information presented herein has been obtained from sources we believe to be reliable, no representationor warranty, expressed or implied, is made as to the accuracy or completeness of that information. Past performanceis not indicative of future results. CONFIDENTIALITY NOTICE: This message and any attachment are confidential. If you are not the intended recipient, please telephoneor email the sender and delete this message and any attachment from your system. If you are not the intended recipientyou must not copy this message or attachment or disclose the contents to any other persons. Balyasny Asset Management (UK) LLP is authorised and regulated by the Financial Conduct Authority in the UK. Balyasny AssetManagement LP is registered as an Investment Advisor with the Securities and Exchange Commission in the USA. BAM prohibits all personnel from having any business related communications over text message or other unapproved communicationapplications. Unless pre-approved, BAM employees are only permitted to communicate over email, Bloomberg andBAM telephone lines.
On Sat, May 24, 2025 at 12:37 PM Tom Lane <tgl@sss.pgh.pa.us> wrote: > OK, here's a set of draft patches for that. I haven't reviewed the code in detail yet, but here are some thoughts on your notes: > * Is 128kB unreasonably large? I think we may want some daylight > above 64kB, but I'm not sure how much. Having a different token limit between gssenc and non-gssenc users doesn't seem right to me; if you somehow start relying on the much larger tokens with gssenc, and later want to switch to TLS, you'd suddenly be out of luck. (A larger token does apparently help with unconstrained delegation. But that page I shared from Microsoft upthread is saying that the deprecation of unconstrained delegation, plus SID compression, means that 48k should be good enough for anyone. Whether or not that's true in practice, I don't know, and I think 64k should definitely be our minimum.) > * I concluded that the error report that's being given for the case > is just flat-out bogus. +1 > * It seems pretty silly to have separate symbols for > PQ_GSS_SEND_BUFFER_SIZE and PQ_GSS_RECV_BUFFER_SIZE > when we're requiring those to be the same, so I merged > them into one symbol PQ_GSS_MAX_PACKET_SIZE. That seems fine. > * The backend's secure_open_gssapi allowed received initialization packets > to be up to buffer-size long, whereas libpq will choke sending them > if they're more than buffer-size minus sizeof(uint32). This isn't > actually a bug, since the buffer management is handled in such a way > that it's safe, but it seems very inconsistent. I changed the limit > to subtract off sizeof(uint32) in all cases, which incidentally > allowed removing one variant of the translatable message string. That discrepancy is confusing to me. Is there a way to standardize both sides in the other direction, so that they actually handle tokens up to the "max size"? --Jacob
Jacob Champion <jacob.champion@enterprisedb.com> writes: > On Sat, May 24, 2025 at 12:37 PM Tom Lane <tgl@sss.pgh.pa.us> wrote: >> * The backend's secure_open_gssapi allowed received initialization packets >> to be up to buffer-size long, whereas libpq will choke sending them >> if they're more than buffer-size minus sizeof(uint32). This isn't >> actually a bug, since the buffer management is handled in such a way >> that it's safe, but it seems very inconsistent. I changed the limit >> to subtract off sizeof(uint32) in all cases, which incidentally >> allowed removing one variant of the translatable message string. > That discrepancy is confusing to me. Is there a way to standardize > both sides in the other direction, so that they actually handle tokens > up to the "max size"? I don't think so, because that would create exactly the cross-version discrepancy we need to avoid. (That is, if sender thinks it can do 16384 when receiver's limit is 16384-4, kaboom.) The patch proposes to allow slop in this during the auth phase when the packet size is really being determined by the underlying GSSAPI library anyway. But once we're past that and our own code is slicing up the data stream into packets, I think the max packet size is indeed an inalterable part of the protocol. Could we address your confusion by improving the comment about the packet-size #define to point out that it includes the header word? regards, tom lane
On Tue, May 27, 2025 at 3:15 PM Tom Lane <tgl@sss.pgh.pa.us> wrote: > I don't think so, because that would create exactly the cross-version > discrepancy we need to avoid. (That is, if sender thinks it can do > 16384 when receiver's limit is 16384-4, kaboom.) The patch proposes > to allow slop in this during the auth phase when the packet size is > really being determined by the underlying GSSAPI library anyway. > But once we're past that and our own code is slicing up the data > stream into packets, I think the max packet size is indeed an > inalterable part of the protocol. Oh, I see. Yeah, that's unfortunate but makes sense. > Could we address your confusion by improving the comment about the > packet-size #define to point out that it includes the header word? Yes, I think so. Thanks! --Jacob
Would this patch be targeting next release cycle in August? Thanks, Chris -----Original Message----- From: Jacob Champion <jacob.champion@enterprisedb.com> Sent: Tuesday, May 27, 2025 11:25 PM To: Tom Lane <tgl@sss.pgh.pa.us> Cc: Chris Gooch <cgooch@bamfunds.com>; pgsql-bugs@lists.postgresql.org Subject: Re: [EXT] Re: GSS Auth issue when user member of lots of AD groups On Tue, May 27, 2025 at 3:15 PM Tom Lane <tgl@sss.pgh.pa.us> wrote: > I don't think so, because that would create exactly the cross-version > discrepancy we need to avoid. (That is, if sender thinks it can do > 16384 when receiver's limit is 16384-4, kaboom.) The patch proposes > to allow slop in this during the auth phase when the packet size is > really being determined by the underlying GSSAPI library anyway. > But once we're past that and our own code is slicing up the data > stream into packets, I think the max packet size is indeed an > inalterable part of the protocol. Oh, I see. Yeah, that's unfortunate but makes sense. > Could we address your confusion by improving the comment about the > packet-size #define to point out that it includes the header word? Yes, I think so. Thanks! --Jacob This email and any attachments should not be construed as an offer or recommendation to sell or buy or a solicitation ofan offer to sell or buy any specific security, fund or instrument or to participate in any particular investment strategy.The information contained herein is given as of a certain date and does not purport to give information as of anyother date. Although the information presented herein has been obtained from sources we believe to be reliable, no representationor warranty, expressed or implied, is made as to the accuracy or completeness of that information. Past performanceis not indicative of future results. CONFIDENTIALITY NOTICE: This message and any attachment are confidential. If you are not the intended recipient, please telephoneor email the sender and delete this message and any attachment from your system. If you are not the intended recipientyou must not copy this message or attachment or disclose the contents to any other persons. Balyasny Asset Management (UK) LLP is authorised and regulated by the Financial Conduct Authority in the UK. Balyasny AssetManagement LP is registered as an Investment Advisor with the Securities and Exchange Commission in the USA. BAM prohibits all personnel from having any business related communications over text message or other unapproved communicationapplications. Unless pre-approved, BAM employees are only permitted to communicate over email, Bloomberg andBAM telephone lines.
Chris Gooch <cgooch@bamfunds.com> writes: > Would this patch be targeting next release cycle in August? Yeah, I'd expect that we'll get it committed in a day or two and then it will be in the next quarterly releases. regards, tom lane
Jacob Champion <jacob.champion@enterprisedb.com> writes: > On Tue, May 27, 2025 at 3:15 PM Tom Lane <tgl@sss.pgh.pa.us> wrote: >> Could we address your confusion by improving the comment about the >> packet-size #define to point out that it includes the header word? > Yes, I think so. Here's a delta patch responding to your review comments so far: knock the max auth-packet size down to 64k, and annotate the size macros as including the header word. If you're planning to spend more time looking at this, please let me know, otherwise I'll go ahead and push the fixes. regards, tom lane diff -pud be-secure-gssapi.c~ be-secure-gssapi.c --- be-secure-gssapi.c~ 2025-05-29 13:56:23.359900699 -0400 +++ be-secure-gssapi.c 2025-05-29 13:59:54.931254443 -0400 @@ -49,16 +49,15 @@ * Therefore, this #define is effectively part of the protocol * spec and can't ever be changed. */ -#define PQ_GSS_MAX_PACKET_SIZE 16384 +#define PQ_GSS_MAX_PACKET_SIZE 16384 /* includes uint32 header word */ /* * However, during the authentication exchange we must cope with whatever * message size the GSSAPI library wants to send (because our protocol * doesn't support splitting those messages). Depending on configuration - * those messages might be as much as 64kB. To provide some safety margin - * we use 128kB buffers during transport negotiation. + * those messages might be as much as 64kB. */ -#define PQ_GSS_AUTH_BUFFER_SIZE 131072 +#define PQ_GSS_AUTH_BUFFER_SIZE 65536 /* includes uint32 header word */ /* * Since we manage at most one GSS-encrypted connection per backend, diff -pud fe-secure-gssapi.c~ fe-secure-gssapi.c --- fe-secure-gssapi.c~ 2025-05-29 13:56:23.359900699 -0400 +++ fe-secure-gssapi.c 2025-05-29 13:59:59.988262778 -0400 @@ -50,16 +50,15 @@ * Therefore, this #define is effectively part of the protocol * spec and can't ever be changed. */ -#define PQ_GSS_MAX_PACKET_SIZE 16384 +#define PQ_GSS_MAX_PACKET_SIZE 16384 /* includes uint32 header word */ /* * However, during the authentication exchange we must cope with whatever * message size the GSSAPI library wants to send (because our protocol * doesn't support splitting those messages). Depending on configuration - * those messages might be as much as 64kB. To provide some safety margin - * we use 128kB buffers during transport negotiation. + * those messages might be as much as 64kB. */ -#define PQ_GSS_AUTH_BUFFER_SIZE 131072 +#define PQ_GSS_AUTH_BUFFER_SIZE 65536 /* includes uint32 header word */ /* * We need these state variables per-connection. To allow the functions
On Thu, May 29, 2025 at 11:09 AM Tom Lane <tgl@sss.pgh.pa.us> wrote: > Here's a delta patch responding to your review comments so far: > knock the max auth-packet size down to 64k, and annotate the > size macros as including the header word. Thanks! > If you're planning to spend more time looking at this, please > let me know, otherwise I'll go ahead and push the fixes. I plan to get a full test+review back to you by end-of-day. (I don't see anything obviously scary yet, so if I miss my self-imposed deadline, no need to wait for me.) --Jacob
Jacob Champion <jacob.champion@enterprisedb.com> writes: > On Thu, May 29, 2025 at 11:09 AM Tom Lane <tgl@sss.pgh.pa.us> wrote: >> If you're planning to spend more time looking at this, please >> let me know, otherwise I'll go ahead and push the fixes. > I plan to get a full test+review back to you by end-of-day. (I don't > see anything obviously scary yet, so if I miss my self-imposed > deadline, no need to wait for me.) Sure, no rush. I just thought I'd get this off my queue if you were done looking. regards, tom lane
On Thu, May 29, 2025 at 11:41 AM Tom Lane <tgl@sss.pgh.pa.us> wrote: > Jacob Champion <jacob.champion@enterprisedb.com> writes: > > I plan to get a full test+review back to you by end-of-day. (I don't > > see anything obviously scary yet, so if I miss my self-imposed > > deadline, no need to wait for me.) > > Sure, no rush. I just thought I'd get this off my queue if > you were done looking. Okay, on closer review this LGTM. I was trying to get src/test/kerberos to shove a bunch of authorization data into its tickets, but I haven't figured out how to get krb5kdc to do that yet, so Chris's tests are the best we have at the moment. Eventually I'll get around to reading the ASN.1 so that pg-pytest can test this case, but that's not a job for today. Chris, I'm curious: what's the failure look like for the "1. Patched Client to Unpatched Server" case when the ticket is bigger than 16k? Thanks! --Jacob
Hi Jacob, In that scenario the client did not get any GSSAPI specific errors and drops to prompt for password. The server however hadthis in the logs "oversize GSSAPI packet sent by the client (20131 > 16384)" Thanks, Chris -----Original Message----- From: Jacob Champion <jacob.champion@enterprisedb.com> Sent: Friday, May 30, 2025 12:17 AM To: Tom Lane <tgl@sss.pgh.pa.us>; Chris Gooch <cgooch@bamfunds.com> Cc: pgsql-bugs@lists.postgresql.org Subject: Re: [EXT] Re: GSS Auth issue when user member of lots of AD groups On Thu, May 29, 2025 at 11:41 AM Tom Lane <tgl@sss.pgh.pa.us> wrote: > Jacob Champion <jacob.champion@enterprisedb.com> writes: > > I plan to get a full test+review back to you by end-of-day. (I don't > > see anything obviously scary yet, so if I miss my self-imposed > > deadline, no need to wait for me.) > > Sure, no rush. I just thought I'd get this off my queue if you were > done looking. Okay, on closer review this LGTM. I was trying to get src/test/kerberos to shove a bunch of authorization data into its tickets, but I haven't figured outhow to get krb5kdc to do that yet, so Chris's tests are the best we have at the moment. Eventually I'll get around toreading the ASN.1 so that pg-pytest can test this case, but that's not a job for today. Chris, I'm curious: what's thefailure look like for the "1. Patched Client to Unpatched Server" case when the ticket is bigger than 16k? Thanks! --Jacob This email and any attachments should not be construed as an offer or recommendation to sell or buy or a solicitation ofan offer to sell or buy any specific security, fund or instrument or to participate in any particular investment strategy.The information contained herein is given as of a certain date and does not purport to give information as of anyother date. Although the information presented herein has been obtained from sources we believe to be reliable, no representationor warranty, expressed or implied, is made as to the accuracy or completeness of that information. Past performanceis not indicative of future results. CONFIDENTIALITY NOTICE: This message and any attachment are confidential. If you are not the intended recipient, please telephoneor email the sender and delete this message and any attachment from your system. If you are not the intended recipientyou must not copy this message or attachment or disclose the contents to any other persons. Balyasny Asset Management (UK) LLP is authorised and regulated by the Financial Conduct Authority in the UK. Balyasny AssetManagement LP is registered as an Investment Advisor with the Securities and Exchange Commission in the USA. BAM prohibits all personnel from having any business related communications over text message or other unapproved communicationapplications. Unless pre-approved, BAM employees are only permitted to communicate over email, Bloomberg andBAM telephone lines.
Chris Gooch <cgooch@bamfunds.com> writes: > In that scenario the client did not get any GSSAPI specific errors and drops to prompt for password. The server howeverhad this in the logs "oversize GSSAPI packet sent by the client (20131 > 16384)" Yeah, that's expected. By default, a GSSAPI-enabled libpq will try to open a GSSAPI connection first, but silently fall back to not-GSSAPI if the server rejects it --- there's not any close inquiry into why the server rejected it. Jacob Champion <jacob.champion@enterprisedb.com> writes: > Okay, on closer review this LGTM. Pushed, thanks for reviewing. > I was trying to get src/test/kerberos to shove a bunch of > authorization data into its tickets, but I haven't figured out how > to get krb5kdc to do that yet, so Chris's tests are the best we have > at the moment. Eventually I'll get around to reading the ASN.1 so > that pg-pytest can test this case, but that's not a job for today. Sounds reasonable. I think Chris' testing is good enough for now. The one thing I was slightly concerned about was whether any data could remain in the buffers at the instant we downsize them, but that seems improbable (and it wouldn't depend on the ticket size anyway, I should think). regards, tom lane