Hi, I've encountered an issue in libkrb5 while running MongoDB with Valgrind, and authenticating to it with GSSAPI. Here is a keytab file that I've made for reproducing an issue that was reported to me: sajack@spencerjacksonDesktop /home/sajack/mongo % klist -kt archTest.keytab Keytab name: FILE:archTest.keytab KVNO Timestamp Principal ---- ------------------- ------------------------------------------------------ 2 05/09/2016 16:14:02 mongodb@MONGOTEST.COM 2 05/09/2016 16:14:02 mongodb/database.mongotest.com@MONGOTEST.COM 2 05/09/2016 16:14:02 mongodb/database.mongotest.com@MONGOTEST.COM 2 05/09/2016 16:14:02 mongodb/database.mongotest.com@MONGOTEST.COM 2 05/09/2016 16:14:02 mongodb/database.mongotest.com@MONGOTEST.COM While using this keytab, Valgrind reports the following warnings: ==31907== Thread 18: ==31907== Invalid read of size 4 ==31907== at 0x7EE68E0: data_eq (k5-int.h:2233) ==31907== by 0x7EE68E0: krb5_sname_match (sname_match.c:49) ==31907== by 0x55D07A0: check_keytab (acquire_cred.c:156) ==31907== by 0x55D07A0: acquire_accept_cred (acquire_cred.c:225) ==31907== by 0x55D07A0: acquire_cred_context.isra.10 (acquire_cred.c:797) ==31907== by 0x55D0F99: krb5_gss_acquire_cred_from (acquire_cred.c:1224) ==31907== by 0x55BEF4F: gss_add_cred_from (g_acquire_cred.c:455) ==31907== by 0x55BF55A: gss_acquire_cred_from (g_acquire_cred.c:190) ==31907== by 0x55BF715: gss_acquire_cred (g_acquire_cred.c:107) ==31907== by 0xAA30120: gssapi_server_mech_authneg (gssapi.c:721) ==31907== by 0xAA31803: gssapi_server_mech_step (gssapi.c:1267) ==31907== by 0x53A5E72: sasl_server_step (server.c:1618) ==31907== by 0x53A5BF2: sasl_server_start (server.c:1533) ==31907== by 0x1960C5E: mongo::CyrusSaslAuthenticationSession::step(mongo::StringData, std::__1::basic_string, std::__1::allocator >*) (cyrus_sasl_authentication_session.cpp:353) ==31907== by 0x13BCC1B: mongo::(anonymous namespace)::doSaslStep(mongo::ClientBasic const*, mongo::SaslAuthenticationSession*, mongo::BS$ NObj const&, mongo::BSONObjBuilder*) (sasl_commands.cpp:187) ==31907== Address 0x117ea934 is 4 bytes after a block of size 16 alloc'd ==31907== at 0x5184947: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) ==31907== by 0x7EBD4B2: krb5_ktfileint_internal_read_entry.isra.5 (kt_file.c:1243) ==31907== by 0x7EBECEB: krb5_ktfileint_read_entry (kt_file.c:1407) ==31907== by 0x7EBECEB: krb5_ktfile_get_next (kt_file.c:508) ==31907== by 0x55D07C8: check_keytab (acquire_cred.c:155) ==31907== by 0x55D07C8: acquire_accept_cred (acquire_cred.c:225) ==31907== by 0x55D07C8: acquire_cred_context.isra.10 (acquire_cred.c:797) ==31907== by 0x55D0F99: krb5_gss_acquire_cred_from (acquire_cred.c:1224) ==31907== by 0x55BEF4F: gss_add_cred_from (g_acquire_cred.c:455) ==31907== by 0x55BF55A: gss_acquire_cred_from (g_acquire_cred.c:190) [16/1973] ==31907== by 0x55BF715: gss_acquire_cred (g_acquire_cred.c:107) ==31907== by 0xAA30120: gssapi_server_mech_authneg (gssapi.c:721) ==31907== by 0xAA31803: gssapi_server_mech_step (gssapi.c:1267) ==31907== by 0x53A5E72: sasl_server_step (server.c:1618) ==31907== by 0x53A5BF2: sasl_server_start (server.c:1533) ==31907== ==31907== Invalid read of size 4 ==31907== at 0x7EE68E0: data_eq (k5-int.h:2233) ==31907== by 0x7EE68E0: krb5_sname_match (sname_match.c:49) ==31907== by 0x7EDEF6E: decrypt_ticket (rd_req_dec.c:396) ==31907== by 0x7EDEF6E: rd_req_decoded_opt (rd_req_dec.c:504) ==31907== by 0x7EDFBAE: krb5_rd_req_decoded (rd_req_dec.c:808) ==31907== by 0x55CDEF1: kg_accept_krb5 (accept_sec_context.c:642) ==31907== by 0x55CF3B0: krb5_gss_accept_sec_context_ext (accept_sec_context.c:1303) ==31907== by 0x55CF4E1: krb5_gss_accept_sec_context (accept_sec_context.c:1332) ==31907== by 0x55BE681: gss_accept_sec_context (g_accept_sec_context.c:267) ==31907== by 0xAA30243: gssapi_server_mech_authneg (gssapi.c:747) ==31907== by 0xAA31803: gssapi_server_mech_step (gssapi.c:1267) ==31907== by 0x53A5E72: sasl_server_step (server.c:1618) ==31907== by 0x53A5BF2: sasl_server_start (server.c:1533) ==31907== by 0x1960C5E: mongo::CyrusSaslAuthenticationSession::step(mongo::StringData, std::__1::basic_string, std::__1::allocator >*) (cyrus_sasl_authentication_session.cpp:353) ==31907== Address 0x11801304 is 4 bytes after a block of size 16 alloc'd ==31907== at 0x5184947: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) ==31907== by 0x7EBD4B2: krb5_ktfileint_internal_read_entry.isra.5 (kt_file.c:1243) ==31907== by 0x7EBECEB: krb5_ktfileint_read_entry (kt_file.c:1407) ==31907== by 0x7EBECEB: krb5_ktfile_get_next (kt_file.c:508) ==31907== by 0x7EDEF52: decrypt_ticket (rd_req_dec.c:394) ==31907== by 0x7EDEF52: rd_req_decoded_opt (rd_req_dec.c:504) ==31907== by 0x7EDFBAE: krb5_rd_req_decoded (rd_req_dec.c:808) ==31907== by 0x55CDEF1: kg_accept_krb5 (accept_sec_context.c:642) ==31907== by 0x55CF3B0: krb5_gss_accept_sec_context_ext (accept_sec_context.c:1303) ==31907== by 0x55CF4E1: krb5_gss_accept_sec_context (accept_sec_context.c:1332) ==31907== by 0x55BE681: gss_accept_sec_context (g_accept_sec_context.c:267) ==31907== by 0xAA30243: gssapi_server_mech_authneg (gssapi.c:747) ==31907== by 0xAA31803: gssapi_server_mech_step (gssapi.c:1267) ==31907== by 0x53A5E72: sasl_server_step (server.c:1618) My understanding of these warnings is that krb5_sname_match is called against all principals stored in a keytab to find which entry it should accept tickets with. However it does not check if the keytab's principal actually has a hostname, before it is dereferenced and compared to the acceptor hostname. This leads to uninitialized memory being read. In rare situations, if the uninitialized value read out of princ->data[1].length is equal to the length stored in matching->data[1].length, data_eq in k5-int.h could attempt to call memcmp with invalid pointers causing a segfault. The below patch prevents Valgrind from emitting these warnings. >From a46ee6666b2ea7ba9c864a614a61adf7aee0ffd6 Mon Sep 17 00:00:00 2001 From: Spencer Jackson Date: Mon, 9 May 2016 18:23:16 -0400 Subject: [PATCH] Prevent uninitialized read in krb5_sname_match --- src/lib/krb5/krb/sname_match.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/lib/krb5/krb/sname_match.c b/src/lib/krb5/krb/sname_match.c index 0c7bd39..61f1de8 100644 --- a/src/lib/krb5/krb/sname_match.c +++ b/src/lib/krb5/krb/sname_match.c @@ -41,12 +41,13 @@ krb5_sname_match(krb5_context context, krb5_const_principal matching, return FALSE; /* Check the service name (must be present in matching). */ - if (!data_eq(matching->data[0], princ->data[0])) + if (princ->length == 0 || !data_eq(matching->data[0], princ->data[0])) return FALSE; - /* Check the hostname if present in matching and not ignored. */ - if (matching->data[1].length != 0 && !context->ignore_acceptor_hostname && - !data_eq(matching->data[1], princ->data[1])) + /* Check the hostname if present in both matching and princ, and not ignored. + * matching->length == 2, so princ must have greater or equal length to be comparable. */ + if (!context->ignore_acceptor_hostname && matching->data[1].length != 0 && + (princ->length < 2 || !data_eq(matching->data[1], princ->data[1]))) return FALSE; /* All elements match. */ -- 2.8.2