Thread: crypt and null termination
Look at this from the BSD/OS crypt() manual page:
The crypt function performs password encryption. It is derived from the NBS Data Encryption Standard.
Additionalcode has been added to deter key search attempts. The first argument to crypt is a NUL-terminated
string(normally a password typed by a user). The second is a character array, 9 bytes in length, consisting of an
underscore(``_'') followed by 4 bytes of iteration count and 4 bytes of salt. Both the iteration count and the
saltare encoded with 6 bits per character, least significant bits first. The values 0 to 63 are encoded by the
characters``./0-9A- Za-z'', respectively.
...
For compatibility with historical versions of crypt(3), the setting may consist of 2 bytes of salt, encoded as
above,in which case an iteration count of 25 is used, fewer perturbations of DES are available, at most 8
charactersof key are used, and the returned value is a NUL-terminated string 13 bytes in length.
It seems to say that the salt passed to crypt should be null-terminated, but
we call crypt from libpq as:
crypt_pwd = crypt(password, conn->salt);
and conn.salt is char[2]. Isn't this a problem?
-- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610)
853-3000+ If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill,
Pennsylvania19026
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> It seems to say that the salt passed to crypt should be null-terminated,
Hmm. The HPUX man page for crypt() just says thatsalt is a two-character string chosen from the set [a-zA-Z0-9./]
which I think is the traditional spec. Looks like BSD has adopted some
local extensions.
Note that the BSD page specifies that the extended salt format starts
with '_', which is not one of the allowed characters in the traditional
format. I bet they check that before trying to fetch more than 2 bytes.
The second paragraph you quote doesn't say anything about null
termination.
Still, it wouldn't be a bad idea to add a null byte ... couldn't hurt.
regards, tom lane
Bruce Momjian <pgman@candle.pha.pa.us> writes: > Look at this from the BSD/OS crypt() manual page: > > The crypt function performs password encryption. It is derived from the > NBS Data Encryption Standard. Additional code has been added to deter > key search attempts. The first argument to crypt is a NUL-terminated > string (normally a password typed by a user). The second is a character > array, 9 bytes in length, consisting of an underscore (``_'') followed by > 4 bytes of iteration count and 4 bytes of salt. Both the iteration count > and the salt are encoded with 6 bits per character, least significant > bits first. The values 0 to 63 are encoded by the characters ``./0-9A- > Za-z'', respectively. > > ... > > For compatibility with historical versions of crypt(3), the setting may > consist of 2 bytes of salt, encoded as above, in which case an iteration > count of 25 is used, fewer perturbations of DES are available, at most 8 > characters of key are used, and the returned value is a NUL-terminated > string 13 bytes in length. > > It seems to say that the salt passed to crypt should be null-terminated, but > we call crypt from libpq as: > > crypt_pwd = crypt(password, conn->salt); > > and conn.salt is char[2]. Isn't this a problem? I don't think it is. Note that it refers to the salt as a "character array", not a string. Also, since '_' isn't in the allowed encoding set, it can tell the difference between a 9-byte salt and a 2-byte salt without a terminating NUL. -Doug -- Free Dmitry Sklyarov! http://www.freesklyarov.org/ We will return to our regularly scheduled signature shortly.
> Bruce Momjian <pgman@candle.pha.pa.us> writes: > > It seems to say that the salt passed to crypt should be null-terminated, > > Hmm. The HPUX man page for crypt() just says that > salt is a two-character string chosen from the set [a-zA-Z0-9./] > which I think is the traditional spec. Looks like BSD has adopted some > local extensions. > > Note that the BSD page specifies that the extended salt format starts > with '_', which is not one of the allowed characters in the traditional > format. I bet they check that before trying to fetch more than 2 bytes. > The second paragraph you quote doesn't say anything about null > termination. > > Still, it wouldn't be a bad idea to add a null byte ... couldn't hurt. That was my feeling, and it may explain some of our crypt() platform failures! -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 853-3000+ If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania19026
Bruce Momjian <pgman@candle.pha.pa.us> writes: > > > and conn.salt is char[2]. Isn't this a problem? > > > > I don't think it is. Note that it refers to the salt as a "character > > array", not a string. Also, since '_' isn't in the allowed encoding > > set, it can tell the difference between a 9-byte salt and a 2-byte > > salt without a terminating NUL. > > I didn't pick up that array item. > > Anyway, the patch is small so I will apply it. There is no telling what > OS's expect a character string there. Certainly won't hurt. I just looked at the docs for glibc on Linux, and it has its own semi-weird extension format for MD5-based hashing, but doesn't seem to require null termination--it uses an initial '$', which again isn't part of the encoding set, as a discriminator rather than '_', and will treat either another '$' or a NUL as the terminator for the extended salt. -Doug -- Free Dmitry Sklyarov! http://www.freesklyarov.org/ We will return to our regularly scheduled signature shortly.
> > and conn.salt is char[2]. Isn't this a problem? > > I don't think it is. Note that it refers to the salt as a "character > array", not a string. Also, since '_' isn't in the allowed encoding > set, it can tell the difference between a 9-byte salt and a 2-byte > salt without a terminating NUL. I didn't pick up that array item. Anyway, the patch is small so I will apply it. There is no telling what OS's expect a character string there. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 853-3000 + If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania 19026 Index: src/backend/libpq/crypt.c =================================================================== RCS file: /home/projects/pgsql/cvsroot/pgsql/src/backend/libpq/crypt.c,v retrieving revision 1.35 diff -c -r1.35 crypt.c *** src/backend/libpq/crypt.c 2001/08/17 02:59:19 1.35 --- src/backend/libpq/crypt.c 2001/08/17 03:07:19 *************** *** 295,302 **** switch (port->auth_method) { case uaCrypt: ! crypt_pwd = crypt(passwd, port->cryptSalt); break; case uaMD5: crypt_pwd = palloc(MD5_PASSWD_LEN+1); if (isMD5(passwd)) --- 295,306 ---- switch (port->auth_method) { case uaCrypt: ! { ! char salt[3]; ! StrNCpy(salt, port->cryptSalt,3); ! crypt_pwd = crypt(passwd, salt); break; + } case uaMD5: crypt_pwd = palloc(MD5_PASSWD_LEN+1); if (isMD5(passwd)) Index: src/interfaces/libpq/fe-auth.c =================================================================== RCS file: /home/projects/pgsql/cvsroot/pgsql/src/interfaces/libpq/fe-auth.c,v retrieving revision 1.51 diff -c -r1.51 fe-auth.c *** src/interfaces/libpq/fe-auth.c 2001/08/17 02:59:19 1.51 --- src/interfaces/libpq/fe-auth.c 2001/08/17 03:07:27 *************** *** 443,450 **** switch (areq) { case AUTH_REQ_CRYPT: ! crypt_pwd = crypt(password, conn->cryptSalt); break; case AUTH_REQ_MD5: { char *crypt_pwd2; --- 443,455 ---- switch (areq) { case AUTH_REQ_CRYPT: ! { ! char salt[3]; ! ! StrNCpy(salt, conn->cryptSalt,3); ! crypt_pwd = crypt(password, salt); break; + } case AUTH_REQ_MD5: { char *crypt_pwd2;
Bruce Momjian writes: > Anyway, the patch is small so I will apply it. There is no telling what > OS's expect a character string there. There's a pretty good telling: Nobody ever reported a problem related to this. -- Peter Eisentraut peter_e@gmx.net http://funkturm.homeip.net/~peter
> Bruce Momjian writes: > > > Anyway, the patch is small so I will apply it. There is no telling what > > OS's expect a character string there. > > There's a pretty good telling: Nobody ever reported a problem related to > this. We have had crypts that didn't work across platforms. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 853-3000+ If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania19026
Bruce Momjian writes: > > Bruce Momjian writes: > > > > > Anyway, the patch is small so I will apply it. There is no telling what > > > OS's expect a character string there. > > > > There's a pretty good telling: Nobody ever reported a problem related to > > this. > > We have had crypts that didn't work across platforms. That's because they used different encoding algorithms. A missing null terminator has different effects, across platforms or not. -- Peter Eisentraut peter_e@gmx.net http://funkturm.homeip.net/~peter