From: Jonathan S. Katz <jkatz@postgresql.org>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Michael Paquier <michael@paquier.xyz>
Cc: pgsql-bugs@lists.postgresql.org, Stephen Frost <sfrost@snowman.net>
Subject: Re: Possible to store invalid SCRAM-SHA-256 Passwords
Date: Mon, 22 Apr 2019 19:36:45 -0400
Message-ID: <15140775-08c3-45bd-6dab-fdcd290d1bf0@postgresql.org> (raw)
In-Reply-To: <30284.1555972921@sss.pgh.pa.us>
References: <016deb6b-1f0a-8e9f-1833-a8675b170aa9@postgresql.org>
<4240a41f-a957-fd7d-7c24-31683f259b9d@postgresql.org>
<20190422015051.GA3433@paquier.xyz>
<3033.1555940554@sss.pgh.pa.us>
<5a93333c-dde6-ee52-f870-4dccfc50eb6f@postgresql.org>
<30284.1555972921@sss.pgh.pa.us>
On 4/22/19 6:42 PM, Tom Lane wrote:
> "Jonathan S. Katz" <jkatz@postgresql.org> writes:
>> OK, so I have something that sort of works, i.e:
>
>> if (strncmp(shadow_pass, "md5", 3) == 0 &&
>> strlen(shadow_pass) == MD5_PASSWD_LEN &&
>> strspn(shadow_pass, MD5_PASSWD_CHARSET) == MD5_PASSWD_LEN
>> )
>
>> where MD5_PASSWD_CHARSET = "mabcdef0123456789"
>
>> ...but you may notice something: the CHARSET contains an "m" as we store
>> that "md5" prefix on the md5 hashed passwords.
>
> Yeah, that's silly; why not
>
> strspn(shadow_pass + 3, MD5_PASSWD_CHARSET) == MD5_PASSWD_LEN - 3
>
> It's not like this code isn't very well aware of the first 3 characters
> being not like the others.
I like that :) Please see attached patch, which is diff'd from the one
upthread.
I tested using the following:
/* Log in with "abc" */
CREATE ROLE test1 PASSWORD 'md5cdde562ece166a02f5392b656dcf2502' LOGIN;
/* Logs in with "md5cdde562ece166a02f5392b656dcf250g" */
CREATE ROLE test2 PASSWORD 'md5cdde562ece166a02f5392b656dcf250g' LOGIN;
/* Logs in with "md5cdde562ece166a02f5392b656dcf250m" */
CREATE ROLE test3 PASSWORD 'md5cdde562ece166a02f5392b656dcf250m' LOGIN;
I debated adding a test...without being able to simulate a log in, I
don't know if it tests much other than "yes, you can store an invalid
md5 hash and it treats it as plaintext."
Thanks,
Jonathan
From ab205e6fa82081d4b00fa8b67b0f33163f950cc4 Mon Sep 17 00:00:00 2001
From: "Jonathan S. Katz" <jonathan.katz@excoventures.com>
Date: Mon, 22 Apr 2019 19:27:15 -0400
Subject: [PATCH] Provide additional check to determine if a password is a
valid MD5 hash.
The present check in "get_password_type" only checks if a password contains
the "md5" prefix and is of length MD5_PASSWD_LEN. However, this mean that
there could be invalid md5 characters stored in the digest.
This fix explicitly checks to see if a password prefixed with "md5" actually
contains the valid set of characters that can be used in a MD5 hash.
---
src/backend/libpq/crypt.c | 3 ++-
src/include/common/md5.h | 1 +
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/backend/libpq/crypt.c b/src/backend/libpq/crypt.cindex db19f51c80..5617862243 100644--- a/src/backend/libpq/crypt.c+++ b/src/backend/libpq/crypt.c@@ -96,7 +96,8 @@ get_password_type(const char *shadow_pass)
uint8 stored_key[SCRAM_KEY_LEN];
uint8 server_key[SCRAM_KEY_LEN];
- if (strncmp(shadow_pass, "md5", 3) == 0 && strlen(shadow_pass) == MD5_PASSWD_LEN)+ if (strncmp(shadow_pass, "md5", 3) == 0 && strlen(shadow_pass) == MD5_PASSWD_LEN &&+ strspn(shadow_pass + 3, MD5_PASSWD_CHARSET) == MD5_PASSWD_LEN - 3)
return PASSWORD_TYPE_MD5;
if (strncmp(shadow_pass, "SCRAM-SHA-256$", strlen("SCRAM-SHA-256$")) == 0 &&
parse_scram_verifier(shadow_pass, &iterations, &encoded_salt, stored_key, server_key))
diff --git a/src/include/common/md5.h b/src/include/common/md5.hindex cd54c874b1..80b7475f40 100644--- a/src/include/common/md5.h+++ b/src/include/common/md5.h@@ -16,6 +16,7 @@
#ifndef PG_MD5_H
#define PG_MD5_H
+#define MD5_PASSWD_CHARSET "0123456789abcdef"
#define MD5_PASSWD_LEN 35
extern bool pg_md5_hash(const void *buff, size_t len, char *hexsum);
--
2.14.3 (Apple Git-98)
Attachments:
[text/plain] 0001-Provide-additional-check-to-determine-if-a-password-.patch (1.8K, ../15140775-08c3-45bd-6dab-fdcd290d1bf0@postgresql.org/2-0001-Provide-additional-check-to-determine-if-a-password-.patch)
download | inline diff:
From ab205e6fa82081d4b00fa8b67b0f33163f950cc4 Mon Sep 17 00:00:00 2001
From: "Jonathan S. Katz" <jonathan.katz@excoventures.com>
Date: Mon, 22 Apr 2019 19:27:15 -0400
Subject: [PATCH] Provide additional check to determine if a password is a
valid MD5 hash.
The present check in "get_password_type" only checks if a password contains
the "md5" prefix and is of length MD5_PASSWD_LEN. However, this mean that
there could be invalid md5 characters stored in the digest.
This fix explicitly checks to see if a password prefixed with "md5" actually
contains the valid set of characters that can be used in a MD5 hash.
---
src/backend/libpq/crypt.c | 3 ++-
src/include/common/md5.h | 1 +
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/backend/libpq/crypt.c b/src/backend/libpq/crypt.cindex db19f51c80..5617862243 100644--- a/src/backend/libpq/crypt.c+++ b/src/backend/libpq/crypt.c@@ -96,7 +96,8 @@ get_password_type(const char *shadow_pass)
uint8 stored_key[SCRAM_KEY_LEN];
uint8 server_key[SCRAM_KEY_LEN];
- if (strncmp(shadow_pass, "md5", 3) == 0 && strlen(shadow_pass) == MD5_PASSWD_LEN)+ if (strncmp(shadow_pass, "md5", 3) == 0 && strlen(shadow_pass) == MD5_PASSWD_LEN &&+ strspn(shadow_pass + 3, MD5_PASSWD_CHARSET) == MD5_PASSWD_LEN - 3)
return PASSWORD_TYPE_MD5;
if (strncmp(shadow_pass, "SCRAM-SHA-256$", strlen("SCRAM-SHA-256$")) == 0 &&
parse_scram_verifier(shadow_pass, &iterations, &encoded_salt, stored_key, server_key))
diff --git a/src/include/common/md5.h b/src/include/common/md5.hindex cd54c874b1..80b7475f40 100644--- a/src/include/common/md5.h+++ b/src/include/common/md5.h@@ -16,6 +16,7 @@
#ifndef PG_MD5_H
#define PG_MD5_H
+#define MD5_PASSWD_CHARSET "0123456789abcdef"
#define MD5_PASSWD_LEN 35
extern bool pg_md5_hash(const void *buff, size_t len, char *hexsum);
--
2.14.3 (Apple Git-98)
[application/pgp-signature] signature.asc (832B, ../15140775-08c3-45bd-6dab-fdcd290d1bf0@postgresql.org/3-signature.asc)
download
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-bugs@postgresql.org
Cc: jkatz@postgresql.org, tgl@sss.pgh.pa.us, michael@paquier.xyz, sfrost@snowman.net
Subject: Re: Possible to store invalid SCRAM-SHA-256 Passwords
In-Reply-To: <15140775-08c3-45bd-6dab-fdcd290d1bf0@postgresql.org>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox