Re: chkpass Major Issue - compares 'contains' and not 'equal'

From: D'Arcy Cain <darcy(at)druid(dot)net>
To: "David G(dot) Johnston" <david(dot)g(dot)johnston(at)gmail(dot)com>, Eyedia Tech <eyedia(at)debjyoti(dot)com>
Cc: "pgsql-bugs(at)postgresql(dot)org" <pgsql-bugs(at)postgresql(dot)org>
Subject: Re: chkpass Major Issue - compares 'contains' and not 'equal'
Date: 2018-06-07 18:20:00
Message-ID: a70d54d1-7103-d6d4-983d-b603de3869b3@druid.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: 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(at)druid(dot)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(at)druid(dot)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(at)Vex(dot)Net, VoIP: sip:darcy(at)druid(dot)net

In response to

Browse pgsql-bugs by date

  From Date Subject
Next Message PG Bug reporting form 2018-06-07 21:35:53 BUG #15233: Error in estimation leads to very bad parralel plan in simple 2 table join.
Previous Message Tom Lane 2018-06-07 17:06:33 Re: BUG #15232: Query execution changes based on using 'explain analyze' or not