From 560be526c44aceef850de8ec6f84bb1a6a6d6e23 Mon Sep 17 00:00:00 2001 From: Jan Dittberner Date: Sun, 5 May 2024 19:59:53 +0200 Subject: [PATCH 1/3] Fix client certificate login This change fixes the client certificate login for cases where duplicate serial numbers have been issued and recorded in the emailcerts table. Email addresses from the client certificate are used as an additional matching parameter. - includes/lib/general.php got a new function get_email_addresses_from_client_cert to create an array of email addresses from the environment variables set by Apache httpd - includes/loggedin.php and www/index.php use the new function to pass email addresses to the get_user_id_from_cert function - get_user_id_from_cert in includes/lib/general.php has been enhanced to use a JOIN over the emailcerts, root_certs and email tables. All parameters are escaped via mysql_real_escape_string - SQL errors in get_user_id_from_cert are now handled - a match from get_user_id_from_cert is only returned when there is exactly one row in the result set The code and the used query have been tested with Apache 2.4.10 and PHP 5.6 from Debian Jessie and a MariaDB 10.11 in strict mode using a container based test setup to match the current production setup as close as possible. --- includes/lib/general.php | 93 ++++++++++++++++++++++++++++++++-------- includes/loggedin.php | 2 +- www/index.php | 2 +- 3 files changed, 78 insertions(+), 19 deletions(-) diff --git a/includes/lib/general.php b/includes/lib/general.php index 127c6b7..6afc226 100644 --- a/includes/lib/general.php +++ b/includes/lib/general.php @@ -1,6 +1,6 @@ 0) { + return array_unique($addresses); + } + + // fallback for older Apache httpd versions that do not support email SAN fields + $envNameBase = "SSL_CLIENT_S_DN_Email"; + + if (array_key_exists($envNameBase, $_SERVER)) { + $addresses[] = $_SERVER[$envNameBase]; + } + + for ($i = 1; $i <= $maxAddresses; $i++) { + $envName = sprintf("%s_%d", $envNameBase, $i); + if (array_key_exists($envName, $_SERVER)) { + $addresses[] = $_SERVER[$envName]; + } + } + + return array_unique($addresses); +} + /** * Checks if the user may log in and retrieve the user id * * Usually called with $_SERVER['SSL_CLIENT_M_SERIAL'] and - * $_SERVER['SSL_CLIENT_I_DN_CN'] + * $_SERVER['SSL_CLIENT_I_DN_CN'] * * @param $serial string - * usually $_SERVER['SSL_CLIENT_M_SERIAL'] + * usually $_SERVER['SSL_CLIENT_M_SERIAL'] * @param $issuer_cn string - * usually $_SERVER['SSL_CLIENT_I_DN_CN'] + * usually $_SERVER['SSL_CLIENT_I_DN_CN'] + * @param $addresses array + * list of email addresses from the certificate * @return int - * the user id, -1 in case of error + * the user id, -1 in case of error + * + * @see get_email_addresses_from_client_cert() */ -function get_user_id_from_cert($serial, $issuer_cn) -{ - $query = "select `memid` from `emailcerts` where - `serial`='".mysql_escape_string($serial)."' and - `rootcert`= (select `id` from `root_certs` where - `Cert_Text`='".mysql_escape_string($issuer_cn)."') and - `revoked`=0 and disablelogin=0 and - UNIX_TIMESTAMP(`expire`) - UNIX_TIMESTAMP() > 0"; +function get_user_id_from_cert($serial, $issuer_cn, $addresses) { + $addresses_for_sql = array_map('mysql_real_escape_string', $addresses); + + $query = sprintf("SELECT DISTINCT ec.`memid` +FROM `emailcerts` ec + JOIN root_certs r ON r.id = ec.rootcert + JOIN email e ON ec.memid = e.memid +WHERE ec.serial = '%s' + AND r.`Cert_Text` = '%s' + AND e.email IN ('%s') + AND ec.revoked = 0 + AND ec.disablelogin = 0 + AND UNIX_TIMESTAMP(ec.expire) > UNIX_TIMESTAMP()", mysql_real_escape_string($serial), + mysql_real_escape_string($issuer_cn), implode("', '", $addresses_for_sql)); + header("Content-type: text/plain"); $res = mysql_query($query); - if(mysql_num_rows($res) > 0) - { - $row = mysql_fetch_assoc($res); - return intval($row['memid']); + if ($res === false) { + trigger_error(sprintf("MySQL error %d: %s", mysql_errno(), mysql_error())); + + return -1; + } + + if (mysql_num_rows($res) === 1) { + $row = mysql_fetch_row($res); + return intval($row[0]); } return -1; diff --git a/includes/loggedin.php b/includes/loggedin.php index c14f8c2..30c6b55 100644 --- a/includes/loggedin.php +++ b/includes/loggedin.php @@ -54,7 +54,7 @@ if($_SERVER['HTTP_HOST'] == $_SESSION['_config']['securehostname'] && ($_SESSION['profile']['id'] == 0 || $_SESSION['profile']['loggedin'] == 0)) { $user_id = get_user_id_from_cert($_SERVER['SSL_CLIENT_M_SERIAL'], - $_SERVER['SSL_CLIENT_I_DN_CN']); + $_SERVER['SSL_CLIENT_I_DN_CN'], get_email_addresses_from_client_cert()); if($user_id >= 0) { diff --git a/www/index.php b/www/index.php index 8c5560c..fed651b 100644 --- a/www/index.php +++ b/www/index.php @@ -153,7 +153,7 @@ require_once('../includes/notary.inc.php'); { include_once("../includes/lib/general.php"); $user_id = get_user_id_from_cert($_SERVER['SSL_CLIENT_M_SERIAL'], - $_SERVER['SSL_CLIENT_I_DN_CN']); + $_SERVER['SSL_CLIENT_I_DN_CN'], get_email_addresses_from_client_cert()); if($user_id >= 0) { From 5f89d4803602939e2b860e24f96ba58b9fb2b830 Mon Sep 17 00:00:00 2001 From: Jan Dittberner Date: Sun, 5 May 2024 21:22:16 +0200 Subject: [PATCH 2/3] Remove leftover header call --- includes/lib/general.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/includes/lib/general.php b/includes/lib/general.php index 6afc226..cb5f257 100644 --- a/includes/lib/general.php +++ b/includes/lib/general.php @@ -90,7 +90,7 @@ WHERE ec.serial = '%s' AND ec.disablelogin = 0 AND UNIX_TIMESTAMP(ec.expire) > UNIX_TIMESTAMP()", mysql_real_escape_string($serial), mysql_real_escape_string($issuer_cn), implode("', '", $addresses_for_sql)); - header("Content-type: text/plain"); + $res = mysql_query($query); if ($res === false) { trigger_error(sprintf("MySQL error %d: %s", mysql_errno(), mysql_error())); From 9626e7f6fc36aea32f2ed2eb0aec915264bdbd8c Mon Sep 17 00:00:00 2001 From: Jan Dittberner Date: Sun, 5 May 2024 21:32:20 +0200 Subject: [PATCH 3/3] Fix initial index for email SAN lookup --- includes/lib/general.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/includes/lib/general.php b/includes/lib/general.php index cb5f257..e7e54db 100644 --- a/includes/lib/general.php +++ b/includes/lib/general.php @@ -30,7 +30,7 @@ function get_email_addresses_from_client_cert() { // try SAN email addresses first $envNameBase = "SSL_CLIENT_SAN_Email"; - for ($i = 1; $i <= $maxAddresses; $i++) { + for ($i = 0; $i <= $maxAddresses; $i++) { $envName = sprintf("%s_%d", $envNameBase, $i); if (!array_key_exists($envName, $_SERVER)) { break;