rpms/krb5
6
0
Fork 0
Code Issues Pull Requests Releases Activity

Fix libkrad client cleanup code

Browse Source 
Resolves: rhbz#2072059

Signed-off-by: Alexander Bokovoy <abokovoy@redhat.com>
This commit is contained in:
Alexander Bokovoy 2022-04-05 22:14:44 +03:00
parent 29a69aee06
commit fc958d4773
2 changed files with 216 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

209
krb5-krad-remote.patch Normal file
View File

@ -0,0 +1,209 @@
From ce160f8826bae223876a6527a731c36b6912db15 Mon Sep 17 00:00:00 2001
From: Greg Hudson <ghudson@mit.edu>
Date: Tue, 9 Nov 2021 13:00:43 -0500
Subject: [PATCH 1/2] Avoid use after free during libkrad cleanup
libkrad client requests contain a list of references to remotes, with
no back-references or reference counts. To prevent accesses to
dangling references during cleanup, cancel all requests on all remotes
before freeing any remotes.
Remove the code for aging out unused servers. This code was fairly
safe as all requests referencing a remote should have completed or
timed out during an hour of disuse, but in the current design we have
no way to guarantee or check that. The set of addresses we send
RADIUS requests to will generally be small, so aging out servers is
unnecessary.
ticket: 9035 (new)
---
src/lib/krad/client.c | 42 ++++++++++++++---------------------------
src/lib/krad/internal.h | 4 ++++
src/lib/krad/remote.c | 11 ++++++++---
3 files changed, 26 insertions(+), 31 deletions(-)
diff --git a/src/lib/krad/client.c b/src/lib/krad/client.c
index 6365dd1c6..810940afc 100644
--- a/src/lib/krad/client.c
+++ b/src/lib/krad/client.c
@@ -64,7 +64,6 @@ struct request_st {
struct server_st {
krad_remote *serv;
- time_t last;
K5_LIST_ENTRY(server_st) list;
};
@@ -81,15 +80,10 @@ get_server(krad_client *rc, const struct addrinfo *ai, const char *secret,
krad_remote **out)
{
krb5_error_code retval;
- time_t currtime;
server *srv;
- if (time(&currtime) == (time_t)-1)
- return errno;
-
K5_LIST_FOREACH(srv, &rc->servers, list) {
if (kr_remote_equals(srv->serv, ai, secret)) {
- srv->last = currtime;
*out = srv->serv;
return 0;
}
@@ -98,7 +92,6 @@ get_server(krad_client *rc, const struct addrinfo *ai, const char *secret,
srv = calloc(1, sizeof(server));
if (srv == NULL)
return ENOMEM;
- srv->last = currtime;
retval = kr_remote_new(rc->kctx, rc->vctx, ai, secret, &srv->serv);
if (retval != 0) {
@@ -173,28 +166,12 @@ request_new(krad_client *rc, krad_code code, const krad_attrset *attrs,
return 0;
}
-/* Close remotes that haven't been used in a while. */
-static void
-age(struct server_head *head, time_t currtime)
-{
- server *srv, *tmp;
-
- K5_LIST_FOREACH_SAFE(srv, head, list, tmp) {
- if (currtime == (time_t)-1 || currtime - srv->last > 60 * 60) {
- K5_LIST_REMOVE(srv, list);
- kr_remote_free(srv->serv);
- free(srv);
- }
- }
-}
-
/* Handle a response from a server (or related errors). */
static void
on_response(krb5_error_code retval, const krad_packet *reqp,
const krad_packet *rspp, void *data)
{
request *req = data;
- time_t currtime;
size_t i;
/* Do nothing if we are already completed. */
@@ -221,10 +198,6 @@ on_response(krb5_error_code retval, const krad_packet *reqp,
for (i = 0; req->remotes[i].remote != NULL; i++)
kr_remote_cancel(req->remotes[i].remote, req->remotes[i].packet);
- /* Age out servers that haven't been used in a while. */
- if (time(&currtime) != (time_t)-1)
- age(&req->rc->servers, currtime);
-
request_free(req);
}
@@ -247,10 +220,23 @@ krad_client_new(krb5_context kctx, verto_ctx *vctx, krad_client **out)
void
krad_client_free(krad_client *rc)
{
+ server *srv;
+
if (rc == NULL)
return;
- age(&rc->servers, -1);
+ /* Cancel all requests before freeing any remotes, since each request's
+ * callback data may contain references to multiple remotes. */
+ K5_LIST_FOREACH(srv, &rc->servers, list)
+ kr_remote_cancel_all(srv->serv);
+
+ while (!K5_LIST_EMPTY(&rc->servers)) {
+ srv = K5_LIST_FIRST(&rc->servers);
+ K5_LIST_REMOVE(srv, list);
+ kr_remote_free(srv->serv);
+ free(srv);
+ }
+
free(rc);
}
diff --git a/src/lib/krad/internal.h b/src/lib/krad/internal.h
index 0143d155a..7619563fc 100644
--- a/src/lib/krad/internal.h
+++ b/src/lib/krad/internal.h
@@ -109,6 +109,10 @@ kr_remote_send(krad_remote *rr, krad_code code, krad_attrset *attrs,
void
kr_remote_cancel(krad_remote *rr, const krad_packet *pkt);
+/* Cancel all requests awaiting responses. */
+void
+kr_remote_cancel_all(krad_remote *rr);
+
/* Determine if this remote object refers to the remote resource identified
* by the addrinfo struct and the secret. */
krb5_boolean
diff --git a/src/lib/krad/remote.c b/src/lib/krad/remote.c
index 7e491e994..06ae751bc 100644
--- a/src/lib/krad/remote.c
+++ b/src/lib/krad/remote.c
@@ -421,15 +421,20 @@ error:
return retval;
}
+void
+kr_remote_cancel_all(krad_remote *rr)
+{
+ while (!K5_TAILQ_EMPTY(&rr->list))
+ request_finish(K5_TAILQ_FIRST(&rr->list), ECANCELED, NULL);
+}
+
void
kr_remote_free(krad_remote *rr)
{
if (rr == NULL)
return;
- while (!K5_TAILQ_EMPTY(&rr->list))
- request_finish(K5_TAILQ_FIRST(&rr->list), ECANCELED, NULL);
-
+ kr_remote_cancel_all(rr);
free(rr->secret);
if (rr->info != NULL)
free(rr->info->ai_addr);
--
2.35.1
From e0084425df784952e76b3bcc8ae9d08300234733 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose@redhat.com>
Date: Mon, 8 Nov 2021 17:47:17 +0100
Subject: [PATCH 2/2] More python3 fixes for t_daemon.py
[ghudson@mit.edu: use a list comprehension instead of map()]
---
src/lib/krad/t_daemon.py | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/lib/krad/t_daemon.py b/src/lib/krad/t_daemon.py
index 7668cd7f8..4a3de079c 100755
--- a/src/lib/krad/t_daemon.py
+++ b/src/lib/krad/t_daemon.py
@@ -50,7 +50,7 @@ class TestServer(server.Server):
for key in pkt.keys():
if key == "User-Password":
- passwd = map(pkt.PwDecrypt, pkt[key])
+ passwd = [pkt.PwDecrypt(x) for x in pkt[key]]
reply = self.CreateReplyPacket(pkt)
if passwd == ['accept']:
@@ -61,8 +61,8 @@ class TestServer(server.Server):
srv = TestServer(addresses=["localhost"],
hosts={"127.0.0.1":
- server.RemoteHost("127.0.0.1", "foo", "localhost")},
- dict=dictionary.Dictionary(StringIO.StringIO(DICTIONARY)))
+ server.RemoteHost("127.0.0.1", b"foo", "localhost")},
+ dict=dictionary.Dictionary(StringIO(DICTIONARY)))
# Write a sentinel character to let the parent process know we're listening.
sys.stdout.write("~")
--
2.35.1

9
krb5.spec
View File

@ -42,7 +42,7 @@
Summary: The Kerberos network authentication system
Name: krb5
Version: 1.19.2
Release: %{?zdpd}8%{?dist}
Release: %{?zdpd}9%{?dist}
# rharwood has trust path to signing key and verifies on check-in
Source0: https://web.mit.edu/kerberos/dist/krb5/%{version}/krb5-%{version}%{?dashpre}.tar.gz
@ -95,7 +95,8 @@ Patch34: Use-OpenSSL-s-KBKDF-and-KRB5KDF-for-deriving-long-te.patch
Patch35: Handle-OpenSSL-3-s-providers.patch
Patch36: Remove-TCL-based-libkadm5-API-tests.patch
Patch37: Use-SHA256-instead-of-SHA1-for-PKINIT-CMS-digest.patch
Patch38: krb5-krad-larger-attrs.patch
Patch38: krb5-krad-remote.patch
Patch39: krb5-krad-larger-attrs.patch
License: MIT
URL: https://web.mit.edu/kerberos/www/
@ -645,6 +646,10 @@ exit 0
%{_libdir}/libkadm5srv_mit.so.*
%changelog
* Tue Apr 05 2022 Alexander Bokovoy <abokovoy@redhat.com> - 1.19.2-9
- Fix libkrad client cleanup
- Fixes rhbz#2072059
* Tue Apr 05 2022 Alexander Bokovoy <abokovoy@redhat.com> - 1.19.2-8
- Allow use of larger RADIUS attributes in krad library