8000 Refactor the code for verifying user's password. · postgrespro/postgres@e7f051b · GitHub
[go: up one dir, main page]
Skip to content

    • Commit e7f051b

    Browse files
    hlinnakahlinnaka
    committed
    Refactor the code for verifying user's password.
    Split md5_crypt_verify() into three functions: * get_role_password() to fetch user's password from pg_authid, and check its expiration. * md5_crypt_verify() to check an MD5 authentication challenge * plain_crypt_verify() to check a plaintext password. get_role_password() will be needed as a separate function by the upcoming SCRAM authentication patch set. Most of the remaining functionality in md5_crypt_verify() was different for MD5 and plaintext authentication, so split that for readability. While we're at it, simplify the *_crypt_verify functions by using stack-allocated buffers to hold the temporary MD5 hashes, instead of pallocing. Reviewed by Michael Paquier. Discussion: https://www.postgresql.org/message-id/3029e460-d47c-710e-507e-d8ba759d7cbb@iki.fi
    1 parent 58445c5 commit e7f051b

    File tree

    3 files changed

    +153
    -91
    lines changed

    3 files changed

    +153
    -91
    lines changed

    ‎src/backend/libpq/auth.c

    Lines changed: 14 additions & 4 deletions
    Original file line numberDiff line numberDiff line change
    @@ -704,6 +704,7 @@ CheckMD5Auth(Port *port, char **logdetail)
    704704
    {
    705705
    char md5Salt[4]; /* Password salt */
    706706
    char *passwd;
    707+
    char *shadow_pass;
    707708
    int result;
    708709

    709710
    if (Db_user_namespace)
    @@ -722,12 +723,16 @@ CheckMD5Auth(Port *port, char **logdetail)
    722723
    sendAuthRequest(port, AUTH_REQ_MD5, md5Salt, 4);
    723724

    724725
    passwd = recv_password_packet(port);
    725-
    726726
    if (passwd == NULL)
    727727
    return STATUS_EOF; /* client wouldn't send password */
    728728

    729-
    result = md5_crypt_verify(port->user_name, passwd, md5Salt, 4, logdetail);
    729+
    result = get_role_password(port->user_name, &shadow_pass, logdetail);
    730+
    if (result == STATUS_OK)
    731+
    result = md5_crypt_verify(port->user_name, shadow_pass, passwd,
    732+
    md5Salt, 4, logdetail);
    730733

    734+
    if (shadow_pass)
    735+
    pfree(shadow_pass);
    731736
    pfree(passwd);
    732737

    733738
    return result;
    @@ -743,16 +748,21 @@ CheckPasswordAuth(Port *port, char **logdetail)
    743748
    {
    744749
    char *passwd;
    745750
    int result;
    751+
    char *shadow_pass;
    746752

    747753
    sendAuthRequest(port, AUTH_REQ_PASSWORD, NULL, 0);
    748754

    749755
    passwd = recv_password_packet(port);
    750-
    751756
    if (passwd == NULL)
    752757
    return STATUS_EOF; /* client wouldn't send password */
    753758

    754-
    result = md5_crypt_verify(port->user_name, passwd, NULL, 0, logdetail);
    759+
    result = get_role_password(port->user_name, &shadow_pass, logdetail);
    760+
    if (result == STATUS_OK)
    761+
    result = plain_crypt_verify(port->user_name, shadow_pass, passwd,
    762+
    logdetail);
    755763

    764+
    if (shadow_pass)
    765+
    pfree(shadow_pass);
    756766
    pfree(passwd);
    757767

    758768
    return result;

    ‎src/backend/libpq/crypt.c

    Lines changed: 132 additions & 85 deletions
    Original file line numberDiff line numberDiff line change
    @@ -30,28 +30,28 @@
    3030

    3131

    3232
    /*
    33-
    * Check given password for given user, and return STATUS_OK or STATUS_ERROR.
    33+
    * Fetch stored password for a user, for authentication.
    3434
    *
    35-
    * 'client_pass' is the password response given by the remote user. If
    36-
    * 'md5_salt' is not NULL, it is a response to an MD5 authentication
    37-
    * challenge, with the given salt. Otherwise, it is a plaintext password.
    35+
    * Returns STATUS_OK on success. On error, returns STATUS_ERROR, and stores
    36+
    * a palloc'd string describing the reason, for the postmaster log, in
    37+
    * *logdetail. The error reason should *not* be sent to the client, to avoid
    38+
    * giving away user information!
    3839
    *
    39-
    * In the error case, optionally store a palloc'd string at *logdetail
    40-
    * that will be sent to the postmaster log (but not the client).
    40+
    * If the password is expired, it is still returned in *shadow_pass, but the
    41+
    * return code is STATUS_ERROR. On other errors, *shadow_pass is set to
    42+
    * NULL.
    4143
    */
    4244
    int
    43-
    md5_crypt_verify(const char *role, char *client_pass,
    44-
    char *md5_salt, int md5_salt_len, char **logdetail)
    45+
    get_role_password(const char *role, char **shadow_pass, char **logdetail)
    4546
    {
    4647
    int retval = STATUS_ERROR;
    47-
    char *shadow_pass,
    48-
    *crypt_pwd;
    4948
    TimestampTz vuntil = 0;
    50-
    char *crypt_client_pass = client_pass;
    5149
    HeapTuple roleTup;
    5250
    Datum datum;
    5351
    bool isnull;
    5452

    53+
    *shadow_pass = NULL;
    54+
    5555
    /* Get role info from pg_authid */
    5656
    roleTup = SearchSysCache1(AUTHNAME, PointerGetDatum(role));
    5757
    if (!HeapTupleIsValid(roleTup))
    @@ -70,7 +70,7 @@ md5_crypt_verify(const char *role, char *client_pass,
    7070
    role);
    7171
    return STATUS_ERROR; /* user has no password */
    7272
    }
    73-
    shadow_pass = TextDatumGetCString(datum);
    73+
    *shadow_pass = TextDatumGetCString(datum);
    7474

    7575
    datum = SysCacheGetAttr(AUTHNAME, roleTup,
    7676
    Anum_pg_authid_rolvaliduntil, &isnull);
    @@ -79,104 +79,151 @@ md5_crypt_verify(const char *role, char *client_pass,
    7979

    8080
    ReleaseSysCache(roleTup);
    8181

    82-
    if (*shadow_pass == '\0')
    82+
    if (**shadow_pass == '\0')
    8383
    {
    8484
    *logdetail = psprintf(_("User \"%s\" has an empty password."),
    8585
    role);
    86+
    pfree(*shadow_pass);
    87+
    *shadow_pass = NULL;
    8688
    return STATUS_ERROR; /* empty password */
    8789
    }
    8890

    8991
    /*
    90-
    * Compare with the encrypted or plain password depending on the
    91-
    * authentication method being used for this connection. (We do not
    92-
    * bother setting logdetail for pg_md5_encrypt failure: the only possible
    93-
    * error is out-of-memory, which is unlikely, and if it did happen adding
    94-
    * a psprintf call would only make things worse.)
    92+
    * Password OK, now check to be sure we are not past rolvaliduntil
    9593
    */
    96-
    if (md5_salt)
    94+
    if (isnull)
    95+
    retval = STATUS_OK;
    96+
    else if (vuntil < GetCurrentTimestamp())
    9797
    {
    98-
    /* MD5 authentication */
    99-
    Assert(md5_salt_len > 0);
    100-
    crypt_pwd = palloc(MD5_PASSWD_LEN + 1);
    101-
    if (isMD5(shadow_pass))
    102-
    {
    103-
    /* stored password already encrypted, only do salt */
    104-
    if (!pg_md5_encrypt(shadow_pass + strlen("md5"),
    105-
    md5_salt, md5_salt_len,
    106-
    crypt_pwd))
    107-
    {
    108-
    pfree(crypt_pwd);
    109-
    return STATUS_ERROR;
    110-
    }
    111-
    }
    112-
    else
    98+
    *logdetail = psprintf(_("User \"%s\" has an expired password."),
    99+
    role);
    100+
    retval = STATUS_ERROR;
    101+
    }
    102+
    else
    103+
    retval = STATUS_OK;
    104+
    105+
    return retval;
    106+
    }
    107+
    < 12AE /td>108+
    /*
    109+
    * Check MD5 authentication response, and return STATUS_OK or STATUS_ERROR.
    110+
    *
    111+
    * 'shadow_pass' is the user's correct password or password hash, as stored
    112+
    * in pg_authid.rolpassword.
    113+
    * 'client_pass' is the response given by the remote user to the MD5 challenge.
    114+
    * 'md5_salt' is the salt used in the MD5 authentication challenge.
    115+
    *
    116+
    * In the error case, optionally store a palloc'd string at *logdetail
    117+
    * that will be sent to the postmaster log (but not the client).
    118+
    */
    119+
    int
    120+
    md5_crypt_verify(const char *role, const char *shadow_pass,
    121+
    const char *client_pass,
    122+
    const char *md5_salt, int md5_salt_len,
    123+
    char **logdetail)
    124+
    {
    125+
    int retval;
    126+
    char crypt_pwd[MD5_PASSWD_LEN + 1];
    127+
    char crypt_pwd2[MD5_PASSWD_LEN + 1];
    128+
    129+
    Assert(md5_salt_len > 0);
    130+
    131+
    /*
    132+
    * Compute the correct answer for the MD5 challenge.
    133+
    *
    134+
    * We do not bother setting logdetail for any pg_md5_encrypt failure
    135+
    * below: the only possible error is out-of-memory, which is unlikely, and
    136+
    * if it did happen adding a psprintf call would only make things worse.
    137+
    */
    138+
    if (isMD5(shadow_pass))
    139+
    {
    140+
    /* stored password already encrypted, only do salt */
    141+
    if (!pg_md5_encrypt(shadow_pass + strlen("md5"),
    142+
    md5_salt, md5_salt_len,
    143+
    crypt_pwd))
    113144
    {
    114-
    /* stored password is plain, double-encrypt */
    115-
    char *crypt_pwd2 = palloc(MD5_PASSWD_LEN + 1);
    116-
    117-
    if (!pg_md5_encrypt(shadow_pass,
    118-
    role,
    119-
    strlen(role),
    120-
    crypt_pwd2))
    121-
    {
    122-
    pfree(crypt_pwd);
    123-
    pfree(crypt_pwd2);
    < F438 /td>
    124-
    return STATUS_ERROR;
    125-
    }
    126-
    if (!pg_md5_encrypt(crypt_pwd2 + strlen("md5"),
    127-
    md5_salt, md5_salt_len,
    128-
    crypt_pwd))
    129-
    {
    130-
    pfree(crypt_pwd);
    131-
    pfree(crypt_pwd2);
    132-
    return STATUS_ERROR;
    133-
    }
    134-
    pfree(crypt_pwd2);
    145+
    return STATUS_ERROR;
    135146
    }
    136147
    }
    137148
    else
    138149
    {
    139-
    /* Client sent password in plaintext */
    140-
    if (isMD5(shadow_pass))
    150+
    /* stored password is plain, double-encrypt */
    151+
    if (!pg_md5_encrypt(shadow_pass,
    152+
    role,
    153+
    strlen(role),
    154+
    crypt_pwd2))
    141155
    {
    142-
    /* Encrypt user-supplied password to match stored MD5 */
    143-
    crypt_client_pass = palloc(MD5_PASSWD_LEN + 1);
    144-
    if (!pg_md5_encrypt(client_pass,
    145-
    role,
    146-
    strlen(role),
    147-
    crypt_client_pass))
    148-
    {
    149-
    pfree(crypt_client_pass);
    150-
    return STATUS_ERROR;
    151-
    }
    156+
    return STATUS_ERROR;
    157+
    }
    158+
    if (!pg_md5_encrypt(crypt_pwd2 + strlen("md5"),
    159+
    md5_salt, md5_salt_len,
    160+
    crypt_pwd))
    161+
    {
    162+
    return STATUS_ERROR;
    152163
    }
    153-
    crypt_pwd = shadow_pass;
    154164
    }
    155165

    156-
    if (strcmp(crypt_client_pass, crypt_pwd) == 0)
    166+
    if (strcmp(client_pass, crypt_pwd) == 0)
    167+
    retval = STATUS_OK;
    168+
    else
    157169
    {
    158-
    /*
    159-
    * Password OK, now check to be sure we are not past rolvaliduntil
    160-
    */
    161-
    if (isnull)
    162-
    retval = STATUS_OK;
    163-
    else if (vuntil < GetCurrentTimestamp())
    170+
    *logdetail = psprintf(_("Password does not match for user \"%s\"."),
    171+
    role);
    172+
    retval = STATUS_ERROR;
    173+
    }
    174+
    175+
    return retval;
    176+
    }
    177+
    178+
    /*
    179+
    * Check given password for given user, and return STATUS_OK or STATUS_ERROR.
    180+
    *
    181+
    * 'shadow_pass' is the user's correct password or password hash, as stored
    182+
    * in pg_authid.rolpassword.
    183+
    * 'client_pass' is the password given by the remote user.
    184+
    *
    185+
    * In the error case, optionally store a palloc'd string at *logdetail
    186+
    * that will be sent to the postmaster log (but not the client).
    187+
    */
    188+
    int
    189+
    plain_crypt_verify(const char *role, const char *shadow_pass,
    190+
    const char *client_pass,
    191+
    char **logdetail)
    192+
    {
    193+
    int retval;
    194+
    char crypt_client_pass[MD5_PASSWD_LEN + 1];
    195+
    196+
    /*
    197+
    * Client sent password in plaintext. If we have an MD5 hash stored, hash
    198+
    * the password the client sent, and compare the hashes. Otherwise
    199+
    * compare the plaintext passwords directly.
    200+
    */
    201+
    if (isMD5(shadow_pass))
    202+
    {
    203+
    if (!pg_md5_encrypt(client_pass,
    204+
    role,
    205+
    strlen(role),
    206+
    crypt_client_pass))
    164207
    {
    165-
    *logdetail = psprintf(_("User \"%s\" has an expired password."),
    166-
    role);
    167-
    retval = STATUS_ERROR;
    208+
    /*
    209+
    * We do not bother setting logdetail for pg_md5_encrypt failure:
    210+
    * the only possible error is out-of-memory, which is unlikely,
    211+
    * and if it did happen adding a psprintf call would only make
    212+
    * things worse.
    213+
    */
    214+
    return STATUS_ERROR;
    168215
    }
    169-
    else
    170-
    retval = STATUS_OK;
    216+
    client_pass = crypt_client_pass;
    171217
    }
    218+
    219+
    if (strcmp(client_pass, shadow_pass) == 0)
    220+
    retval = STATUS_OK;
    172221
    else
    222+
    {
    173223
    *logdetail = psprintf(_("Password does not match for user \"%s\"."),
    174224
    role);
    175-
    176-
    if (crypt_pwd != shadow_pass)
    177-
    pfree(crypt_pwd);
    178-
    if (crypt_client_pass != client_pass)
    179-
    pfree(crypt_client_pass);
    225+
    retval = STATUS_ERROR;
    226+
    }
    180227

    181228
    return retval;
    182229
    }

    ‎src/include/libpq/crypt.h

    Lines changed: 7 additions & 2 deletions
    Original file line numberDiff line numberDiff line change
    @@ -15,7 +15,12 @@
    1515

    1616
    #include "datatype/timestamp.h"
    1717

    18< 707F code class="diff-text syntax-highlighted-line deletion">-
    extern int md5_crypt_verify(const char *role, char *client_pass,
    19-
    char *md5_salt, int md5_salt_len, char **logdetail);
    18+
    extern int get_role_password(const char *role, char **shadow_pass, char **logdetail);
    19+
    20+
    extern int md5_crypt_verify(const char *role, const char *shadow_pass,
    21+
    const char *client_pass, const char *md5_salt,
    22+
    int md5_salt_len, char **logdetail);
    23+
    extern int plain_crypt_verify(const char *role, const char *shadow_pass,
    24+
    const char *client_pass, char **logdetail);
    2025

    2126
    #endif

    0 commit comments

    Comments
     (0)
    0