diff mbox series

[2/2] crypt.3, encrypt.3: Add notes about _XOPEN_CRYPT.

Message ID 6d9a7e45-5d31-efbd-78ad-2808d492eb21@redhat.com
State New
Headers show
Series [1/2] posixoptions.7: Expand XSI Options groups. | expand

Commit Message

Carlos O'Donell April 12, 2018, 7:05 p.m. UTC 
The distribution may choose not to support _XOPEN_CRYPT in the
case that the distribution has transitioned from glibc crypt to
libxcrypt.

Signed-off-by: Carlos O'Donell <carlos@redhat.com>

Comments

Michael Kerrisk \(man-pages\) April 13, 2018, 9:34 a.m. UTC | #1 
On 04/12/2018 09:05 PM, Carlos O'Donell wrote:
> The distribution may choose not to support _XOPEN_CRYPT in the
> case that the distribution has transitioned from glibc crypt to
> libxcrypt.
> 
> Signed-off-by: Carlos O'Donell <carlos@redhat.com>

Thanks, Carlos. I applied the patch, but rather than having the
exact same text in both pages, I simply added a cross reference 
in encrypt(3).

Cheers,

Michael
Carlos O'Donell April 13, 2018, 1:22 p.m. UTC | #2 
On 04/13/2018 04:34 AM, Michael Kerrisk (man-pages) wrote:
> On 04/12/2018 09:05 PM, Carlos O'Donell wrote:
>> The distribution may choose not to support _XOPEN_CRYPT in the
>> case that the distribution has transitioned from glibc crypt to
>> libxcrypt.
>>
>> Signed-off-by: Carlos O'Donell <carlos@redhat.com>
> 
> Thanks, Carlos. I applied the patch, but rather than having the
> exact same text in both pages, I simply added a cross reference 
> in encrypt(3).

That makes sense. I thought about exactly the same thing, but
decided that I'd let you use your liberty as editor to make that
decision upon review.

Thanks for applying the fixes.

Consider my fixes to posixoptions.7 as penance for complicating
encrypt.3. We have users going through that transition now for
us in Fedora (libcrypt -> libxcrypt), and we wanted to provide
some kind of note about the APIs moving. The changes are not
yet in upstream glibc, but may eventually go there, at which
point the note will still be valid.

We will need similar notes I think for our SunRPC -> TIRPC
transition, touching the various RPC man pages and explaining
any differences.
Carlos O'Donell April 13, 2018, 3:10 p.m. UTC | #3 
On 04/12/2018 02:05 PM, Carlos O'Donell wrote:
> +.BR _XOPEN_CRPYT

s/CRPYT/CRYPT/g

Which I missed in the spell checker due to my overzealous use of ignore.

Caught by Nikos Mavrogiannopoulos <nmavrogi@redhat.com> during review.

> +is not available and include crypt.h for the function prototypes; otherwise
> +libxcrypt is a ABI compatible drop-in replacement.
Michael Kerrisk \(man-pages\) April 13, 2018, 7:24 p.m. UTC | #4 
On 04/13/2018 05:10 PM, Carlos O'Donell wrote:
> On 04/12/2018 02:05 PM, Carlos O'Donell wrote:
>> +.BR _XOPEN_CRPYT
> 
> s/CRPYT/CRYPT/g
> 
> Which I missed in the spell checker due to my overzealous use of ignore.
> 
> Caught by Nikos Mavrogiannopoulos <nmavrogi@redhat.com> during review.

Thanks. Fixed!

Cheers,

Michael
Rical Jasan April 14, 2018, 4:43 a.m. UTC | #5 
On 04/12/2018 12:05 PM, Carlos O'Donell wrote:
> diff --git a/man3/crypt.3 b/man3/crypt.3
> index 3e6552ad8..58c62d93d 100644
> --- a/man3/crypt.3
> +++ b/man3/crypt.3
> @@ -188,8 +188,26 @@ POSIX.1-2001, POSIX.1-2008, SVr4, 4.3BSD.
>  .BR crypt_r ()
>  is a GNU extension.
>  .SH NOTES
> -.SS Glibc notes
> -The glibc2 version of this function supports additional
> +.SS Availability in glibc
> +The
> +.BR crypt (),
> +.BR encrypt (),
> +and
> +.BR setkey ()
> +functions are part of the POSIX.1-2008 XSI Options Group for Encryption
> +and are optional. If the interfaces are not available then the symbolic
> +constant
> +.BR _XOPEN_CRYPT
> +is either not defined or defined to -1, and can be checked at runtime with
> +.BR sysconf ().
> +This may be the case if the downstream distribution has switched from glibc
> +crypt to libxcrypt.  When recompiling applications in such distributions the
> +user must detect if
> +.BR _XOPEN_CRPYT

"CRYPT"

(Looks like this was caught already.)

> +is not available and include crypt.h for the function prototypes; otherwise
> +libxcrypt is a ABI compatible drop-in replacement.

"an ABI"

I would also hyphenate "ABI-compatible".

(The same issues were also in encrypt.3 but it sounds like the content
was deduplicated already.)


Rical
Michael Kerrisk \(man-pages\) April 14, 2018, 5:39 a.m. UTC | #6 
Hi Rical,

On 14 April 2018 at 06:43, Rical Jasan <rj@2c3t.io> wrote:
> On 04/12/2018 12:05 PM, Carlos O'Donell wrote:
>> diff --git a/man3/crypt.3 b/man3/crypt.3
>> index 3e6552ad8..58c62d93d 100644
>> --- a/man3/crypt.3
>> +++ b/man3/crypt.3
>> @@ -188,8 +188,26 @@ POSIX.1-2001, POSIX.1-2008, SVr4, 4.3BSD.
>>  .BR crypt_r ()
>>  is a GNU extension.
>>  .SH NOTES
>> -.SS Glibc notes
>> -The glibc2 version of this function supports additional
>> +.SS Availability in glibc
>> +The
>> +.BR crypt (),
>> +.BR encrypt (),
>> +and
>> +.BR setkey ()
>> +functions are part of the POSIX.1-2008 XSI Options Group for Encryption
>> +and are optional. If the interfaces are not available then the symbolic
>> +constant
>> +.BR _XOPEN_CRYPT
>> +is either not defined or defined to -1, and can be checked at runtime with
>> +.BR sysconf ().
>> +This may be the case if the downstream distribution has switched from glibc
>> +crypt to libxcrypt.  When recompiling applications in such distributions the
>> +user must detect if
>> +.BR _XOPEN_CRPYT
>
> "CRYPT"
>
> (Looks like this was caught already.)

Yup.

>> +is not available and include crypt.h for the function prototypes; otherwise
>> +libxcrypt is a ABI compatible drop-in replacement.
>
> "an ABI"

Already found and fixed.

> I would also hyphenate "ABI-compatible".

Agreed, and already fixed.

> (The same issues were also in encrypt.3 but it sounds like the content
> was deduplicated already.)

Correct.

Thanks,

Michael
Nikos Mavrogiannopoulos April 14, 2018, 7:11 p.m. UTC | #7 
On Fri, Apr 13, 2018 at 9:24 PM, Michael Kerrisk (man-pages)
<mtk.manpages@gmail.com> wrote:
>
> On 04/13/2018 05:10 PM, Carlos O'Donell wrote:
> > On 04/12/2018 02:05 PM, Carlos O'Donell wrote:
> >> +.BR _XOPEN_CRPYT
> >
> > s/CRPYT/CRYPT/g
> >
> > Which I missed in the spell checker due to my overzealous use of ignore.
> >
> > Caught by Nikos Mavrogiannopoulos <nmavrogi@redhat.com> during review.
>
> Thanks. Fixed!

[re-sending in non-html]

Hi Michael,
 What about the second part of the suggestion. The crypt.3 manpage
currently suggests including <unistd.h>, with _XOPEN_SOURCE defined,
and that does not work with current glibc. Wouldn't it make sense to
switch to <crypt.h>?

regards,
Nikos
Florian Weimer April 14, 2018, 7:56 p.m. UTC | #8 
On 04/14/2018 09:11 PM, Nikos Mavrogiannopoulos wrote:
>   What about the second part of the suggestion. The crypt.3 manpage
> currently suggests including <unistd.h>, with _XOPEN_SOURCE defined,
> and that does not work with current glibc.

Current Fedora glibc.  We need to align the Fedora behavior with 
upstream again, either by lobbying for an upstream change, or changing 
Fedora back to how things were before.

Thanks,
Florian
Zack Weinberg April 16, 2018, 1:35 a.m. UTC | #9 
On Sat, Apr 14, 2018 at 3:56 PM, Florian Weimer <fweimer@redhat.com> wrote:
> On 04/14/2018 09:11 PM, Nikos Mavrogiannopoulos wrote:
>>
>>   What about the second part of the suggestion. The crypt.3 manpage
>> currently suggests including <unistd.h>, with _XOPEN_SOURCE defined,
>> and that does not work with current glibc.
>
> Current Fedora glibc.  We need to align the Fedora behavior with upstream
> again, either by lobbying for an upstream change, or changing Fedora back to
> how things were before.

Portable code has always needed to cope with the possibility of
crypt() not being declared by unistd.h -- that goes all the way back
to the old export control policy in the 1990s.  This is what I wrote
for the new crypt(3) manpage shipped with libxcrypt:

SYNOPSIS
       #include <crypt.h>

       char *crypt(const char *phrase, const char *setting);
       char *crypt_r(const char *phrase, const char *setting, struct
               crypt_data *data);
       char *crypt_rn(const char *phrase, const char *setting, void *data, int
               size);
       char *crypt_ra(const char *phrase, const char *setting, void **data,
               int *size);

       Link with -lcrypt.

[...]
PORTABILITY NOTES
       crypt is included in POSIX, but crypt_r, crypt_rn, and crypt_ra are not
       part of any standard.
[...]
       Due to historical restrictions on the export of cryptographic  software
       from  the  USA,  crypt  is  an  optional POSIX component.  Applications
       should therefore be prepared for crypt  not  to  be  available,  or  to
       always fail (setting errno to ENOSYS) at runtime.

       POSIX  specifies  that  crypt  is declared in unistd.h, but only if the
       macro _XOPEN_CRYPT is defined and has a value greater than or equal  to
       zero.   Since  libcrypt  does  not provide unistd.h, it declares crypt,
       crypt_r, crypt_rn, and crypt_ra in crypt.h instead.

I think this would be a good tack to take in the Linux manpages'
crypt(3) as well, minus explicit mentions of lib(x)crypt.

Incidentally, libxcrypt doesn't prototype encrypt or setkey in public
headers, nor does it allow new programs to link against them. Please
consider dropping encrypt(3) or at least declaring it to be obsolete
at the top of the manpage. Nobody should be using single DES for
anything anymore.

I'm a little surprised to hear Fedora has taken the plunge on
switching to libxcrypt.  I did put in a whole lot of work last year on
making it an ABI-compatible replacement, but I still have a long list
of things that need changing before I consider it *done*, and I
believe so does Bjoern.  But I do look forward to the bug reports from
more extensive testing :-)

I'll send a patch shortly that adds a --disable-libcrypt configure
switch to glibc, hopefully that can work for Fedora and you don't have
to be carrying patches around.

zw
Nikos Mavrogiannopoulos April 17, 2018, 1:25 p.m. UTC | #10 
On Sat, Apr 14, 2018 at 9:56 PM, Florian Weimer <fweimer@redhat.com> wrote:
> On 04/14/2018 09:11 PM, Nikos Mavrogiannopoulos wrote:
>>
>>   What about the second part of the suggestion. The crypt.3 manpage
>> currently suggests including <unistd.h>, with _XOPEN_SOURCE defined,
>> and that does not work with current glibc.
>
>
> Current Fedora glibc.  We need to align the Fedora behavior with upstream
> again, either by lobbying for an upstream change, or changing Fedora back to
> how things were before.

Zack mentions reasons that crypt.h is already in use, and glibc itself
is already documenting the use of crypt.h instead of unistd.h [0].
uclibc also provides crypt.h [1]. Wouldn't that be a sufficient reason
to make the manpage document crypt.h as primary?


[0]. https://www.gnu.org/software/libc/manual/html_node/crypt.html
[1]. https://git.busybox.net/uClibc/tree/include/crypt.h
Florian Weimer April 17, 2018, 1:31 p.m. UTC | #11 
On 04/17/2018 03:25 PM, Nikos Mavrogiannopoulos wrote:
> On Sat, Apr 14, 2018 at 9:56 PM, Florian Weimer <fweimer@redhat.com> wrote:
>> On 04/14/2018 09:11 PM, Nikos Mavrogiannopoulos wrote:
>>>
>>>    What about the second part of the suggestion. The crypt.3 manpage
>>> currently suggests including <unistd.h>, with _XOPEN_SOURCE defined,
>>> and that does not work with current glibc.
>>
>>
>> Current Fedora glibc.  We need to align the Fedora behavior with upstream
>> again, either by lobbying for an upstream change, or changing Fedora back to
>> how things were before.
> 
> Zack mentions reasons that crypt.h is already in use, and glibc itself
> is already documenting the use of crypt.h instead of unistd.h [0].
> uclibc also provides crypt.h [1]. Wouldn't that be a sufficient reason
> to make the manpage document crypt.h as primary?

If it's just about changing the recommendation, then I'm fine with that. 
  But we don't know yet how things will turn out upstream, so spelling 
out *why* <crypt.h> is preferable would still be tricky.

Thanks,
Florian
diff mbox series

Patch

From d099bd9550d28da19dd082d27491f49bd8c721e9 Mon Sep 17 00:00:00 2001
From: Carlos O'Donell <carlos@systemhalted.org>
Date: Thu, 12 Apr 2018 13:58:27 -0500
Subject: [PATCH 2/2] crypt.3, encrypt.3: Add notes about _XOPEN_CRYPT.

The distribution may choose not to support _XOPEN_CRYPT in the
case that the distribution has transitioned from glibc crypt to
libxcrypt.

Signed-off-by: Carlos O'Donell <carlos@redhat.com>
---
 man3/crypt.3   | 22 ++++++++++++++++++++--
 man3/encrypt.3 | 19 +++++++++++++++++++
 2 files changed, 39 insertions(+), 2 deletions(-)

diff --git a/man3/crypt.3 b/man3/crypt.3
index 3e6552ad8..58c62d93d 100644
--- a/man3/crypt.3
+++ b/man3/crypt.3
@@ -188,8 +188,26 @@  POSIX.1-2001, POSIX.1-2008, SVr4, 4.3BSD.
 .BR crypt_r ()
 is a GNU extension.
 .SH NOTES
-.SS Glibc notes
-The glibc2 version of this function supports additional
+.SS Availability in glibc
+The
+.BR crypt (),
+.BR encrypt (),
+and
+.BR setkey ()
+functions are part of the POSIX.1-2008 XSI Options Group for Encryption
+and are optional. If the interfaces are not available then the symbolic
+constant
+.BR _XOPEN_CRYPT
+is either not defined or defined to -1, and can be checked at runtime with
+.BR sysconf ().
+This may be the case if the downstream distribution has switched from glibc
+crypt to libxcrypt.  When recompiling applications in such distributions the
+user must detect if
+.BR _XOPEN_CRPYT
+is not available and include crypt.h for the function prototypes; otherwise
+libxcrypt is a ABI compatible drop-in replacement.
+.SS Features in glibc
+The glibc version of this function supports additional
 encryption algorithms.
 .PP
 If
diff --git a/man3/encrypt.3 b/man3/encrypt.3
index 3a1cf9598..3df4870c3 100644
--- a/man3/encrypt.3
+++ b/man3/encrypt.3
@@ -147,6 +147,25 @@  and
 .BR setkey_r ()
 are GNU extensions.
 .SH NOTES
+.SS Availability in glibc
+The
+.BR crypt (),
+.BR encrypt (),
+and
+.BR setkey ()
+functions are part of the POSIX.1-2008 XSI Options Group for Encryption
+and are optional. If the interfaces are not available then the symbolic
+constant
+.BR _XOPEN_CRYPT
+is either not defined or defined to -1, and can be checked at runtime with
+.BR sysconf ().
+This may be the case if the downstream distribution has switched from glibc
+crypt to libxcrypt.  When recompiling applications in such distributions the
+user must detect if
+.BR _XOPEN_CRPYT
+is not available and include crypt.h for the function prototypes; otherwise
+libxcrypt is a ABI compatible drop-in replacement.
+.SS Features in glibc
 In glibc 2.2, these functions use the DES algorithm.
 .SH EXAMPLE
 .EX
-- 
2.14.3