Project

General

Profile

Actions
Copy link

Bug #3050

closed

mod_authn_dbi doesn't seem to compare passwords correctly

Added by herbmillerjr over 3 years ago. Updated over 3 years ago.

Status:
Invalid
Priority:
Normal
Category:
mod_auth
Target version:
-
ASK QUESTIONS IN Forums:
No

Description

I connected to a MariaDB database using mod_dbi and set user g2 and password g2 using SHA-256 (tried with MD5 as well). The error.log kept reporting 2020-12-25 17:52:57: (/tmp/lighttpd1.4/src/mod_auth.c.836) password doesn't match for / username: g2 IP: 127.0.0.1 even though I was entering the correct password. Digging around with strace and gdb showed the password making it though most of the code correctly until it's processed by http_auth_const_time_memeq (http_auth.c). The variables av and bv aren't even close to the same and one is longer than the other.

It seems to be the data being passed into the function that is the problem. Giving more space to shapw in mod_authn_dbi_password_cmp and artificially limiting the size passed into http_auth_const_time_memeq corrected the problem for me. It's now accepting the correct password and rejecting incorrect ones. I'm out of my element in plain C, so I'm sure the attached patch isn't the correct solution, but hopefully it helps reveal what the problem was. Also, I'm not sure why the username and realm are being passed into the context since we're only comparing passwords here.

Files

mod_auth_pass_cmp.patch (1.16 KB) mod_auth_pass_cmp.patch herbmillerjr, 2020-12-26 00:28
Actions
Copy link
 #1

Updated by gstrauss over 3 years ago

Thank you.

The binary password should be compared rather than the string, as otherwise the string needs to be normalized (uppercase vs lowercase hex) for comparison.

I'll review the code.

Actions
Copy link
 #2

Updated by gstrauss over 3 years ago

What value is stored in the database? The database should contain the SHA256 of "username:realm:g2" or the MD5 of "username:realm:g2". The database should not contain "g2".

[Edit] to clarify, the value returned from the database should be the hash value.
See mod_authn_dbi doc

Please note:
RFC 7616 support was added in lighttpd 1.4.54, but among popular browsers, only Opera currently supports algorithm=SHA-256
https://redmine.lighttpd.net/boards/2/topics/8903
https://redmine.lighttpd.net/boards/2/topics/8955

Actions
Copy link
 #3

Updated by gstrauss over 3 years ago

  • Status changed from New to Invalid
  • Target version deleted (1.4.x)

Based on your comment about not knowing how the username and realm are used, I would suggest you also read up on RFC 7616

The view or query from the database should return the equivalent of MD5(CONCAT(username,':',realm,':',plaintext_password))

Actions
Copy link
 #4

Updated by gstrauss over 3 years ago

... I am still reviewing the code, as the pattern differs from the deprecated mod_authn_mysql.

Actions
Copy link
 #5

Updated by herbmillerjr over 3 years ago

gstrauss wrote in #note-4:

... I am still reviewing the code, as the pattern differs from the deprecated mod_authn_mysql.

Thanks! And thank you for the RFC link. I missed that when I was looking through the docs. I am storing the hash in the database.

Actions
Copy link
 #6

Updated by gstrauss over 3 years ago

herbmillerjr are you using Basic Auth or Digest Auth? Please share your auth.* config settings, but please omit "username" and "password"

Actions
Copy link
 #7

Updated by gstrauss over 3 years ago

mod_authn_dbi_password_cmp() is called in the case of Basic Auth, so I presume you are using that.

The hash value stored in the database is intentionally more than simply the hash of the password so that the "username:realm:" act as a salt. Without that, if the database were compromised, you would be able to see different users with the same password since the hash would be the same if the password was the same.

Actions
Copy link
 #8

Updated by herbmillerjr over 3 years ago 

auth.backend = "dbi" 
auth.backend.dbi += (
      "sql"           => "SELECT passwd FROM users WHERE user='?' AND realm='?'",
      "dbtype"        => "mysql",
      "dbname"        => "lighttpd",
)

auth.require = (
    "/" => (
        "method"    => "basic",
        "realm"     => "g2",
        "algorithm" => "SHA-256",
        "require"   => "user=g2",

    )
)

You got it. Yep, it's basic.

That was indeed the problem. I wasn't storing the password correctly (with the salt).

Actions
Copy link
 #9

Updated by gstrauss over 3 years ago

I updated the documentation for mod_authn_mysql to call out this difference from the improved mod_authn_dbi. (The documentation for mod_authn_dbi already specifies the required format of the data coming from the database.)

Actions
Copy link

Also available in: Atom