From 33638de180a8157e369ad6c61f9e3406d9e85404 Mon Sep 17 00:00:00 2001
From: Stanislav Levin <slev@altlinux.org>
Date: Tue, 23 Jan 2024 19:12:53 +0300
Subject: [PATCH 1/3] ipapython: Clean up krb5_error
`krb5_error` has different definition in MIT krb.
https://web.mit.edu/kerberos/krb5-latest/doc/appdev/refs/types/krb5_error.html
> Error message structure.
>
> Declaration:
> typedef struct _krb5_error krb5_error
While `krb5_error_code`
https://web.mit.edu/kerberos/www/krb5-latest/doc/appdev/refs/types/krb5_error_code.html#c.krb5_error_code
> krb5_error_code
> Used to convey an operation status.
>
> The value 0 indicates success; any other values are com_err codes. Use krb5_get_error_message() to obtain a string describing the error.
>
> Declaration
> typedef krb5_int32 krb5_error_code
And this is what was actually used.
To prevent confusion of types `krb5_error` was replaced with
`krb5_error_code`.
Fixes: https://pagure.io/freeipa/issue/9519
Signed-off-by: Stanislav Levin <slev@altlinux.org>
Reviewed-By: Alexander Bokovoy <abokovoy@redhat.com>
---
ipapython/session_storage.py | 25 ++++++++++++-------------
1 file changed, 12 insertions(+), 13 deletions(-)
diff --git a/ipapython/session_storage.py b/ipapython/session_storage.py
index c43ef7d4e..371cf1524 100644
--- a/ipapython/session_storage.py
+++ b/ipapython/session_storage.py
@@ -111,7 +111,7 @@ class KRB5Error(Exception):
def krb5_errcheck(result, func, arguments):
- """Error checker for krb5_error return value"""
+ """Error checker for krb5_error_code return value"""
if result != 0:
raise KRB5Error(result, func.__name__, arguments)
@@ -119,14 +119,13 @@ def krb5_errcheck(result, func, arguments):
krb5_context = ctypes.POINTER(_krb5_context)
krb5_ccache = ctypes.POINTER(_krb5_ccache)
krb5_data_p = ctypes.POINTER(_krb5_data)
-krb5_error = ctypes.c_int32
krb5_creds = _krb5_creds
krb5_pointer = ctypes.c_void_p
krb5_cc_cursor = krb5_pointer
krb5_init_context = LIBKRB5.krb5_init_context
krb5_init_context.argtypes = (ctypes.POINTER(krb5_context), )
-krb5_init_context.restype = krb5_error
+krb5_init_context.restype = krb5_error_code
krb5_init_context.errcheck = krb5_errcheck
krb5_free_context = LIBKRB5.krb5_free_context
@@ -143,30 +142,30 @@ krb5_free_data_contents.restype = None
krb5_cc_default = LIBKRB5.krb5_cc_default
krb5_cc_default.argtypes = (krb5_context, ctypes.POINTER(krb5_ccache), )
-krb5_cc_default.restype = krb5_error
+krb5_cc_default.restype = krb5_error_code
krb5_cc_default.errcheck = krb5_errcheck
krb5_cc_close = LIBKRB5.krb5_cc_close
krb5_cc_close.argtypes = (krb5_context, krb5_ccache, )
-krb5_cc_close.restype = krb5_error
+krb5_cc_close.restype = krb5_error_code
krb5_cc_close.errcheck = krb5_errcheck
krb5_parse_name = LIBKRB5.krb5_parse_name
krb5_parse_name.argtypes = (krb5_context, ctypes.c_char_p,
ctypes.POINTER(krb5_principal), )
-krb5_parse_name.restype = krb5_error
+krb5_parse_name.restype = krb5_error_code
krb5_parse_name.errcheck = krb5_errcheck
krb5_cc_set_config = LIBKRB5.krb5_cc_set_config
krb5_cc_set_config.argtypes = (krb5_context, krb5_ccache, krb5_principal,
ctypes.c_char_p, krb5_data_p, )
-krb5_cc_set_config.restype = krb5_error
+krb5_cc_set_config.restype = krb5_error_code
krb5_cc_set_config.errcheck = krb5_errcheck
krb5_cc_get_principal = LIBKRB5.krb5_cc_get_principal
krb5_cc_get_principal.argtypes = (krb5_context, krb5_ccache,
ctypes.POINTER(krb5_principal), )
-krb5_cc_get_principal.restype = krb5_error
+krb5_cc_get_principal.restype = krb5_error_code
krb5_cc_get_principal.errcheck = krb5_errcheck
# krb5_build_principal is a variadic function but that can't be expressed
@@ -177,26 +176,26 @@ krb5_build_principal.argtypes = (krb5_context, ctypes.POINTER(krb5_principal),
ctypes.c_uint, ctypes.c_char_p,
ctypes.c_char_p, ctypes.c_char_p,
ctypes.c_char_p, ctypes.c_char_p, )
-krb5_build_principal.restype = krb5_error
+krb5_build_principal.restype = krb5_error_code
krb5_build_principal.errcheck = krb5_errcheck
krb5_cc_start_seq_get = LIBKRB5.krb5_cc_start_seq_get
krb5_cc_start_seq_get.argtypes = (krb5_context, krb5_ccache,
ctypes.POINTER(krb5_cc_cursor), )
-krb5_cc_start_seq_get.restype = krb5_error
+krb5_cc_start_seq_get.restype = krb5_error_code
krb5_cc_start_seq_get.errcheck = krb5_errcheck
krb5_cc_next_cred = LIBKRB5.krb5_cc_next_cred
krb5_cc_next_cred.argtypes = (krb5_context, krb5_ccache,
ctypes.POINTER(krb5_cc_cursor),
ctypes.POINTER(krb5_creds), )
-krb5_cc_next_cred.restype = krb5_error
+krb5_cc_next_cred.restype = krb5_error_code
krb5_cc_next_cred.errcheck = krb5_errcheck
krb5_cc_end_seq_get = LIBKRB5.krb5_cc_end_seq_get
krb5_cc_end_seq_get.argtypes = (krb5_context, krb5_ccache,
ctypes.POINTER(krb5_cc_cursor), )
-krb5_cc_end_seq_get.restype = krb5_error
+krb5_cc_end_seq_get.restype = krb5_error_code
krb5_cc_end_seq_get.errcheck = krb5_errcheck
krb5_free_cred_contents = LIBKRB5.krb5_free_cred_contents
@@ -212,7 +211,7 @@ krb5_principal_compare.restype = krb5_boolean
krb5_unparse_name = LIBKRB5.krb5_unparse_name
krb5_unparse_name.argtypes = (krb5_context, krb5_principal,
ctypes.POINTER(ctypes.c_char_p), )
-krb5_unparse_name.restype = krb5_error
+krb5_unparse_name.restype = krb5_error_code
krb5_unparse_name.errcheck = krb5_errcheck
krb5_free_unparsed_name = LIBKRB5.krb5_free_unparsed_name
--
2.43.0
From f8a616dc6196324145372713da772fe9b2352e53 Mon Sep 17 00:00:00 2001
From: Stanislav Levin <slev@altlinux.org>
Date: Tue, 23 Jan 2024 19:19:43 +0300
Subject: [PATCH 2/3] ipapython: Correct return type of krb5_free_cred_contents
According to https://web.mit.edu/kerberos/krb5-latest/doc/appdev/refs/api/krb5_free_cred_contents.html
> krb5_free_cred_contents - Free the contents of a krb5_creds structure.
>
> void krb5_free_cred_contents(krb5_context context, krb5_creds * val)
> param:
> [in] context - Library context
>
> [in] val - Credential structure to free contents of
>
> This function frees the contents of val , but not the structure itself.
https://github.com/krb5/krb5/blob/5b00197227231943bd2305328c8260dd0b0dbcf0/src/lib/krb5/krb/kfree.c#L166
This leads to undefined behavior and `krb5_free_cred_contents` can
raise KRB5Error (because of garbage data) while actually its foreign
function doesn't.
Fixes: https://pagure.io/freeipa/issue/9519
Signed-off-by: Stanislav Levin <slev@altlinux.org>
Reviewed-By: Alexander Bokovoy <abokovoy@redhat.com>
---
ipapython/session_storage.py | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/ipapython/session_storage.py b/ipapython/session_storage.py
index 371cf1524..dc36f5493 100644
--- a/ipapython/session_storage.py
+++ b/ipapython/session_storage.py
@@ -200,8 +200,7 @@ krb5_cc_end_seq_get.errcheck = krb5_errcheck
krb5_free_cred_contents = LIBKRB5.krb5_free_cred_contents
krb5_free_cred_contents.argtypes = (krb5_context, ctypes.POINTER(krb5_creds))
-krb5_free_cred_contents.restype = krb5_error
-krb5_free_cred_contents.errcheck = krb5_errcheck
+krb5_free_cred_contents.restype = None
krb5_principal_compare = LIBKRB5.krb5_principal_compare
krb5_principal_compare.argtypes = (krb5_context, krb5_principal,
--
2.43.0
From 59b8a9fb7169561c7ba9168fe84f47ae94e5ce23 Mon Sep 17 00:00:00 2001
From: Stanislav Levin <slev@altlinux.org>
Date: Tue, 23 Jan 2024 19:52:34 +0300
Subject: [PATCH 3/3] ipapython: Propagate KRB5Error exceptions on iterating
ccache
`ipapython.session_storage.get_data` iterates over
credentials in a credential cache till `krb5_cc_next_cred` returns
an error. This function doesn't expect any error on calling
other kerberos foreign functions during iteration. But that can
actually happen and KRB5Error exceptions stop an iteration while
they should be propagated.
With this change iteration will exactly stop on `krb5_cc_next_cred`
error as it was supposed to be.
Fixes: https://pagure.io/freeipa/issue/9519
Signed-off-by: Stanislav Levin <slev@altlinux.org>
Reviewed-By: Alexander Bokovoy <abokovoy@redhat.com>
---
ipapython/session_storage.py | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/ipapython/session_storage.py b/ipapython/session_storage.py
index dc36f5493..e890dc9b1 100644
--- a/ipapython/session_storage.py
+++ b/ipapython/session_storage.py
@@ -312,8 +312,12 @@ def get_data(princ_name, key):
checkcreds = krb5_creds()
# the next function will throw an error and break out of the
# while loop when we try to access past the last cred
- krb5_cc_next_cred(context, ccache, ctypes.byref(cursor),
- ctypes.byref(checkcreds))
+ try:
+ krb5_cc_next_cred(context, ccache, ctypes.byref(cursor),
+ ctypes.byref(checkcreds))
+ except KRB5Error:
+ break
+
if (krb5_principal_compare(context, principal,
checkcreds.client) == 1 and
krb5_principal_compare(context, srv_princ,
@@ -328,8 +332,6 @@ def get_data(princ_name, key):
else:
krb5_free_cred_contents(context,
ctypes.byref(checkcreds))
- except KRB5Error:
- pass
finally:
krb5_cc_end_seq_get(context, ccache, ctypes.byref(cursor))
--
2.43.0