Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20130411225913.GI20323@brightrain.aerifal.cx>
Date: Thu, 11 Apr 2013 18:59:13 -0400
From: Rich Felker <dalias@...ifal.cx>
To: musl@...ts.openwall.com
Subject: Re: crypt_data: structure size in crypt.c vs. crypt.h

On Thu, Apr 11, 2013 at 06:42:26PM -0400, Zvi Gilboa wrote:
> Greetings,
> 
> Looking at crypt.c and crypt.h, I was wondering whether having
> different-size /crypt_data/ structures was intentional, and if so,
> for what reason.  At the moment, the declarations are (and see also
> the attached code):

Yes and no. struct crypt_data becomes part of the ABI of the program
that calls crypt_r, so it needs to be "large enough for any future
password hash". The number 256 was chosen as a nice round value. On
the other hand, the static buffer used by crypt() only needs to be
"large enough for any currently-supported password hash", which is
presently 128.

> /<crypt.h>/
> struct crypt_data {
>     int initialized;
>     char *__buf[256]*;
> }; /* 260 bytes when sizeof(int)==4 */
> 
> /<crypt.c>/
> char *__crypt_r(const char *, const char *, *struct crypt_data **);
> 
> char *crypt(const char *key, const char *salt)
> {
>     static char *buf[128]*;
>     return __crypt_r(key, salt, *(struct crypt_data *)buf*);
>     /* when sizeof(int)==4, this leaves __buf with 124 bytes. */

No, the size is the full 128. There really is no such member as
"initialized" as far as musl's __crypt_r is concerned; the whole
object is used simply as a char[] buffer. The only reason the
"initialized" member exists is to meet the API requirements of the
crypt_r function -- per the API, applications are supposed to either
set crypt_data.initialized=0, or zero-initialize the whole object,
before calling crypt_r. If a member named "initialized" did not exist,
such programs would fail to compile. However, musl itself has no use
for any initialized member, since it only uses crypt_data as an output
buffer, not a working buffer for crypto state.

> On that note: since the /initialized /member is (currently) never
> referenced by name, adding a comment about that to the code might
> help readers who are yet to be initiated:)

Where do you think would be best to add it? In __crypt_r? The
"natural" place would be the headers, but the current policy is to
avoid comments in the headers, especially ones which would be
implementation-specific rather than documenting the API.

Rich

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.