Fix client certificate login #16

Merged
jandd merged 4 commits from fix-client-certificate-login into main 1 month ago
jandd commented 2 months ago
Owner

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.

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.
jandd added 1 commit 2 months ago
560be526c4 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.
jandd requested review from dirk 2 months ago
jandd requested review from knilsson 2 months ago
jandd requested review from ted 2 months ago
knilsson reviewed 2 months ago
knilsson left a comment

Logically, the suggested changes seem to do what is said.
Applauds on the very descriptive PR! Made me feel I could follow both the intent and the actual suggested code and compare with the old.

Sadly, I don't know PHP, so I can't confirm that a change this big will do what is expected.

I assume both Dirk and Ted will easily confirm if that is the case.

Perhaps Brian will also be able to review soon.

Logically, the suggested changes seem to do what is said. Applauds on the very descriptive PR! Made me feel I could follow both the intent and the actual suggested code and compare with the old. Sadly, I don't know PHP, so I can't confirm that a change this big will do what is expected. I assume both Dirk and Ted will easily confirm if that is the case. Perhaps Brian will also be able to review soon.
jandd added 1 commit 2 months ago
jandd added 1 commit 2 months ago
jandd reviewed 2 months ago
@ -19,0 +30,4 @@
// try SAN email addresses first
$envNameBase = "SSL_CLIENT_SAN_Email";
for ($i = 0; $i <= $maxAddresses; $i++) {
jandd commented 2 months ago
Poster
Owner

SSL_Client_SAN_Email_0 is the first item, that was the reason for 9626e7f

SSL_Client_SAN_Email_0 is the first item, that was the reason for 9626e7f
jandd marked this conversation as resolved
bmc approved these changes 1 month ago
dirk approved these changes 1 month ago
dirk left a comment
Owner

please verify using webdb2-database before installing on webdb1

please verify using webdb2-database before installing on webdb1
jandd added 1 commit 1 month ago
jandd merged commit abfce60ed4 into main 1 month ago
jandd deleted branch fix-client-certificate-login 1 month ago

Reviewers

knilsson was requested for review 2 months ago
ted was requested for review 2 months ago
bmc approved these changes 1 month ago
dirk approved these changes 1 month ago
The pull request has been merged as abfce60ed4.
Sign in to join this conversation.
No Milestone
No project
No Assignees
4 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: cacert/cacert-webdb#16
Loading…
There is no content yet.