Re: chkpass Major Issue - compares 'contains' and not 'equal' - Mailing list pgsql-bugs

From D'Arcy Cain
Subject Re: chkpass Major Issue - compares 'contains' and not 'equal'
Date
Msg-id a70d54d1-7103-d6d4-983d-b603de3869b3@druid.net
Whole thread Raw
In response to Re: chkpass Major Issue - compares 'contains' and not 'equal'  ("David G. Johnston" <david.g.johnston@gmail.com>)
List pgsql-bugs
On 2018-06-07 10:09 AM, David G. Johnston wrote:
> It is also a documented limitation.
> 
> The encryption uses the standard Unix function |crypt()|, and so it
> suffers from all the usual limitations of that function; notably that
> only the first eight characters of a password are considered.
> 
> https://www.postgresql.org/docs/10/static/chkpass.html
> 
> At this point I'd consider its presence here for backward compatibility
> only and as such the behavior is not something that is likely to be changed.

I have a new version that does MD5 but I haven't had time to work out a
patch yet.  Here it is if someone wants to update the in-tree version.
/*
 * PostgreSQL type definitions for chkpass
 * Written by D'Arcy J.M. Cain
 * darcy@druid.net
 * http://www.druid.net/darcy/
 *
 * $Id: chkpass.c 8899 2015-01-01 22:51:54Z darcy $
 * best viewed with tabs set to 4
 */

#include "postgres.h"

#include <time.h>
#include <unistd.h>
#ifdef HAVE_CRYPT_H
#include <crypt.h>
#endif

#include "fmgr.h"
#include "utils/guc.h"

PG_MODULE_MAGIC;

void _PG_init(void);
extern text *cstring_to_text(const char *s);

/*
 * This type encrypts its input unless the first character is a colon.
 * The output is the encrypted form with a leading colon.  The output
 * format is designed to allow dump and reload operations to work as
 * expected without doing special tricks.
 */


/*
 * CHKPASS is now a variable-width type. The format for an md5-format
 * password is $1$<salt>$<hash> where the hash is always 22 characters
 * when base-64 encoded. We allocate space for that, plus a leading :
 * plus a trailing null.
 */

typedef struct varlena chkpass;
#define SALTLEN 8
#define CHKPASSLEN (VARHDRSZ + SALTLEN + 6 + 22)

/* controlled by the GUC chkpass.use_md5 */
static bool use_md5 = false;

/*
 * Various forward declarations:
 */

Datum        chkpass_in(PG_FUNCTION_ARGS);
Datum        chkpass_out(PG_FUNCTION_ARGS);
Datum        chkpass_rout(PG_FUNCTION_ARGS);

/* Only equal or not equal make sense */
Datum        chkpass_eq(PG_FUNCTION_ARGS);
Datum        chkpass_ne(PG_FUNCTION_ARGS);


/* This function checks that the password is a good one
 * It's just a placeholder for now */
static int
verify_pass(const char *str)
{
    return 0;
}

/*
 * CHKPASS reader.
 */
PG_FUNCTION_INFO_V1(chkpass_in);
Datum
chkpass_in(PG_FUNCTION_ARGS)
{
    char       *str = PG_GETARG_CSTRING(0);
    chkpass    *result;
    char        mysalt[SALTLEN+6];
    int         i;
    static char salt_chars[] =
    "./0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz";

    result = (chkpass *) palloc(CHKPASSLEN);
    /* special case to let us enter encrypted passwords */
    if (*str == ':')
    {
        SET_VARSIZE(result, VARHDRSZ + strlen(str)-1);
        memcpy(VARDATA(result), str+1, VARSIZE(result)-VARHDRSZ);
        PG_RETURN_POINTER(result);
    }

    if (verify_pass(str) != 0)
        ereport(ERROR,
            (errcode(ERRCODE_DATA_EXCEPTION),
            errmsg("password \"%s\" is weak", str)));

    if (use_md5)
    {
        memcpy(mysalt, "$1$", 3);
        for (i = 3; i < (3+SALTLEN); i++)
            mysalt[i] = salt_chars[random() & 0x3f];
        mysalt[3+SALTLEN] = '$';
        mysalt[4+SALTLEN] = 0;
    }
    else
    {
        mysalt[0] = salt_chars[random() & 0x3f];
        mysalt[1] = salt_chars[random() & 0x3f];
        mysalt[2] = 0;    /* technically the terminator is not necessary
                           * but I like to play safe */
    }
    strcpy(result->vl_dat, crypt(str, mysalt));
    SET_VARSIZE(result, VARHDRSZ + strlen(result->vl_dat));
    PG_RETURN_POINTER(result);
}

/*
 * CHKPASS output function.
 */

/* common output code */
static char *
out(chkpass *password)
{
    char       *str;
    size_t      sz;

    sz = VARSIZE(password) - VARHDRSZ;
    str = palloc(sz + 1);
    memcpy(str, VARDATA(password), sz);
    str[sz] = 0;
    return str;
}

PG_FUNCTION_INFO_V1(chkpass_out);
Datum
chkpass_out(PG_FUNCTION_ARGS)
{
    char       *str, *result;

    str = out((chkpass *) PG_GETARG_POINTER(0));
    result = (char *) palloc(strlen(str) + 2);
    result[0] = ':';
    strcpy(result + 1, str);

    PG_RETURN_CSTRING(result);
}

PG_FUNCTION_INFO_V1(chkpass_rout);
Datum
chkpass_rout(PG_FUNCTION_ARGS)
{
    char       *str;

    str = out((chkpass *) PG_GETARG_POINTER(0));
    PG_RETURN_TEXT_P(cstring_to_text(str));
}


/*
 * special output function that doesn't output the colon
 */

/*
 * Boolean tests
 */

PG_FUNCTION_INFO_V1(chkpass_eq);
Datum
chkpass_eq(PG_FUNCTION_ARGS)
{
    chkpass    *a1 = (chkpass *) PG_GETARG_POINTER(0);
    text       *a2 = (text *) PG_GETARG_TEXT_P(1);
    char       *str;
    char       *t;

    str = palloc(VARSIZE(a2)-VARHDRSZ+1);
    memcpy(str, VARDATA(a2), VARSIZE(a2)-VARHDRSZ);
    str[VARSIZE(a2)-VARHDRSZ] = 0;
    t = crypt(str, VARDATA(a1));

    PG_RETURN_BOOL(memcmp(VARDATA(a1), t, VARSIZE(a1)-VARHDRSZ) == 0);
}

PG_FUNCTION_INFO_V1(chkpass_ne);
Datum
chkpass_ne(PG_FUNCTION_ARGS)
{
    chkpass    *a1 = (chkpass *) PG_GETARG_POINTER(0);
    text       *a2 = (text *) PG_GETARG_TEXT_P(1);
    char       *str;
    char       *t;

    str = palloc(VARSIZE(a2)-VARHDRSZ+1);
    memcpy(str, VARDATA(a2), VARSIZE(a2)-VARHDRSZ);
    str[VARSIZE(a2)-VARHDRSZ] = 0;
    t = crypt(str, a1->vl_dat);

    PG_RETURN_BOOL(memcmp(VARDATA(a1), t, VARSIZE(a1)-VARHDRSZ) != 0);
}

/* define a custom variable to control md5 hashing of passwords.
 * This should require a SIGHUP because it would be administratively
 * disasterous to allow users to turn on md5 hashing, however I get
 * errors when loading the library if I use any setting other than
 * PGC_USERSET */
void
_PG_init(void)
{
    static char desc[] = "Whether to use md5 hashing for chkpass
passwords.";
    static char longdesc[] = "If set to true, new passwords will be
hashed using an md5-based algorithm. Otherwise the traditional Unix
algorithm will be used. Either kind of password can be read, provided
the server OS supports it.";

    DefineCustomBoolVariable(
        "chkpass.use_md5",  /* const char * name */
         desc,              /* const char * short_desc */
         longdesc,          /* const char * long_desc */
         &use_md5,          /* bool * valueAddr */
         true,              /* bool bootValue */
         PGC_USERSET,       /* GucContext context */
         0,                 /* int flags */
         NULL,              /* GucBoolCheckHook check_hook */
         NULL,              /* GucBoolAssignHook assign_hook */
         NULL               /* GucShowHook show_hook */
    );
}


-- 
D'Arcy J.M. Cain <darcy@druid.net>         |  Democracy is three wolves
http://www.druid.net/darcy/                |  and a sheep voting on
+1 416 788 2246     (DoD#0082)    (eNTP)   |  what's for dinner.
IM: darcy@Vex.Net, VoIP: sip:darcy@druid.net


pgsql-bugs by date:

Previous
From: Tom Lane
Date:
Subject: Re: BUG #15232: Query execution changes based on using 'explain analyze' or not
Next
From: PG Bug reporting form
Date:
Subject: BUG #15233: Error in estimation leads to very bad parralel plan insimple 2 table join.