cacert/cacert-webdb
9
0
Fork 0
Code Issues Pull requests 3 Projects Releases 4 Wiki Activity

Fix client certificate login #16

Merged
jandd merged 4 commits from fix-client-certificate-login into main 2024-05-20 10:35:31 +00:00
Conversation 0 Commits 4 Files changed 3 +78 -19
jandd commented 2024-05-05 18:09:36 +00:00
Owner
Copy link

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 2024-05-05 18:09:37 +00:00 
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 2024-05-05 18:09:48 +00:00
jandd requested review from knilsson 2024-05-05 18:09:48 +00:00
jandd requested review from ted 2024-05-05 18:09:48 +00:00
knilsson reviewed 2024-05-05 19:19:08 +00:00
knilsson left a comment
Member
Copy link

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 2024-05-05 19:23:04 +00:00
jandd added 1 commit 2024-05-05 19:32:46 +00:00
jandd reviewed 2024-05-05 19:33:29 +00:00
includes/lib/general.php
@ -19,0 +30,4 @@
// try SAN email addresses first
$envNameBase = "SSL_CLIENT_SAN_Email";
for ($i = 0; $i <= $maxAddresses; $i++) {
jandd commented 2024-05-05 19:33:29 +00:00
Author
Owner
Copy link

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 2024-05-09 12:48:34 +00:00
dirk approved these changes 2024-05-20 10:11:27 +00:00
dirk left a comment
Owner
Copy link

please verify using webdb2-database before installing on webdb1

please verify using webdb2-database before installing on webdb1
jandd added 1 commit 2024-05-20 10:35:20 +00:00
jandd merged commit abfce60ed4 into main 2024-05-20 10:35:31 +00:00
jandd deleted branch fix-client-certificate-login 2024-05-20 10:35:32 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
knilsson
ted
bmc
dirk
Labels
Clear labels
  
bug

Something is not working

  
duplicate

This issue or pull request already exists

  
enhancement

New feature

  
help wanted

Need some help

  
invalid

Something is wrong

  
question

More information is needed

  
wontfix

This won't be fixed

No labels
bug
duplicate
enhancement
help wanted
invalid
question
wontfix
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
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
No description provided.
Delete branch "fix-client-certificate-login"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?