URL: https://github.com/SSSD/sssd/pull/5572 Author: yrro Title: #5572: Handle large service tickets in SSS_GSSAPI_SEC_CTX packets Action: synchronized
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/5572/head:pr5572 git checkout pr5572
From 8a68966e4927c90512e3071478f239707b22b8a4 Mon Sep 17 00:00:00 2001 From: Sam Morris <s...@robots.org.uk> Date: Tue, 6 Apr 2021 18:42:19 +0100 Subject: [PATCH 1/6] responder/common/responder_packet: handle large service tickets Resolves: https://github.com/SSSD/sssd/issues/5568 --- src/responder/common/responder_packet.c | 11 +++++++++++ src/responder/common/responder_packet.h | 1 + 2 files changed, 12 insertions(+) diff --git a/src/responder/common/responder_packet.c b/src/responder/common/responder_packet.c index f56d922760..d091332b0c 100644 --- a/src/responder/common/responder_packet.c +++ b/src/responder/common/responder_packet.c @@ -229,6 +229,17 @@ int sss_packet_recv(struct sss_packet *packet, int fd) if (ret != EOK) { return ret; } + /* Kerberos tickets can get pretty big; since Windows Server 2012, the + * limit is 48 KiB! + */ + } else if ((sss_packet_get_cmd(packet) == SSS_GSSAPI_SEC_CTX) + && packet->memsize < SSS_GSSAPI_PACKET_MAX_RECV_SIZE + && new_len < SSS_GSSAPI_PACKET_MAX_RECV_SIZE) { + sss_packet_set_len(packet, 0); + ret = sss_packet_grow(packet, new_len); + if (ret != EOK) { + return ret; + } } else { return EINVAL; } diff --git a/src/responder/common/responder_packet.h b/src/responder/common/responder_packet.h index 509a22a9a9..70bf1e8d34 100644 --- a/src/responder/common/responder_packet.h +++ b/src/responder/common/responder_packet.h @@ -26,6 +26,7 @@ #define SSS_PACKET_MAX_RECV_SIZE 1024 #define SSS_CERT_PACKET_MAX_RECV_SIZE ( 10 * SSS_PACKET_MAX_RECV_SIZE ) +#define SSS_GSSAPI_PACKET_MAX_RECV_SIZE ( SSS_PACKET_MAX_RECV_SIZE + 48 * 1024 ) struct sss_packet; From 7a712666512bc7f1d2fec4e3bb5080dac5e61cd3 Mon Sep 17 00:00:00 2001 From: Sam Morris <s...@robots.org.uk> Date: Wed, 7 Apr 2021 14:21:34 +0100 Subject: [PATCH 2/6] responder/common/responder_packet: reduce duplication of code that handles larger-than-normal packets --- src/responder/common/responder_packet.c | 40 +++++++++++++------------ 1 file changed, 21 insertions(+), 19 deletions(-) diff --git a/src/responder/common/responder_packet.c b/src/responder/common/responder_packet.c index d091332b0c..523c9ddd4c 100644 --- a/src/responder/common/responder_packet.c +++ b/src/responder/common/responder_packet.c @@ -216,25 +216,27 @@ int sss_packet_recv(struct sss_packet *packet, int fd) new_len = sss_packet_get_len(packet); if (new_len > packet->memsize) { - /* Allow certificate based requests to use larger buffer but not - * larger than SSS_CERT_PACKET_MAX_RECV_SIZE. Due to the way - * sss_packet_grow() works the packet len must be set to '0' first and - * then grow to the expected size. */ - if ((sss_packet_get_cmd(packet) == SSS_NSS_GETNAMEBYCERT - || sss_packet_get_cmd(packet) == SSS_NSS_GETLISTBYCERT) - && packet->memsize < SSS_CERT_PACKET_MAX_RECV_SIZE - && new_len < SSS_CERT_PACKET_MAX_RECV_SIZE) { - sss_packet_set_len(packet, 0); - ret = sss_packet_grow(packet, new_len); - if (ret != EOK) { - return ret; - } - /* Kerberos tickets can get pretty big; since Windows Server 2012, the - * limit is 48 KiB! - */ - } else if ((sss_packet_get_cmd(packet) == SSS_GSSAPI_SEC_CTX) - && packet->memsize < SSS_GSSAPI_PACKET_MAX_RECV_SIZE - && new_len < SSS_GSSAPI_PACKET_MAX_RECV_SIZE) { + enum sss_cli_command cmd = sss_packet_get_cmd(packet); + size_t max_recv_size; + + /* Allow certain packet types to use a larger buffer. */ + switch (cmd) { + case SSS_NSS_GETNAMEBYCERT: + case SSS_NSS_GETLISTBYCERT: + max_recv_size = SSS_CERT_PACKET_MAX_RECV_SIZE; + break; + + case SSS_GSSAPI_SEC_CTX: + max_recv_size = SSS_GSSAPI_PACKET_MAX_RECV_SIZE; + break; + + default: + max_recv_size = 0; + } + + /* Due to the way sss_packet_grow() works, the packet len must be set + * to 0 first, and then grown to the expected size. */ + if (max_recv_size && packet->memsize < max_recv_size && new_len < max_recv_size) { sss_packet_set_len(packet, 0); ret = sss_packet_grow(packet, new_len); if (ret != EOK) { From c25ec73a19788ed48bca57d83e3c5c44cc8ef3b2 Mon Sep 17 00:00:00 2001 From: Sam Morris <s...@robots.org.uk> Date: Wed, 7 Apr 2021 14:22:25 +0100 Subject: [PATCH 3/6] responder/common/responder_packet: add debug logging to assist with errors caused by overlarge packets --- src/responder/common/responder_packet.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/responder/common/responder_packet.c b/src/responder/common/responder_packet.c index 523c9ddd4c..dd32a9ae8f 100644 --- a/src/responder/common/responder_packet.c +++ b/src/responder/common/responder_packet.c @@ -243,6 +243,9 @@ int sss_packet_recv(struct sss_packet *packet, int fd) return ret; } } else { + DEBUG(SSSDBG_OP_FAILURE, + "Refusing to read overlarge packet from fd %u (length %zu bytes, cmd %#04x)", + fd, new_len, cmd); return EINVAL; } } From 6f5abac975183d883328da3b537070b11d84e155 Mon Sep 17 00:00:00 2001 From: Sam Morris <s...@robots.org.uk> Date: Wed, 7 Apr 2021 14:23:03 +0100 Subject: [PATCH 4/6] responder/common/responder_packet: further increase packet size for SSS_GSSAPI_SEC_CTX Tokens can be 48 KiB in Windows Server 2012. Limiting to 128 KiB provides extra overhead should that increase in the future. --- src/responder/common/responder_packet.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/responder/common/responder_packet.h b/src/responder/common/responder_packet.h index 70bf1e8d34..fd991969b2 100644 --- a/src/responder/common/responder_packet.h +++ b/src/responder/common/responder_packet.h @@ -26,7 +26,7 @@ #define SSS_PACKET_MAX_RECV_SIZE 1024 #define SSS_CERT_PACKET_MAX_RECV_SIZE ( 10 * SSS_PACKET_MAX_RECV_SIZE ) -#define SSS_GSSAPI_PACKET_MAX_RECV_SIZE ( SSS_PACKET_MAX_RECV_SIZE + 48 * 1024 ) +#define SSS_GSSAPI_PACKET_MAX_RECV_SIZE ( 128 * 1024 ) struct sss_packet; From 281de7a2ad729362dff6f3eba165ad892abf9ec8 Mon Sep 17 00:00:00 2001 From: Sam Morris <s...@robots.org.uk> Date: Wed, 7 Apr 2021 19:59:45 +0100 Subject: [PATCH 5/6] responder/common/responder_packet: remove some unnecessary checks before growing packet --- src/responder/common/responder_packet.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/responder/common/responder_packet.c b/src/responder/common/responder_packet.c index dd32a9ae8f..bcc4b37c48 100644 --- a/src/responder/common/responder_packet.c +++ b/src/responder/common/responder_packet.c @@ -236,7 +236,7 @@ int sss_packet_recv(struct sss_packet *packet, int fd) /* Due to the way sss_packet_grow() works, the packet len must be set * to 0 first, and then grown to the expected size. */ - if (max_recv_size && packet->memsize < max_recv_size && new_len < max_recv_size) { + if (new_len < max_recv_size) { sss_packet_set_len(packet, 0); ret = sss_packet_grow(packet, new_len); if (ret != EOK) { From 0a5f8ad68fc0d51e8123146f470f12f06bd16468 Mon Sep 17 00:00:00 2001 From: Sam Morris <s...@robots.org.uk> Date: Thu, 8 Apr 2021 19:09:33 +0100 Subject: [PATCH 6/6] responder/common/responder_packet: allow packets of max size --- src/responder/common/responder_packet.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/responder/common/responder_packet.c b/src/responder/common/responder_packet.c index bcc4b37c48..aa2d09007a 100644 --- a/src/responder/common/responder_packet.c +++ b/src/responder/common/responder_packet.c @@ -236,7 +236,7 @@ int sss_packet_recv(struct sss_packet *packet, int fd) /* Due to the way sss_packet_grow() works, the packet len must be set * to 0 first, and then grown to the expected size. */ - if (new_len < max_recv_size) { + if (new_len <= max_recv_size) { sss_packet_set_len(packet, 0); ret = sss_packet_grow(packet, new_len); if (ret != EOK) {
_______________________________________________ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure