Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

crypt_data namespace collision #175

Open
jhgit opened this issue Aug 14, 2023 · 4 comments
Open

crypt_data namespace collision #175

jhgit opened this issue Aug 14, 2023 · 4 comments
Labels
bug Confirmed to be a bug in libxcrypt. need more information We cannot do anything about this issue until we have more information. os-freebsd Issues specific to FreeBSD.

Comments

@jhgit
Copy link

jhgit commented Aug 14, 2023

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).

@Artoria2e5
Copy link

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.

@zackw
Copy link
Collaborator

zackw commented Mar 26, 2024

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.

@zackw zackw added bug Confirmed to be a bug in libxcrypt. need more information We cannot do anything about this issue until we have more information. os-freebsd Issues specific to FreeBSD. labels Mar 26, 2024
@solardiz
Copy link
Collaborator

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

@Artoria2e5
Copy link

Artoria2e5 commented May 23, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Confirmed to be a bug in libxcrypt. need more information We cannot do anything about this issue until we have more information. os-freebsd Issues specific to FreeBSD.
Projects
None yet
Development

No branches or pull requests

4 participants