crypt and null termination

Started by Bruce Momjianover 24 years ago9 messageshackers
Jump to latest
#1Bruce Momjian
bruce@momjian.us

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?

-- 
  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
#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Bruce Momjian (#1)
Re: crypt and null termination

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.

regards, tom lane

#3Bruce Momjian
bruce@momjian.us
In reply to: Tom Lane (#2)
Re: crypt and null termination

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, Pennsylvania 19026
#4Doug McNaught
doug@wireboard.com
In reply to: Bruce Momjian (#1)
Re: crypt and null termination

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.

#5Bruce Momjian
bruce@momjian.us
In reply to: Doug McNaught (#4)
Re: crypt and null termination

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

Attachments:

/bjm/difftext/plainDownload+13-11
#6Doug McNaught
doug@wireboard.com
In reply to: Bruce Momjian (#5)
Re: crypt and null termination

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.

#7Peter Eisentraut
peter_e@gmx.net
In reply to: Bruce Momjian (#5)
Re: crypt and null termination

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

#8Bruce Momjian
bruce@momjian.us
In reply to: Peter Eisentraut (#7)
Re: crypt and null termination

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, Pennsylvania 19026
#9Peter Eisentraut
peter_e@gmx.net
In reply to: Bruce Momjian (#8)
Re: crypt and null termination

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