besser82/libxcrypt

crypt_data namespace collision

Opened this issue · 4 comments

jhgit commented

For some operating systems, such as FreeBSD, struct crypt_data is defined in the unistd.h system header.

In the error output below, libxcrypt's crypt.h is in /usr/local/include...

In file included from ../src/libaccountsservice/act-user.c:30:
/usr/local/include/crypt.h:73:8: error: redefinition of 'crypt_data'
struct crypt_data
^
/usr/include/unistd.h:489:8: note: previous definition is here
struct crypt_data {
^

I don't have a good solution yet. One possibility might be to rename crypt_data in libxcrypt to something else (xcrypt_data, for example).

Hmmm... Looks like freebsd unistd also has crypt and crypt_r. That would also cause some redeclaration issues down the line once the struct is renamed, I guess.

xcrypt_* used to be a thing, but it's deprecated.

Before we can practically do anything about this, we need to decide: should libxcrypt on FreeBSD be a binary compatible drop-in replacement for FreeBSD's libcrypt.so.5 ? If so, we will need to keep the names the same, and we will need to figure out a way to reconcile libxcrypt's definition of crypt_data

libxcrypt/lib/crypt.h.in

Lines 70 to 109 in 72f75aa

struct crypt_data
{
/* crypt_r writes the hashed password to this field of its 'data'
argument. crypt_rn and crypt_ra do the same, treating the
untyped data area they are supplied with as this struct. */
char output[CRYPT_OUTPUT_SIZE];
/* Applications are encouraged, but not required, to use this field
to store the "setting" string that must be passed to crypt_*.
Future extensions to the API may make this more ergonomic.
A valid "setting" is either previously hashed password or the
string produced by one of the crypt_gensalt functions; see the
crypt_gensalt documentation for further details. */
char setting[CRYPT_OUTPUT_SIZE];
/* Applications are encouraged, but not required, to use this field
to store the unhashed passphrase they will pass to crypt_*.
Future extensions to the API may make this more ergonomic. */
char input[CRYPT_MAX_PASSPHRASE_SIZE];
/* Reserved for future application-visible fields. For maximum
forward compatibility, applications should set this field to all
bytes zero before calling crypt_r, crypt_rn, or crypt_ra for the
first time with a just-allocated 'struct crypt_data'. Future
extensions to the API may make this more ergonomic. */
char reserved[CRYPT_DATA_RESERVED_SIZE];
/* This field should be set to 0 before calling crypt_r, crypt_rn,
or crypt_ra for the first time with a just-allocated
'struct crypt_data'. This is not required if crypt_ra is allowed
to do the allocation itself (i.e. if the *DATA argument is a null
pointer). Future extensions to the API may make this more ergonomic. */
char initialized;
/* Scratch space used internally. Applications should not read or
write this field. All data written to this area is erased before
returning from the library. */
char internal[CRYPT_DATA_INTERNAL_SIZE];
};
)

with FreeBSD's different, much smaller definition

struct crypt_data {
        int     initialized;    /* For compatibility with glibc. */
        char    __buf[256];     /* Buffer returned by crypt_r(). */
};

If binary backward compatibility is not desirable on FreeBSD, then we should probably rename all the functions and not provide crypt or crypt_r at all.

I am only an occasional user of FreeBSD and am in no way qualified to decide whether libxcrypt should be binary compatible with libcrypt.so.5. We need to hear from FreeBSD core developers about what they think would be appropriate here.

I would personally be happy to cooperate with an effort to make libxcrypt more usable on FreeBSD, including both binary compatibility and replacing all the rest of the surviving (L)GPL code so that it could be adopted officially, but I do not have time to do the work myself, and I do not know whether the other core libxcrypt developers (@besser82, @solardiz, @vt-alt) would be on board with this.

It could help us to know the use cases people are now trying to use libxcrypt on FreeBSD for. What was yours, @jhgit?

I am not the issue author, but today (or rather, past me from a 3 years ago) I did come across one case where there is a point for libxcrypt to act independently of libc headers. In nuvious/pam-duress#14 I mentioned that a little bit of libxcrypt can be used instead of the libc implementation to offer stronger algorithms on unsupported platforms, but that probably won't work given the current collision.