From a660e1060630e8d75ecb43abf69dbb73378c1df9 Mon Sep 17 00:00:00 2001 From: Werner Koch Date: Wed, 19 May 2021 17:18:15 +0200 Subject: [PATCH] dirmngr: For KS_SEARCH return the fingerprint also with LDAP. * dirmngr/ks-engine-ldap.c (extract_keys): Return the fingerprint if available. (ks_ldap_search): Ditto. (extract_keys): Make sure to free the ldap values also in corner cases. (my_ldap_value_free): New. (ks_ldap_get): Ditto. (ks_ldap_search): Ditto. (my_ldap_connect): Ditto. -- For background see these comments from gpgme: /* The output for external keylistings in GnuPG is different from all the other key listings. We catch this here with a special preprocessor that reformats the colon handler lines. */ /* The format is: pub:::::: as defined in 5.2. Machine Readable Indexes of the OpenPGP HTTP Keyserver Protocol (draft). Modern versions of the SKS keyserver return the fingerprint instead of the keyid. We detect this here and use the v4 fingerprint format to convert it to a key id. We want: pub:o::::::::::::: */ Regarding the freeing of values: I was not able to find a specification stating it is okay to pass NULL to ldap_value_free, thus the new wrapper. Also add robustness measures in case ldap_get_value returns an empty array. GnuPG-bug-id: 5441 Signed-off-by: Werner Koch --- dirmngr/ks-engine-ldap.c | 128 ++++++++++++++++++++++++--------------- 1 file changed, 80 insertions(+), 48 deletions(-) diff --git a/dirmngr/ks-engine-ldap.c b/dirmngr/ks-engine-ldap.c index 15def111c..da3d9ee1b 100644 --- a/dirmngr/ks-engine-ldap.c +++ b/dirmngr/ks-engine-ldap.c @@ -295,6 +295,16 @@ epoch2ldaptime (time_t stamp) return xstrdup ("INVALID TIME"); } #endif + + +static void +my_ldap_value_free (char **vals) +{ + if (vals) + ldap_value_free (vals); +} + + /* Print a help output for the schemata supported by this module. */ gpg_error_t @@ -695,26 +705,26 @@ my_ldap_connect (parsed_uri_t uri, LDAP **ldap_connp, { vals = ldap_get_values (ldap_conn, si_res, "pgpBaseKeySpaceDN"); - if (vals) + if (vals && vals[0]) { basedn = xtrystrdup (vals[0]); - ldap_value_free (vals); } + my_ldap_value_free (vals); vals = ldap_get_values (ldap_conn, si_res, "pgpSoftware"); - if (vals) + if (vals && vals[0]) { if (opt.debug) log_debug ("Server: \t%s\n", vals[0]); if (!ascii_strcasecmp (vals[0], "GnuPG")) is_gnupg = 1; - ldap_value_free (vals); } + my_ldap_value_free (vals); vals = ldap_get_values (ldap_conn, si_res, "pgpVersion"); - if (vals) + if (vals && vals[0]) { if (opt.debug) log_debug ("Version:\t%s\n", vals[0]); @@ -730,8 +740,8 @@ my_ldap_connect (parsed_uri_t uri, LDAP **ldap_connp, && !ascii_strcasecmp (fields[1], "ntds")) *r_serverinfo |= SERVERINFO_NTDS; } - ldap_value_free (vals); } + my_ldap_value_free (vals); } /* From man ldap_search_s: "res parameter of @@ -764,22 +774,22 @@ my_ldap_connect (parsed_uri_t uri, LDAP **ldap_connp, * in the future. */ vals = ldap_get_values (ldap_conn, si_res, "baseKeySpaceDN"); - if (vals) + if (vals && vals[0]) { basedn = xtrystrdup (vals[0]); - ldap_value_free (vals); } + my_ldap_value_free (vals); vals = ldap_get_values (ldap_conn, si_res, "software"); - if (vals) + if (vals && vals[0]) { if (opt.debug) log_debug ("ks-ldap: PGP Server: \t%s\n", vals[0]); - ldap_value_free (vals); } + my_ldap_value_free (vals); vals = ldap_get_values (ldap_conn, si_res, "version"); - if (vals) + if (vals && vals[0]) { if (opt.debug) log_debug ("ks-ldap: PGP Server Version:\t%s\n", vals[0]); @@ -793,8 +803,8 @@ my_ldap_connect (parsed_uri_t uri, LDAP **ldap_connp, if (atoi (vals[0]) > 1) *r_serverinfo |= SERVERINFO_PGPKEYV2; - ldap_value_free (vals); } + my_ldap_value_free (vals); } ldap_msgfree (si_res); @@ -848,10 +858,17 @@ extract_keys (estream_t output, char **vals; es_fprintf (output, "INFO %s BEGIN\n", certid); - es_fprintf (output, "pub:%s:", certid); /* Note: ldap_get_values returns a NULL terminated array of strings. */ + + vals = ldap_get_values (ldap_conn, message, "gpgfingerprint"); + if (vals && vals[0] && vals[0][0]) + es_fprintf (output, "pub:%s:", vals[0]); + else + es_fprintf (output, "pub:%s:", certid); + my_ldap_value_free (vals); + vals = ldap_get_values (ldap_conn, message, "pgpkeytype"); if (vals && vals[0]) { @@ -859,8 +876,8 @@ extract_keys (estream_t output, es_fprintf (output, "1"); else if (strcmp (vals[0],"DSS/DH") == 0) es_fprintf (output, "17"); - ldap_value_free (vals); } + my_ldap_value_free (vals); es_fprintf (output, ":"); @@ -870,8 +887,8 @@ extract_keys (estream_t output, int v = atoi (vals[0]); if (v > 0) es_fprintf (output, "%d", v); - ldap_value_free (vals); } + my_ldap_value_free (vals); es_fprintf (output, ":"); @@ -880,8 +897,8 @@ extract_keys (estream_t output, { if (strlen (vals[0]) == 15) es_fprintf (output, "%u", (unsigned int) ldap2epochtime (vals[0])); - ldap_value_free (vals); } + my_ldap_value_free (vals); es_fprintf (output, ":"); @@ -890,8 +907,8 @@ extract_keys (estream_t output, { if (strlen (vals[0]) == 15) es_fprintf (output, "%u", (unsigned int) ldap2epochtime (vals[0])); - ldap_value_free (vals); } + my_ldap_value_free (vals); es_fprintf (output, ":"); @@ -900,8 +917,8 @@ extract_keys (estream_t output, { if (atoi (vals[0]) == 1) es_fprintf (output, "r"); - ldap_value_free (vals); } + my_ldap_value_free (vals); es_fprintf (output, "\n"); @@ -911,8 +928,8 @@ extract_keys (estream_t output, int i; for (i = 0; vals[i]; i++) es_fprintf (output, "uid:%s\n", vals[i]); - ldap_value_free (vals); } + my_ldap_value_free (vals); es_fprintf (output, "INFO %s END\n", certid); } @@ -971,6 +988,7 @@ ks_ldap_get (ctrl_t ctrl, parsed_uri_t uri, const char *keyspec, "dummy", "pgpcertid", "pgpuserid", "pgpkeyid", "pgprevoked", "pgpdisabled", "pgpkeycreatetime", "modifytimestamp", "pgpkeysize", "pgpkeytype", + "gpgfingerprint", NULL }; /* 1 if we want just attribute types; 0 if we want both attribute @@ -1073,7 +1091,7 @@ ks_ldap_get (ctrl_t ctrl, parsed_uri_t uri, const char *keyspec, } } - ldap_value_free (certid); + my_ldap_value_free (certid); } free_strlist (seen); @@ -1178,7 +1196,8 @@ ks_ldap_search (ctrl_t ctrl, parsed_uri_t uri, const char *pattern, { "pgpcertid", "pgpuserid", "pgprevoked", "pgpdisabled", "pgpkeycreatetime", "pgpkeyexpiretime", "modifytimestamp", - "pgpkeysize", "pgpkeytype", NULL + "pgpkeysize", "pgpkeytype", "gpgfingerprint", + NULL }; if (opt.debug) @@ -1218,6 +1237,7 @@ ks_ldap_search (ctrl_t ctrl, parsed_uri_t uri, const char *pattern, add_to_strlist (&dupelist, certid[0]); count++; } + my_ldap_value_free (certid); } if (ldap_err == LDAP_SIZELIMIT_EXCEEDED) @@ -1247,18 +1267,26 @@ ks_ldap_search (ctrl_t ctrl, parsed_uri_t uri, const char *pattern, LDAPMessage *uids; certid = ldap_get_values (ldap_conn, each, "pgpcertid"); - if (! certid || ! certid[0]) - continue; + if (!certid || !certid[0]) + { + my_ldap_value_free (certid); + continue; + } /* Have we seen this certid before? */ if (! strlist_find (dupelist, certid[0])) { add_to_strlist (&dupelist, certid[0]); - es_fprintf (fp, "pub:%s:",certid[0]); + vals = ldap_get_values (ldap_conn, each, "gpgfingerprint"); + if (vals && vals[0] && vals[0][0]) + es_fprintf (fp, "pub:%s:", vals[0]); + else + es_fprintf (fp, "pub:%s:", certid[0]); + my_ldap_value_free (vals); vals = ldap_get_values (ldap_conn, each, "pgpkeytype"); - if (vals) + if (vals && vals[0]) { /* The LDAP server doesn't exactly handle this well. */ @@ -1266,60 +1294,60 @@ ks_ldap_search (ctrl_t ctrl, parsed_uri_t uri, const char *pattern, es_fputs ("1", fp); else if (strcasecmp (vals[0], "DSS/DH") == 0) es_fputs ("17", fp); - ldap_value_free (vals); } + my_ldap_value_free (vals); es_fputc (':', fp); vals = ldap_get_values (ldap_conn, each, "pgpkeysize"); - if (vals) + if (vals && vals[0]) { /* Not sure why, but some keys are listed with a key size of 0. Treat that like an unknown. */ if (atoi (vals[0]) > 0) es_fprintf (fp, "%d", atoi (vals[0])); - ldap_value_free (vals); } + my_ldap_value_free (vals); es_fputc (':', fp); /* YYYYMMDDHHmmssZ */ vals = ldap_get_values (ldap_conn, each, "pgpkeycreatetime"); - if(vals && strlen (vals[0]) == 15) + if(vals && vals[0] && strlen (vals[0]) == 15) { es_fprintf (fp, "%u", (unsigned int) ldap2epochtime(vals[0])); - ldap_value_free (vals); } + my_ldap_value_free (vals); es_fputc (':', fp); vals = ldap_get_values (ldap_conn, each, "pgpkeyexpiretime"); - if (vals && strlen (vals[0]) == 15) + if (vals && vals[0] && strlen (vals[0]) == 15) { es_fprintf (fp, "%u", (unsigned int) ldap2epochtime (vals[0])); - ldap_value_free (vals); } + my_ldap_value_free (vals); es_fputc (':', fp); vals = ldap_get_values (ldap_conn, each, "pgprevoked"); - if (vals) + if (vals && vals[0]) { if (atoi (vals[0]) == 1) es_fprintf (fp, "r"); - ldap_value_free (vals); } + my_ldap_value_free (vals); vals = ldap_get_values (ldap_conn, each, "pgpdisabled"); - if (vals) + if (vals && vals[0]) { if (atoi (vals[0]) ==1) es_fprintf (fp, "d"); - ldap_value_free (vals); } + my_ldap_value_free (vals); #if 0 /* This is not yet specified in the keyserver @@ -1327,12 +1355,12 @@ ks_ldap_search (ctrl_t ctrl, parsed_uri_t uri, const char *pattern, es_fputc (':', fp); vals = ldap_get_values (ldap_conn, each, "modifytimestamp"); - if(vals && strlen (vals[0]) == 15) + if(vals && vals[0] strlen (vals[0]) == 15) { es_fprintf (fp, "%u", (unsigned int) ldap2epochtime (vals[0])); - ldap_value_free (vals); } + my_ldap_value_free (vals); #endif es_fprintf (fp, "\n"); @@ -1343,10 +1371,13 @@ ks_ldap_search (ctrl_t ctrl, parsed_uri_t uri, const char *pattern, uids = ldap_next_entry (ldap_conn, uids)) { vals = ldap_get_values (ldap_conn, uids, "pgpcertid"); - if (! vals) - continue; + if (!vals || !vals[0]) + { + my_ldap_value_free (vals); + continue; + } - if (strcasecmp (certid[0], vals[0]) == 0) + if (!ascii_strcasecmp (certid[0], vals[0])) { char **uidvals; @@ -1356,12 +1387,14 @@ ks_ldap_search (ctrl_t ctrl, parsed_uri_t uri, const char *pattern, uids, "pgpuserid"); if (uidvals) { - /* Need to escape any colons */ - char *quoted = percent_escape (uidvals[0], NULL); - es_fputs (quoted, fp); + /* Need to percent escape any colons */ + char *quoted = try_percent_escape (uidvals[0], + NULL); + if (quoted) + es_fputs (quoted, fp); xfree (quoted); - ldap_value_free (uidvals); } + my_ldap_value_free (uidvals); es_fprintf (fp, "\n"); } @@ -1370,7 +1403,7 @@ ks_ldap_search (ctrl_t ctrl, parsed_uri_t uri, const char *pattern, } } - ldap_value_free (certid); + my_ldap_value_free (certid); } } @@ -1384,8 +1417,7 @@ ks_ldap_search (ctrl_t ctrl, parsed_uri_t uri, const char *pattern, out: if (err) { - if (fp) - es_fclose (fp); + es_fclose (fp); } else {