From cfb32a638e35a8769719d0ebce5545fb9a015ea2 Mon Sep 17 00:00:00 2001 From: Joseph Sutton Date: Sat, 22 Oct 2022 10:11:53 +1300 Subject: [PATCH] Introduce macro for common plugin structure elements Heimdal's HDB plugin interface, and hence Samba's KDC that depends upon it, doesn't work on 32-bit builds due to structure fields being arranged in the wrong order. This problem presents itself in the form of segmentation faults on 32-bit systems, but goes unnoticed on 64-bit builds thanks to extra structure padding absorbing the errant fields. This commit reorders the HDB plugin structure fields to prevent crashes and introduces a common macro to ensure every plugin presents a consistent interface. Samba BUG: https://bugzilla.samba.org/show_bug.cgi?id=15110 Signed-off-by: Joseph Sutton --- kdc/csr_authorizer_plugin.h | 4 +-- kdc/gss_preauth_authorizer_plugin.h | 4 +-- kdc/kdc-plugin.h | 4 +-- kdc/token_validator_plugin.h | 4 +-- lib/base/common_plugin.h | 6 ++--- lib/base/heimbase-svc.h | 5 ++++ lib/base/plugin.c | 2 +- lib/hdb/hdb-ldap.c | 4 +-- lib/hdb/hdb.c | 40 ++++++++++++++--------------- lib/hdb/hdb.h | 4 +-- lib/hdb/test_namespace.c | 8 +++--- lib/kadm5/kadm5-hook.h | 6 ++--- lib/krb5/an2ln_plugin.h | 6 ++--- lib/krb5/db_plugin.h | 6 ++--- lib/krb5/kuserok_plugin.h | 6 ++--- lib/krb5/locate_plugin.h | 6 ++--- lib/krb5/send_to_kdc_plugin.h | 5 ++-- 17 files changed, 57 insertions(+), 63 deletions(-) diff --git a/kdc/csr_authorizer_plugin.h b/kdc/csr_authorizer_plugin.h index 45f42014b..022fedac0 100644 --- a/kdc/csr_authorizer_plugin.h +++ b/kdc/csr_authorizer_plugin.h @@ -62,9 +62,7 @@ * @ingroup krb5_support */ typedef struct krb5plugin_csr_authorizer_ftable_desc { - int minor_version; - krb5_error_code (KRB5_LIB_CALL *init)(krb5_context, void **); - void (KRB5_LIB_CALL *fini)(void *); + HEIM_PLUGIN_FTABLE_COMMON_ELEMENTS(krb5_context); krb5_error_code (KRB5_LIB_CALL *authorize)(void *, /*plug_ctx*/ krb5_context, /*context*/ const char *, /*app*/ diff --git a/kdc/gss_preauth_authorizer_plugin.h b/kdc/gss_preauth_authorizer_plugin.h index 293e59da4..1bc1c914c 100644 --- a/kdc/gss_preauth_authorizer_plugin.h +++ b/kdc/gss_preauth_authorizer_plugin.h @@ -60,9 +60,7 @@ */ typedef struct krb5plugin_gss_preauth_authorizer_ftable_desc { - int minor_version; - krb5_error_code (KRB5_LIB_CALL *init)(krb5_context, void **); - void (KRB5_LIB_CALL *fini)(void *); + HEIM_PLUGIN_FTABLE_COMMON_ELEMENTS(krb5_context); krb5_error_code (KRB5_LIB_CALL *authorize)(void *, /*plug_ctx*/ astgs_request_t, /*r*/ gss_const_name_t, /*initiator_name*/ diff --git a/kdc/kdc-plugin.h b/kdc/kdc-plugin.h index 9fc5946df..05286257b 100644 --- a/kdc/kdc-plugin.h +++ b/kdc/kdc-plugin.h @@ -120,9 +120,7 @@ typedef krb5_error_code #define KRB5_PLUGIN_KDC_VERSION_10 10 typedef struct krb5plugin_kdc_ftable { - int minor_version; - krb5_error_code (KRB5_CALLCONV *init)(krb5_context, void **); - void (KRB5_CALLCONV *fini)(void *); + HEIM_PLUGIN_FTABLE_COMMON_ELEMENTS(krb5_context); krb5plugin_kdc_pac_generate pac_generate; krb5plugin_kdc_pac_verify pac_verify; krb5plugin_kdc_client_access client_access; diff --git a/kdc/token_validator_plugin.h b/kdc/token_validator_plugin.h index 73b05e5d0..8ff00149a 100644 --- a/kdc/token_validator_plugin.h +++ b/kdc/token_validator_plugin.h @@ -67,9 +67,7 @@ * @ingroup krb5_support */ typedef struct krb5plugin_token_validator_ftable_desc { - int minor_version; - krb5_error_code (KRB5_LIB_CALL *init)(krb5_context, void **); - void (KRB5_LIB_CALL *fini)(void *); + HEIM_PLUGIN_FTABLE_COMMON_ELEMENTS(krb5_context); krb5_error_code (KRB5_LIB_CALL *validate)(void *, /*plug_ctx*/ krb5_context, const char *, /*realm*/ diff --git a/lib/base/common_plugin.h b/lib/base/common_plugin.h index 82c589563..ece1d2817 100644 --- a/lib/base/common_plugin.h +++ b/lib/base/common_plugin.h @@ -36,6 +36,8 @@ #ifndef HEIMDAL_BASE_COMMON_PLUGIN_H #define HEIMDAL_BASE_COMMON_PLUGIN_H +#include + #ifdef _WIN32 # ifndef HEIM_CALLCONV # define HEIM_CALLCONV __stdcall @@ -69,9 +71,7 @@ typedef heim_get_instance_func_t krb5_get_instance_t; * All plugin function tables extend the following structure. */ struct heim_plugin_common_ftable_desc { - int version; - int (HEIM_LIB_CALL *init)(heim_pcontext, void **); - void (HEIM_LIB_CALL *fini)(void *); + HEIM_PLUGIN_FTABLE_COMMON_ELEMENTS(heim_pcontext); }; typedef struct heim_plugin_common_ftable_desc heim_plugin_common_ftable; typedef struct heim_plugin_common_ftable_desc *heim_plugin_common_ftable_p; diff --git a/lib/base/heimbase-svc.h b/lib/base/heimbase-svc.h index 083917fb8..0e7454c83 100644 --- a/lib/base/heimbase-svc.h +++ b/lib/base/heimbase-svc.h @@ -75,4 +75,9 @@ heim_dict_t attributes; \ int32_t error_code +#define HEIM_PLUGIN_FTABLE_COMMON_ELEMENTS(CONTEXT_TYPE) \ + int minor_version; \ + int (HEIM_LIB_CALL *init)(CONTEXT_TYPE, void **); \ + void (HEIM_LIB_CALL *fini)(void *) + #endif /* HEIMBASE_SVC_H */ diff --git a/lib/base/plugin.c b/lib/base/plugin.c index 631a3386c..355306dd9 100644 --- a/lib/base/plugin.c +++ b/lib/base/plugin.c @@ -653,7 +653,7 @@ reduce_by_version(heim_object_t value, void *ctx, int *stop) struct iter_ctx *s = ctx; struct heim_plugin *pl = value; - if (pl->ftable && pl->ftable->version >= s->caller->min_version) + if (pl->ftable && pl->ftable->minor_version >= s->caller->min_version) heim_array_append_value(s->result, pl); } diff --git a/lib/hdb/hdb-ldap.c b/lib/hdb/hdb-ldap.c index 4967b4ce0..5cd097f5b 100644 --- a/lib/hdb/hdb-ldap.c +++ b/lib/hdb/hdb-ldap.c @@ -2097,18 +2097,18 @@ fini(void *ctx) struct hdb_method hdb_ldap_interface = { HDB_INTERFACE_VERSION, - 0 /*is_file_based*/, 0 /*can_taste*/, init, fini, + 0 /*is_file_based*/, 0 /*can_taste*/, "ldap", hdb_ldap_create }; struct hdb_method hdb_ldapi_interface = { HDB_INTERFACE_VERSION, - 0 /*is_file_based*/, 0 /*can_taste*/, init, fini, + 0 /*is_file_based*/, 0 /*can_taste*/, "ldapi", hdb_ldapi_create }; diff --git a/lib/hdb/hdb.c b/lib/hdb/hdb.c index 56c403842..171ba9e3f 100644 --- a/lib/hdb/hdb.c +++ b/lib/hdb/hdb.c @@ -66,41 +66,41 @@ const int hdb_interface_version = HDB_INTERFACE_VERSION; static struct hdb_method methods[] = { /* "db:" should be db3 if we have db3, or db1 if we have db1 */ #if HAVE_DB3 - { HDB_INTERFACE_VERSION, 1 /*is_file_based*/, 1 /*can_taste*/, NULL, NULL, + { HDB_INTERFACE_VERSION, NULL, NULL, 1 /*is_file_based*/, 1 /*can_taste*/, "db:", hdb_db3_create}, #elif HAVE_DB1 - { HDB_INTERFACE_VERSION, 1, 1, NULL, NULL, "db:", hdb_db1_create}, + { HDB_INTERFACE_VERSION, NULL, NULL, 1, 1, "db:", hdb_db1_create}, #endif #if HAVE_DB1 - { HDB_INTERFACE_VERSION, 1, 1, NULL, NULL, "db1:", hdb_db1_create}, + { HDB_INTERFACE_VERSION, NULL, NULL, 1, 1, "db1:", hdb_db1_create}, #endif #if HAVE_DB3 - { HDB_INTERFACE_VERSION, 1, 1, NULL, NULL, "db3:", hdb_db3_create}, + { HDB_INTERFACE_VERSION, NULL, NULL, 1, 1, "db3:", hdb_db3_create}, #endif #if HAVE_DB1 - { HDB_INTERFACE_VERSION, 1, 1, NULL, NULL, "mit-db:", hdb_mitdb_create}, + { HDB_INTERFACE_VERSION, NULL, NULL, 1, 1, "mit-db:", hdb_mitdb_create}, #endif #if HAVE_LMDB - { HDB_INTERFACE_VERSION, 1, 1, NULL, NULL, "mdb:", hdb_mdb_create}, - { HDB_INTERFACE_VERSION, 1, 1, NULL, NULL, "lmdb:", hdb_mdb_create}, + { HDB_INTERFACE_VERSION, NULL, NULL, 1, 1, "mdb:", hdb_mdb_create}, + { HDB_INTERFACE_VERSION, NULL, NULL, 1, 1, "lmdb:", hdb_mdb_create}, #endif #if HAVE_NDBM - { HDB_INTERFACE_VERSION, 1, 0, NULL, NULL, "ndbm:", hdb_ndbm_create}, + { HDB_INTERFACE_VERSION, NULL, NULL, 1, 0, "ndbm:", hdb_ndbm_create}, #endif #ifdef HAVE_SQLITE3 - { HDB_INTERFACE_VERSION, 1, 1, NULL, NULL, "sqlite:", hdb_sqlite_create}, + { HDB_INTERFACE_VERSION, NULL, NULL, 1, 1, "sqlite:", hdb_sqlite_create}, #endif /* The keytab interface can't use its hdb_open() method to "taste" a DB */ - { HDB_INTERFACE_VERSION, 1, 0, NULL, NULL, "keytab:", hdb_keytab_create}, + { HDB_INTERFACE_VERSION, NULL, NULL, 1, 0, "keytab:", hdb_keytab_create}, /* The rest are not file-based */ #if defined(OPENLDAP) && !defined(OPENLDAP_MODULE) - { HDB_INTERFACE_VERSION, 0, 0, NULL, NULL, "ldap:", hdb_ldap_create}, - { HDB_INTERFACE_VERSION, 0, 0, NULL, NULL, "ldapi:", hdb_ldapi_create}, + { HDB_INTERFACE_VERSION, NULL, NULL, 0, 0, "ldap:", hdb_ldap_create}, + { HDB_INTERFACE_VERSION, NULL, NULL, 0, 0, "ldapi:", hdb_ldapi_create}, #elif defined(OPENLDAP) - { HDB_INTERFACE_VERSION, 0, 0, NULL, NULL, "ldap:", NULL}, - { HDB_INTERFACE_VERSION, 0, 0, NULL, NULL, "ldapi:", NULL}, + { HDB_INTERFACE_VERSION, NULL, NULL, 0, 0, "ldap:", NULL}, + { HDB_INTERFACE_VERSION, NULL, NULL, 0, 0, "ldapi:", NULL}, #endif - { 0, 0, 0, NULL, NULL, NULL, NULL} + { 0, NULL, NULL, 0, 0, NULL, NULL} }; /** @@ -494,19 +494,19 @@ hdb_init_db(krb5_context context, HDB *db) */ #if defined(HAVE_LMDB) static struct hdb_method default_dbmethod = - { HDB_INTERFACE_VERSION, 1, 1, NULL, NULL, "", hdb_mdb_create }; + { HDB_INTERFACE_VERSION, NULL, NULL, 1, 1, "", hdb_mdb_create }; #elif defined(HAVE_DB3) static struct hdb_method default_dbmethod = - { HDB_INTERFACE_VERSION, 1, 1, NULL, NULL, "", hdb_db3_create }; + { HDB_INTERFACE_VERSION, NULL, NULL, 1, 1, "", hdb_db3_create }; #elif defined(HAVE_DB1) static struct hdb_method default_dbmethod = - { HDB_INTERFACE_VERSION, 1, 1, NULL, NULL, "", hdb_db1_create }; + { HDB_INTERFACE_VERSION, NULL, NULL, 1, 1, "", hdb_db1_create }; #elif defined(HAVE_NDBM) static struct hdb_method default_dbmethod = - { HDB_INTERFACE_VERSION, 0, 1, NULL, NULL, "", hdb_ndbm_create }; + { HDB_INTERFACE_VERSION, NULL, NULL, 0, 1, "", hdb_ndbm_create }; #else static struct hdb_method default_dbmethod = - { 0, 0, 0, NULL, NULL, NULL, NULL}; + { 0, NULL, NULL, 0, 0, NULL, NULL}; #endif static int diff --git a/lib/hdb/hdb.h b/lib/hdb/hdb.h index 0f2c92151..2a3d1c4bb 100644 --- a/lib/hdb/hdb.h +++ b/lib/hdb/hdb.h @@ -307,11 +307,9 @@ typedef struct HDB { #define HDB_INTERFACE_VERSION 11 struct hdb_method { - int version; + HEIM_PLUGIN_FTABLE_COMMON_ELEMENTS(krb5_context); unsigned int is_file_based:1; unsigned int can_taste:1; - krb5_error_code (*init)(krb5_context, void **); - void (*fini)(void *); const char *prefix; krb5_error_code (*create)(krb5_context, HDB **, const char *filename); }; diff --git a/lib/hdb/test_namespace.c b/lib/hdb/test_namespace.c index a4c44ba19..f9b4cdbdd 100644 --- a/lib/hdb/test_namespace.c +++ b/lib/hdb/test_namespace.c @@ -243,17 +243,17 @@ struct hdb_method hdb_test = #ifdef WIN32 /* Not c99 */ HDB_INTERFACE_VERSION, - 1 /*is_file_based*/, 1 /*can_taste*/, hdb_test_init, hdb_test_fini, + 1 /*is_file_based*/, 1 /*can_taste*/, "test", hdb_test_create #else - .version = HDB_INTERFACE_VERSION, - .is_file_based = 1, - .can_taste = 1, + .minor_version = HDB_INTERFACE_VERSION, .init = hdb_test_init, .fini = hdb_test_fini, + .is_file_based = 1, + .can_taste = 1, .prefix = "test", .create = hdb_test_create #endif diff --git a/lib/kadm5/kadm5-hook.h b/lib/kadm5/kadm5-hook.h index 78236a5ed..7d47f556a 100644 --- a/lib/kadm5/kadm5-hook.h +++ b/lib/kadm5/kadm5-hook.h @@ -35,6 +35,8 @@ #define KADM5_HOOK_VERSION_V1 1 +#include + /* * Each hook is called before the operation using KADM5_STAGE_PRECOMMIT and * then after the operation using KADM5_STAGE_POSTCOMMIT. If the hook returns @@ -53,9 +55,7 @@ enum kadm5_hook_stage { #define KADM5_HOOK_FLAG_CONDITIONAL 0x2 /* only change password if different */ typedef struct kadm5_hook_ftable { - int version; - krb5_error_code (KRB5_CALLCONV *init)(krb5_context, void **data); - void (KRB5_CALLCONV *fini)(void *data); + HEIM_PLUGIN_FTABLE_COMMON_ELEMENTS(krb5_context); const char *name; const char *vendor; diff --git a/lib/krb5/an2ln_plugin.h b/lib/krb5/an2ln_plugin.h index 89913b578..b592f23b8 100644 --- a/lib/krb5/an2ln_plugin.h +++ b/lib/krb5/an2ln_plugin.h @@ -36,6 +36,8 @@ #ifndef HEIMDAL_KRB5_AN2LN_PLUGIN_H #define HEIMDAL_KRB5_AN2LN_PLUGIN_H 1 +#include + #define KRB5_PLUGIN_AN2LN "an2ln" #define KRB5_PLUGIN_AN2LN_VERSION_0 0 @@ -80,9 +82,7 @@ typedef krb5_error_code (KRB5_LIB_CALL *set_result_f)(void *, const char *); * @ingroup krb5_support */ typedef struct krb5plugin_an2ln_ftable_desc { - int minor_version; - krb5_error_code (KRB5_LIB_CALL *init)(krb5_context, void **); - void (KRB5_LIB_CALL *fini)(void *); + HEIM_PLUGIN_FTABLE_COMMON_ELEMENTS(krb5_context); krb5_error_code (KRB5_LIB_CALL *an2ln)(void *, krb5_context, const char *, krb5_const_principal, set_result_f, void *); } krb5plugin_an2ln_ftable; diff --git a/lib/krb5/db_plugin.h b/lib/krb5/db_plugin.h index 730c06095..ab676d51a 100644 --- a/lib/krb5/db_plugin.h +++ b/lib/krb5/db_plugin.h @@ -33,6 +33,8 @@ #ifndef HEIMDAL_KRB5_DB_PLUGIN_H #define HEIMDAL_KRB5_DB_PLUGIN_H 1 +#include + #define KRB5_PLUGIN_DB "krb5_db_plug" #define KRB5_PLUGIN_DB_VERSION_0 0 @@ -59,9 +61,7 @@ * @ingroup krb5_support */ typedef struct krb5plugin_db_ftable_desc { - int minor_version; - krb5_error_code (KRB5_LIB_CALL *init)(krb5_context, void **); - void (KRB5_LIB_CALL *fini)(void *); + HEIM_PLUGIN_FTABLE_COMMON_ELEMENTS(krb5_context); } krb5plugin_db_ftable; #endif /* HEIMDAL_KRB5_DB_PLUGIN_H */ diff --git a/lib/krb5/kuserok_plugin.h b/lib/krb5/kuserok_plugin.h index b45071d18..7c3f3b4c8 100644 --- a/lib/krb5/kuserok_plugin.h +++ b/lib/krb5/kuserok_plugin.h @@ -32,6 +32,8 @@ #ifndef HEIMDAL_KRB5_KUSEROK_PLUGIN_H #define HEIMDAL_KRB5_KUSEROK_PLUGIN_H 1 +#include + #define KRB5_PLUGIN_KUSEROK "krb5_plugin_kuserok" #define KRB5_PLUGIN_KUSEROK_VERSION_0 0 @@ -76,9 +78,7 @@ * @ingroup krb5_support */ typedef struct krb5plugin_kuserok_ftable_desc { - int minor_version; - krb5_error_code (KRB5_LIB_CALL *init)(krb5_context, void **); - void (KRB5_LIB_CALL *fini)(void *); + HEIM_PLUGIN_FTABLE_COMMON_ELEMENTS(krb5_context); krb5_error_code (KRB5_LIB_CALL *kuserok)(void *, krb5_context, const char *, unsigned int, const char *, const char *, krb5_const_principal, diff --git a/lib/krb5/locate_plugin.h b/lib/krb5/locate_plugin.h index 52ef0f380..7fcb5ec6f 100644 --- a/lib/krb5/locate_plugin.h +++ b/lib/krb5/locate_plugin.h @@ -38,6 +38,8 @@ #ifndef HEIMDAL_KRB5_LOCATE_PLUGIN_H #define HEIMDAL_KRB5_LOCATE_PLUGIN_H 1 +#include + #define KRB5_PLUGIN_LOCATE "service_locator" #define KRB5_PLUGIN_LOCATE_VERSION 1 #define KRB5_PLUGIN_LOCATE_VERSION_0 0 @@ -70,9 +72,7 @@ typedef krb5_error_code typedef struct krb5plugin_service_locate_ftable { - int minor_version; - krb5_error_code (KRB5_CALLCONV *init)(krb5_context, void **); - void (KRB5_CALLCONV *fini)(void *); + HEIM_PLUGIN_FTABLE_COMMON_ELEMENTS(krb5_context); krb5plugin_service_locate_lookup_old old_lookup; krb5plugin_service_locate_lookup lookup; /* version 2 */ } krb5plugin_service_locate_ftable; diff --git a/lib/krb5/send_to_kdc_plugin.h b/lib/krb5/send_to_kdc_plugin.h index 0fa43d3ab..30d6892e5 100644 --- a/lib/krb5/send_to_kdc_plugin.h +++ b/lib/krb5/send_to_kdc_plugin.h @@ -37,6 +37,7 @@ #define HEIMDAL_KRB5_SEND_TO_KDC_PLUGIN_H 1 #include +#include #define KRB5_PLUGIN_SEND_TO_KDC "send_to_kdc" @@ -61,9 +62,7 @@ typedef krb5_error_code typedef struct krb5plugin_send_to_kdc_ftable { - int minor_version; - krb5_error_code (KRB5_CALLCONV *init)(krb5_context, void **); - void (KRB5_CALLCONV *fini)(void *); + HEIM_PLUGIN_FTABLE_COMMON_ELEMENTS(krb5_context); krb5plugin_send_to_kdc_func send_to_kdc; krb5plugin_send_to_realm_func send_to_realm; /* added in version 2 */ } krb5plugin_send_to_kdc_ftable;