From b0a61849efb3cbf0f1c0fead0f422341a969458c Mon Sep 17 00:00:00 2001 From: Simon Tatham Date: Sat, 17 Sep 2022 07:53:43 +0100 Subject: [PATCH 1/6] Unix GSSAPI: support krb5-config as well as pkg-config. On FreeBSD, I'm told, you can't configure Kerberos via pkg-config. So we need a fallback. Here's some manual code to run krb5-config and pick apart the result, similar to what I already did with gtk-config for our (still not dead!) GTK 1 support. --- cmake/platforms/unix.cmake | 63 +++++++++++++++++++++++++++++++++++++- 1 file changed, 62 insertions(+), 1 deletion(-) diff --git a/cmake/platforms/unix.cmake b/cmake/platforms/unix.cmake index 291d1e64..95339f22 100644 --- a/cmake/platforms/unix.cmake +++ b/cmake/platforms/unix.cmake @@ -108,16 +108,77 @@ if(PUTTY_GSSAPI STREQUAL DYNAMIC) endif() if(PUTTY_GSSAPI STREQUAL STATIC) + set(KRB5_CFLAGS) + set(KRB5_LDFLAGS) + + # First try using pkg-config find_package(PkgConfig) pkg_check_modules(KRB5 krb5-gssapi) + + # Failing that, try the dedicated krb5-config + if(NOT KRB5_FOUND) + find_program(KRB5_CONFIG krb5-config) + if(KRB5_CONFIG) + execute_process(COMMAND ${KRB5_CONFIG} --cflags gssapi + OUTPUT_VARIABLE krb5_config_cflags + OUTPUT_STRIP_TRAILING_WHITESPACE + RESULT_VARIABLE krb5_config_cflags_result) + execute_process(COMMAND ${KRB5_CONFIG} --libs gssapi + OUTPUT_VARIABLE krb5_config_libs + OUTPUT_STRIP_TRAILING_WHITESPACE + RESULT_VARIABLE krb5_config_libs_result) + + if(krb5_config_cflags_result EQUAL 0 AND krb5_config_libs_result EQUAL 0) + set(KRB5_INCLUDE_DIRS) + set(KRB5_LIBRARY_DIRS) + set(KRB5_LIBRARIES) + + # We can safely put krb5-config's cflags directly into cmake's + # cflags, without bothering to extract the include directories. + set(KRB5_CFLAGS ${krb5_config_cflags}) + + # But krb5-config --libs isn't so simple. It will actually + # deliver a mix of libraries and other linker options. We have + # to separate them for cmake purposes, because if we pass the + # whole lot to add_link_options then they'll appear too early + # in the command line (so that by the time our own code refers + # to GSSAPI functions it'll be too late to search these + # libraries for them), and if we pass the whole lot to + # link_libraries then it'll get confused about options that + # aren't libraries. + separate_arguments(krb5_config_libs NATIVE_COMMAND + ${krb5_config_libs}) + foreach(opt ${krb5_config_libs}) + string(REGEX MATCH "^-l" ok ${opt}) + if(ok) + list(APPEND KRB5_LIBRARIES ${opt}) + continue() + endif() + string(REGEX MATCH "^-L" ok ${opt}) + if(ok) + string(REGEX REPLACE "^-L" "" optval ${opt}) + list(APPEND KRB5_LIBRARY_DIRS ${optval}) + continue() + endif() + list(APPEND KRB5_LDFLAGS ${opt}) + endforeach() + + message(STATUS "Found Kerberos via krb5-config") + set(KRB5_FOUND YES) + endif() + endif() + endif() + if(KRB5_FOUND) include_directories(${KRB5_INCLUDE_DIRS}) link_directories(${KRB5_LIBRARY_DIRS}) link_libraries(${KRB5_LIBRARIES}) + add_compile_options(${KRB5_CFLAGS}) + add_link_options(${KRB5_LDFLAGS}) set(STATIC_GSSAPI ON) else() message(WARNING - "Could not find krb5 via pkg-config -- \ + "Could not find krb5 via pkg-config or krb5-config -- \ cannot provide static GSSAPI support") set(NO_GSSAPI ON) endif() From 374107eb1e2ae576c10cdd538f45f18918df8c4b Mon Sep 17 00:00:00 2001 From: Simon Tatham Date: Sat, 17 Sep 2022 07:09:29 +0100 Subject: [PATCH 2/6] Unix static GSSAPI: fix an uninitialised structure field. When linking statically against Kerberos, the setup code in ssh_got_ssh_version() was trying to look up want_id==0 in the list of one GSSAPI library, but unfortunately, the id field of that record was not initialised at all, so if it happened to be nonzero nonsense, the loop wouldn't find a library at all and would fail an assertion. --- unix/gss.c | 1 + 1 file changed, 1 insertion(+) diff --git a/unix/gss.c b/unix/gss.c index cd9971c7..bd599fcc 100644 --- a/unix/gss.c +++ b/unix/gss.c @@ -140,6 +140,7 @@ struct ssh_gss_liblist *ssh_gss_setup(Conf *conf) list->libraries = snew(struct ssh_gss_library); list->nlibraries = 1; + list->libraries[0].id = 0; list->libraries[0].gsslogmsg = "Using statically linked GSSAPI"; #define BIND_GSS_FN(name) \ From 35a87984f67ebc2db3f670cb1431f08991853a5e Mon Sep 17 00:00:00 2001 From: Simon Tatham Date: Sat, 17 Sep 2022 07:28:46 +0100 Subject: [PATCH 3/6] Unix GSSAPI: support static linking against Heimdal. Heimdal provides its own definitions of OIDs like GSS_C_NT_USER_NAME in the form of macros, which conflict with our attempt to redefine them as variables - the macro gets expanded into the middle of the variable declaration, leaving the poor C compiler trying to parse a non-declaration along the lines of const_gss_OID (&__gss_c_nt_anonymous_oid_desc) = oids+5; Easily fixed by just not redefining these at all if they're already defined as macros. To make that easier, I've broken up the oids[] array into individual gss_OID_desc declarations, so I can put each one inside the appropriate ifdef. In the process, I've removed the 'const' from the gss_OID_desc declarations. That's on purpose! The problem is that not all implementations of the GSSAPI headers make const_gss_OID a pointer to a *const* gss_OID_desc; sometimes it's just a plain one and the 'const' prefix is just a comment to the user. So removing that const prevents compiler warnings (or worse) about address-taking a const thing and assigning it into a non-const pointer. --- ssh/pgssapi.c | 106 ++++++++++++++++++++++++++++++++------------------ 1 file changed, 68 insertions(+), 38 deletions(-) diff --git a/ssh/pgssapi.c b/ssh/pgssapi.c index 1f54d805..1730444d 100644 --- a/ssh/pgssapi.c +++ b/ssh/pgssapi.c @@ -9,38 +9,63 @@ #ifndef NO_LIBDL -/* Reserved static storage for GSS_oids. Comments are quotes from RFC 2744. */ -static const gss_OID_desc oids[] = { +/* Reserved static storage for GSS_oids. + * Constants of the form GSS_C_NT_* are specified by rfc 2744. + * Comments are quotes from RFC 2744 itself. + * + * These may be #defined to complex expressions by the local header + * file, if we're including one in static-GSSAPI mode. (For example, + * Heimdal defines them to things like + * (&__gss_c_nt_user_name_oid_desc).) So we only define them if + * needed. */ + +#ifndef GSS_C_NT_USER_NAME +static gss_OID_desc oid_GSS_C_NT_USER_NAME = { /* The implementation must reserve static storage for a * gss_OID_desc object containing the value */ - {10, (void *)"\x2a\x86\x48\x86\xf7\x12\x01\x02\x01\x01"}, + 10, "\x2a\x86\x48\x86\xf7\x12\x01\x02\x01\x01", /* corresponding to an object-identifier value of * {iso(1) member-body(2) United States(840) mit(113554) * infosys(1) gssapi(2) generic(1) user_name(1)}. The constant * GSS_C_NT_USER_NAME should be initialized to point - * to that gss_OID_desc. + * to that gss_OID_desc. */ +}; +const_gss_OID GSS_C_NT_USER_NAME = &oid_GSS_C_NT_USER_NAME; +#endif - * The implementation must reserve static storage for a +#ifndef GSS_C_NT_MACHINE_UID_NAME +static gss_OID_desc oid_GSS_C_NT_MACHINE_UID_NAME = { + /* The implementation must reserve static storage for a * gss_OID_desc object containing the value */ - {10, (void *)"\x2a\x86\x48\x86\xf7\x12\x01\x02\x01\x02"}, + 10, "\x2a\x86\x48\x86\xf7\x12\x01\x02\x01\x02", /* corresponding to an object-identifier value of * {iso(1) member-body(2) United States(840) mit(113554) * infosys(1) gssapi(2) generic(1) machine_uid_name(2)}. * The constant GSS_C_NT_MACHINE_UID_NAME should be - * initialized to point to that gss_OID_desc. + * initialized to point to that gss_OID_desc. */ +}; +const_gss_OID GSS_C_NT_MACHINE_UID_NAME = &oid_GSS_C_NT_MACHINE_UID_NAME; +#endif - * The implementation must reserve static storage for a +#ifndef GSS_C_NT_STRING_UID_NAME +static gss_OID_desc oid_GSS_C_NT_STRING_UID_NAME = { + /* The implementation must reserve static storage for a * gss_OID_desc object containing the value */ - {10, (void *)"\x2a\x86\x48\x86\xf7\x12\x01\x02\x01\x03"}, + 10, "\x2a\x86\x48\x86\xf7\x12\x01\x02\x01\x03", /* corresponding to an object-identifier value of * {iso(1) member-body(2) United States(840) mit(113554) * infosys(1) gssapi(2) generic(1) string_uid_name(3)}. * The constant GSS_C_NT_STRING_UID_NAME should be - * initialized to point to that gss_OID_desc. - * - * The implementation must reserve static storage for a + * initialized to point to that gss_OID_desc. */ +}; +const_gss_OID GSS_C_NT_STRING_UID_NAME = &oid_GSS_C_NT_STRING_UID_NAME; +#endif + +#ifndef GSS_C_NT_HOSTBASED_SERVICE_X +static gss_OID_desc oid_GSS_C_NT_HOSTBASED_SERVICE_X = { + /* The implementation must reserve static storage for a * gss_OID_desc object containing the value */ - {6, (void *)"\x2b\x06\x01\x05\x06\x02"}, + 6, "\x2b\x06\x01\x05\x06\x02", /* corresponding to an object-identifier value of * {iso(1) org(3) dod(6) internet(1) security(5) * nametypes(6) gss-host-based-services(2))}. The constant @@ -52,29 +77,44 @@ static const gss_OID_desc oids[] = { * GSS_C_NT_HOSTBASED_SERVICE_X should be accepted a synonym * for GSS_C_NT_HOSTBASED_SERVICE when presented as an input * parameter, but should not be emitted by GSS-API - * implementations - * - * The implementation must reserve static storage for a + * implementations */ +}; +const_gss_OID GSS_C_NT_HOSTBASED_SERVICE_X = &oid_GSS_C_NT_HOSTBASED_SERVICE_X; +#endif + +#ifndef GSS_C_NT_HOSTBASED_SERVICE +static gss_OID_desc oid_GSS_C_NT_HOSTBASED_SERVICE = { + /* The implementation must reserve static storage for a * gss_OID_desc object containing the value */ - {10, (void *)"\x2a\x86\x48\x86\xf7\x12\x01\x02\x01\x04"}, + 10, "\x2a\x86\x48\x86\xf7\x12\x01\x02\x01\x04", /* corresponding to an object-identifier value of {iso(1) * member-body(2) Unites States(840) mit(113554) infosys(1) * gssapi(2) generic(1) service_name(4)}. The constant * GSS_C_NT_HOSTBASED_SERVICE should be initialized - * to point to that gss_OID_desc. - * - * The implementation must reserve static storage for a + * to point to that gss_OID_desc. */ +}; +const_gss_OID GSS_C_NT_HOSTBASED_SERVICE = &oid_GSS_C_NT_HOSTBASED_SERVICE; +#endif + +#ifndef GSS_C_NT_ANONYMOUS +static gss_OID_desc oid_GSS_C_NT_ANONYMOUS = { + /* The implementation must reserve static storage for a * gss_OID_desc object containing the value */ - {6, (void *)"\x2b\x06\01\x05\x06\x03"}, + 6, "\x2b\x06\01\x05\x06\x03", /* corresponding to an object identifier value of * {1(iso), 3(org), 6(dod), 1(internet), 5(security), * 6(nametypes), 3(gss-anonymous-name)}. The constant * and GSS_C_NT_ANONYMOUS should be initialized to point - * to that gss_OID_desc. - * - * The implementation must reserve static storage for a + * to that gss_OID_desc. */ +}; +const_gss_OID GSS_C_NT_ANONYMOUS = &oid_GSS_C_NT_ANONYMOUS; +#endif + +#ifndef GSS_C_NT_EXPORT_NAME +static gss_OID_desc oid_GSS_C_NT_EXPORT_NAME = { + /* The implementation must reserve static storage for a * gss_OID_desc object containing the value */ - {6, (void *)"\x2b\x06\x01\x05\x06\x04"}, + 6, "\x2b\x06\x01\x05\x06\x04", /* corresponding to an object-identifier value of * {1(iso), 3(org), 6(dod), 1(internet), 5(security), * 6(nametypes), 4(gss-api-exported-name)}. The constant @@ -82,23 +122,13 @@ static const gss_OID_desc oids[] = { * to that gss_OID_desc. */ }; - -/* Here are the constants which point to the static structure above. - * - * Constants of the form GSS_C_NT_* are specified by rfc 2744. - */ -const_gss_OID GSS_C_NT_USER_NAME = oids+0; -const_gss_OID GSS_C_NT_MACHINE_UID_NAME = oids+1; -const_gss_OID GSS_C_NT_STRING_UID_NAME = oids+2; -const_gss_OID GSS_C_NT_HOSTBASED_SERVICE_X = oids+3; -const_gss_OID GSS_C_NT_HOSTBASED_SERVICE = oids+4; -const_gss_OID GSS_C_NT_ANONYMOUS = oids+5; -const_gss_OID GSS_C_NT_EXPORT_NAME = oids+6; +const_gss_OID GSS_C_NT_EXPORT_NAME = &oid_GSS_C_NT_EXPORT_NAME; +#endif #endif /* NO_LIBDL */ static gss_OID_desc gss_mech_krb5_desc = -{ 9, (void *)"\x2a\x86\x48\x86\xf7\x12\x01\x02\x02" }; +{ 9, "\x2a\x86\x48\x86\xf7\x12\x01\x02\x02" }; /* iso(1) member-body(2) United States(840) mit(113554) infosys(1) gssapi(2) krb5(2)*/ const gss_OID GSS_MECH_KRB5 = &gss_mech_krb5_desc; From a95e38e9b18ce69b542a9a8c0f18ea8f4c7abb3a Mon Sep 17 00:00:00 2001 From: Simon Tatham Date: Sat, 17 Sep 2022 07:50:55 +0100 Subject: [PATCH 4/6] GSSAPI fix: don't pass GSS_C_NO_NAME to inquire_cred_by_mech. This was pointed out by another compiler warning. The 'name' parameter of inquire_cred_by_mech is not a gss_name_t (which is the type of GSS_C_NO_NAME); it's a gss_name_t *, because it's an _output_ parameter. We're not telling the library that we aren't _passing_ a name: we're telling it that we don't need it to _return_ us a name. So the appropriate null pointer representation is just NULL. (This was harmless apart from a compiler warning, because gss_name_t is a pointer type in turn and GSS_C_NO_NAME expands to a null pointer anyway. It was just a wrongly-typed null pointer.) --- ssh/gssc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ssh/gssc.c b/ssh/gssc.c index 0224afe2..d10caf8b 100644 --- a/ssh/gssc.c +++ b/ssh/gssc.c @@ -75,7 +75,7 @@ static Ssh_gss_stat ssh_gssapi_acquire_cred(struct ssh_gss_library *lib, gssctx->maj_stat = gss->inquire_cred_by_mech(&gssctx->min_stat, cred, (gss_OID) GSS_MECH_KRB5, - GSS_C_NO_NAME, + NULL, &time_rec, NULL, NULL); From 732ec31a17a7feac9c24c427f79734e9a97b922f Mon Sep 17 00:00:00 2001 From: Simon Tatham Date: Sun, 18 Sep 2022 15:02:32 +0100 Subject: [PATCH 5/6] Add explicit cmake setting for 'build without GTK'. If you have GTK installed on your system but want to build without it anyway (e.g. if you're buliding a package suitable for headless systems), it's useful to be able to explicitly instruct PuTTY's build system not to use GTK even if it's there. This would already work if you unilaterally set PUTTY_GTK_VERSION to some value other than 1, 2, 3 or ANY. Added NONE as an officially supported option, and included it in the list that cmake-gui will present. Also, made the check for libX11 conditional on having GTK, since there's no need to bother with it otherwise. --- cmake/gtk.cmake | 2 +- cmake/platforms/unix.cmake | 28 +++++++++++++++------------- 2 files changed, 16 insertions(+), 14 deletions(-) diff --git a/cmake/gtk.cmake b/cmake/gtk.cmake index 85ecb7de..34396d2f 100644 --- a/cmake/gtk.cmake +++ b/cmake/gtk.cmake @@ -3,7 +3,7 @@ set(PUTTY_GTK_VERSION "ANY" CACHE STRING "Which major version of GTK to build with") set_property(CACHE PUTTY_GTK_VERSION - PROPERTY STRINGS ANY 3 2 1) + PROPERTY STRINGS ANY 3 2 1 NONE) set(GTK_FOUND FALSE) diff --git a/cmake/platforms/unix.cmake b/cmake/platforms/unix.cmake index 95339f22..04db8129 100644 --- a/cmake/platforms/unix.cmake +++ b/cmake/platforms/unix.cmake @@ -65,21 +65,23 @@ endif() include(cmake/gtk.cmake) -# See if we have X11 available. This requires libX11 itself, and also -# the GDK integration to X11. -find_package(X11) +if(GTK_FOUND) + # See if we have X11 available. This requires libX11 itself, and also + # the GDK integration to X11. + find_package(X11) -function(check_x11) - list(APPEND CMAKE_REQUIRED_INCLUDES ${GTK_INCLUDE_DIRS}) - check_include_file(gdk/gdkx.h HAVE_GDK_GDKX_H) + function(check_x11) + list(APPEND CMAKE_REQUIRED_INCLUDES ${GTK_INCLUDE_DIRS}) + check_include_file(gdk/gdkx.h HAVE_GDK_GDKX_H) - if(X11_FOUND AND HAVE_GDK_GDKX_H) - set(NOT_X_WINDOWS OFF PARENT_SCOPE) - else() - set(NOT_X_WINDOWS ON PARENT_SCOPE) - endif() -endfunction() -check_x11() + if(X11_FOUND AND HAVE_GDK_GDKX_H) + set(NOT_X_WINDOWS OFF PARENT_SCOPE) + else() + set(NOT_X_WINDOWS ON PARENT_SCOPE) + endif() + endfunction() + check_x11() +endif() include_directories(${CMAKE_SOURCE_DIR}/charset ${GTK_INCLUDE_DIRS} ${X11_INCLUDE_DIR}) link_directories(${GTK_LIBRARY_DIRS}) From fda41e199093b41f026fb8c06e8e2cddcace4d83 Mon Sep 17 00:00:00 2001 From: Simon Tatham Date: Sun, 18 Sep 2022 15:08:31 +0100 Subject: [PATCH 6/6] Add cmake check for whether setpgrp takes arguments. FreeBSD declares setpgrp() as taking two arguments, like Linux's setpgid(). Detect that at configure time and adjust the call in Pageant appropriately. --- cmake/cmake.h.in | 2 ++ cmake/platforms/unix.cmake | 15 +++++++++++++++ unix/pageant.c | 4 ++++ 3 files changed, 21 insertions(+) diff --git a/cmake/cmake.h.in b/cmake/cmake.h.in index 4ce869f4..91d52d78 100644 --- a/cmake/cmake.h.in +++ b/cmake/cmake.h.in @@ -43,6 +43,8 @@ #cmakedefine01 HAVE_CLOCK_MONOTONIC #cmakedefine01 HAVE_CLOCK_GETTIME #cmakedefine01 HAVE_SO_PEERCRED +#cmakedefine01 HAVE_NULLARY_SETPGRP +#cmakedefine01 HAVE_BINARY_SETPGRP #cmakedefine01 HAVE_PANGO_FONT_FAMILY_IS_MONOSPACE #cmakedefine01 HAVE_PANGO_FONT_MAP_LIST_FAMILIES diff --git a/cmake/platforms/unix.cmake b/cmake/platforms/unix.cmake index 04db8129..98474485 100644 --- a/cmake/platforms/unix.cmake +++ b/cmake/platforms/unix.cmake @@ -51,6 +51,21 @@ int main(int argc, char **argv) { cr.pid + cr.uid + cr.gid; }" HAVE_SO_PEERCRED) +check_c_source_compiles(" +#include +#include + +int main(int argc, char **argv) { + setpgrp(); +}" HAVE_NULLARY_SETPGRP) +check_c_source_compiles(" +#include +#include + +int main(int argc, char **argv) { + setpgrp(0, 0); +}" HAVE_BINARY_SETPGRP) + if(HAVE_GETADDRINFO AND PUTTY_IPV6) set(NO_IPV6 OFF) else() diff --git a/unix/pageant.c b/unix/pageant.c index cef57b5b..6d125fce 100644 --- a/unix/pageant.c +++ b/unix/pageant.c @@ -330,7 +330,11 @@ void pageant_fork_and_print_env(bool retain_tty) /* Get out of our previous process group, to avoid being * blasted by passing signals. But keep our controlling tty, * so we can keep checking to see if we still have one. */ +#if defined HAVE_NULLARY_SETPGRP setpgrp(); +#elif defined HAVE_BINARY_SETPGRP + setpgrp(0, 0); +#endif } else { /* Do that, but also leave our entire session and detach from * the controlling tty (if any). */