Re: [ptxdist] [PATCH v6] libxcrypt: new package

[Michael Olbrich <m.olbrich@pengutronix.de>]

Hi,

On Thu, May 05, 2022 at 02:46:20PM +0200, Andreas Helmcke wrote:
> Also implement the needed logic to (optionally) replace
> the libcrypt from the selected libc with libxcrypt.
> 
> libxcrypt is a modern library for one-way hashing of passwords.
> It supports a wide variety of both modern and historical hashing
> methods: yescrypt, gost-yescrypt, scrypt, bcrypt, sha512crypt,
> sha256crypt, md5crypt, SunMD5, sha1crypt, NT, bsdicrypt, bigcrypt,
> and descrypt. It provides the traditional Unix crypt and crypt_r
> interfaces, as well as a set of extended interfaces pioneered by
> Openwall Linux, crypt_rn, crypt_ra, crypt_gensalt, crypt_gensalt_rn,
> and crypt_gensalt_ra.
> 
> libxcrypt is intended to be used by login(1), passwd(1), and other
> similar programs; that is, to hash a small number of passwords
> during an interactive authentication dialogue with a human.  It is
> not suitable for use in bulk password-cracking applications, or in
> any other situation where speed is more important than careful
> handling of sensitive data.  However, it is intended to be fast and
> lightweight enough for use in servers that must field thousands of
> login attempts per minute.

I think I need some more background to decide what the correct integration
is. From what I understand, the situation is like this:

1. libxcrypt is fully binary backwards compatible. So we don't need to
build it before building any packages that could use the libcrypt from
glibc.

2. libxcrypt provides additional functionality. So packages that want to
use this must have the correct dependencies.

If this is correct then we need some changes. Mixing 'select' and 'choice'
does not work properly with kconfig.


> Co-authored-by: Andreas Helmcke <ahe@helmcke.name>
> Signed-off-by: Andreas Helmcke <ahe@helmcke.name>
> Signed-off-by: Björn Esser <bes@pengutronix.de>
> ---
> 
> v5 -> v6: (by Andreas Helmcke)
>   - Updated commit message to properly address authors
> 
> v4 -> v5: (by Andreas Helmcke)
>   - Update libxcrypt 4.4.10 -> 4.4.24
>   - Changed download url to official tar, which does not need autoconf
>   - Changed the config variable names to reflect menu structure
>   - Corrected two typos
> 
> original work by Björn Esser :
> v3 -> v4:
>   - Update libxcrypt 4.4.9 -> 4.4.10
> 
> v2 -> v3:
>   - Added 3 files that also needed minor adaptions and I forgot to add
>     to the initial patch.
> 
> v1 -> v2:
>   - Adapt the two remarks pointed out by Dennis Osterland
> 
> 
> 
>  rules/glibc.in       |   4 ++
>  rules/libc.in        |   7 ++-
>  rules/libcrypt.in    |  38 +++++++++++++++
>  rules/libcrypt.make  |  16 ++++++
>  rules/libxcrypt.in   | 114 +++++++++++++++++++++++++++++++++++++++++++
>  rules/libxcrypt.make |  95 ++++++++++++++++++++++++++++++++++++
>  rules/uclibc.in      |   4 ++
>  7 files changed, 274 insertions(+), 4 deletions(-)
>  create mode 100644 rules/libcrypt.in
>  create mode 100644 rules/libcrypt.make
>  create mode 100644 rules/libxcrypt.in
>  create mode 100644 rules/libxcrypt.make
> 
> diff --git a/rules/glibc.in b/rules/glibc.in
> index 16e5e84d1..1d1fa4980 100644
> --- a/rules/glibc.in
> +++ b/rules/glibc.in
> @@ -79,12 +79,16 @@ config GLIBC_DL
>           functionality you should probably use libtool instead. It is much
> more cross
>           platform compatible than dlopen, etc. It also supports BeOS. See
> related links.
> 
> +if LIBC_CRYPT_NATIVE_CRYPT
> +
>  config GLIBC_CRYPT
>         bool
>         prompt "Install libcrypt"
>         help
>           The encryption/decryption library
> 
> +endif
> +
>  config GLIBC_UTIL
>         bool
>         prompt "Install libutil"
> diff --git a/rules/libc.in b/rules/libc.in
> index 1614affd9..01fe55af5 100644
> --- a/rules/libc.in
> +++ b/rules/libc.in
> @@ -57,10 +57,9 @@ config LIBC_DL
>         select GLIBC_DL                 if LIBC_GLIBC
>         select UCLIBC_DL                if LIBC_UCLIBC
> 
> -config LIBC_CRYPT
> -       bool
> -       select GLIBC_CRYPT              if LIBC_GLIBC
> -       select UCLIBC_CRYPT             if LIBC_UCLIBC

Keep this but with 'if LIBC_GLIBC && !LIBC_CRYPT_NATIVE_CRYPT' etc.

and define LIBC_CRYPT_NATIVE_CRYPT here with 'default !LIBXCRYPT' here.

> +#
> +# LIBC_CRYPT is handled by rules/libcrypt.in.
> +#
> 
>  config LIBC_UTIL
>         bool
> diff --git a/rules/libcrypt.in b/rules/libcrypt.in
> new file mode 100644
> index 000000000..117cb72a5
> --- /dev/null
> +++ b/rules/libcrypt.in
> @@ -0,0 +1,38 @@
> +## SECTION=core
> +
> +menuconfig LIBC_CRYPT
> +       bool
> +       prompt "POSIX crypt implementation    "
> +       select LIBXCRYPT        if !LIBC_CRYPT_NATIVE_CRYPT
> +       select LIBC_CRYPT_INTERNAL_CRYPT        if LIBC_CRYPT_NATIVE_CRYPT
> +
> +if LIBC_CRYPT
> +
> +choice
> +       prompt "POSIX crypt implementation    "
> +       default LIBC_CRYPT_NATIVE_CRYPT
> +
> +       config LIBC_CRYPT_NATIVE_CRYPT
> +               bool
> +               prompt "libc internal"
> +               help
> +                 This menu entry selects the basic libcrypt provided
> +                 by the selected libc implementation of the system.
> +
> +       config LIBC_CRYPT_EXTENDED_CRYPT
> +               bool
> +               prompt "libxcrypt    "
> +               help
> +                 This menu entry selects the extended libcrypt
> +                 implementation provided by the libxcrypt package.
> +
> +                 Please see "System Libraries" for the configuration
> +                 options of libxcrypt.
> +endchoice
> +
> +config LIBC_CRYPT_INTERNAL_CRYPT
> +       bool
> +       select GLIBC_CRYPT      if LIBC_GLIBC
> +       select UCLIBC_CRYPT     if LIBC_UCLIBC
> +
> +endif

drop this.

> diff --git a/rules/libcrypt.make b/rules/libcrypt.make
> new file mode 100644
> index 000000000..6f1448fe0
> --- /dev/null
> +++ b/rules/libcrypt.make
> @@ -0,0 +1,16 @@
> +# -*-makefile-*-
> +#
> +# Copyright (C) 2019 by Bjoern Esser <bes@pengutronix.de>
> +#
> +# For further information about the PTXdist project and license conditions
> +# see the README file.
> +#
> +
> +#
> +# We provide this package
> +#
> +PACKAGES-$(PTXCONF_LIBC_CRYPT) += libcrypt
> +
> +LIBCRYPT_LICENSE:= ignore
> +
> +# vim: syntax=make
> diff --git a/rules/libxcrypt.in b/rules/libxcrypt.in
> new file mode 100644
> index 000000000..281dabde2
> --- /dev/null
> +++ b/rules/libxcrypt.in
> @@ -0,0 +1,114 @@
> +## SECTION=system_libraries
> +
> +menuconfig LIBXCRYPT
> +       bool
> +       prompt "libxcrypt                     "
> +       depends on !LIBC_CRYPT_NATIVE_CRYPT

drop this line.

Now libxcrypt will be used as soon as it is manually selected or when a
package depends on it.

> +       help
> +         Extended crypt library for descrypt, md5crypt, bcrypt, and others.
> +
> +         libxcrypt is a modern library for one-way hashing of passwords.
> +         It supports a wide variety of both modern and historical hashing
> +         methods: yescrypt, gost-yescrypt, scrypt, bcrypt, sha512crypt,
> +         sha256crypt, md5crypt, SunMD5, sha1crypt, NT, bsdicrypt, bigcrypt,
> +         and descrypt.  It provides the traditional Unix crypt and crypt_r
> +         interfaces, as well as a set of extended interfaces pioneered by
> +         Openwall Linux, crypt_rn, crypt_ra, crypt_gensalt,
> +         crypt_gensalt_rn, and crypt_gensalt_ra.
> +
> +         libxcrypt is intended to be used by login(1), passwd(1), and other
> +         similar programs; that is, to hash a small number of passwords
> +         during an interactive authentication dialogue with a human.  It is
> +         not suitable for use in bulk password-cracking applications, or in
> +         any other situation where speed is more important than careful
> +         handling of sensitive data.  However, it is intended to be fast
> and
> +         lightweight enough for use in servers that must field thousands of
> +         login attempts per minute.
> +
> +if LIBXCRYPT
> +
> +config LIBXCRYPT_GLIBC_BINARY_COMPAT
> +       bool
> +       prompt "Enable full glibc binary compatibility"
> +       help
> +         When enabled, this option includes the interfaces for full binary
> +         compatibility with glibc.
> +
> +         This setting only affects existing binaries; new programs cannot
> +         be linked against them.

Right, this answers one of my questions. I think this should be always
enabled. Otherwise, we need to ensure, that all packages that use libcrypt
are built after libxcrypt and that's not trivial.
While we do have some 'select LIBCRYPT', its a best effort kind of thing
because it does not fail when it's missing.

If we replace libcrypt in the toolchain, then we can disable this.

> +
> +if LIBXCRYPT_GLIBC_BINARY_COMPAT
> +
> +config LIBXCRYPT_OBSOLETE_STUBS
> +       bool
> +       prompt "Replace obsolete functions with non-functional stubs"
> +       help
> +         If enabled, this option replaces the obsolete APIs (fcrypt,
> +         encrypt{,_r}, and setkey{,_r}) with stubs that set errno to
> +         ENOSYS and return without performing any real operations.
> +
> +         For security reasons, the encrypt{,r} functions will also
> +         overwrite their data-block argument with random bits.
> +
> +         The fcrypt function will also always return NULL-pointer.

Hmmm, disabling obsolete stuff is tempting but I'm not so sure if that
makes the system more secure. That would require proper error handling by
all users. I guess an option for this is ok.

> +
> +endif
> +
> +config LIBXCRYPT_BCRYPT_X
> +       bool
> +       prompt "Support for verifying weak bcrypt ($2x$) hashes"
> +       help
> +         The alternative prefix "$2x$" provides bug-compatibility with
> +         crypt_blowfish 1.0.4 and earlier, which incorrectly processed
> +         characters with the 8th bit set.
> +
> +config LIBXCRYPT_SHA1CRYPT
> +       bool
> +       prompt "sha1crypt ($sha1) hashing method"
> +       help
> +         A hash based on HMAC-SHA1.  Originally developed for NetBSD.
> +
> +         Enable this for compatibility with passphrases that have been
> +         hashed on NetBSD.
> +
> +config LIBXCRYPT_SUNMD5
> +       bool
> +       prompt "SunMD5 ($md5) hashing method"
> +       help
> +         A hash based on the MD5 algorithm, with additional cleverness
> +         to make precomputation difficult.
> +
> +         Enable this for full compatibility with passphrases that have
> +         been hashed on Solaris.
> +
> +config LIBXCRYPT_NTHASH
> +       bool
> +       prompt "NTHASH ($3$) hashing method"
> +       help
> +         The hashing method used for network authentication in some
> +         versions of the SMB/CIFS protocol.
> +
> +         Available, for cross-compatibility's sake, on FreeBSD.
> +
> +config LIBXCRYPT_BSDICRYPT
> +       bool
> +       prompt "bsdicrypt ($2x$) hashing method"
> +       help
> +         A weak extension of traditional DES, which eliminates the
> +         length limit, increases the salt size, and makes the time
> +         cost tunable.
> +
> +         It originates with BSDI and is also available on at least
> +         NetBSD, OpenBSD, FreeBSD, and MacOSX.
> +
> +config LIBXCRYPT_BIGCRYPT
> +       bool
> +       prompt "bigcrypt hashing method"
> +       help
> +         A weak extension of traditional DES, available on some
> +         System V-derived Unixes.  All it does is raise the length
> +         limit from 8 to 128 characters, and it does this in a crude
> +         way that allows attackers to guess chunks of a long passphrase
> +         in parallel.

I don't think we need these compat options. From the help texts, none of
those are needed for compatibility with libcrypt from glibc. So I think we
should just disable this stuff. We can add an option when it's actually
needed.

> +
> +endif
> diff --git a/rules/libxcrypt.make b/rules/libxcrypt.make
> new file mode 100644
> index 000000000..266e42640
> --- /dev/null
> +++ b/rules/libxcrypt.make
> @@ -0,0 +1,95 @@
> +# -*-makefile-*-
> +#
> +# Copyright (C) 2019 by Bjoern Esser <bes@pengutronix.de>
> +#
> +# For further information about the PTXdist project and license conditions
> +# see the README file.
> +#
> +
> +#
> +# We provide this package
> +#
> +PACKAGES-$(PTXCONF_LIBXCRYPT) += libxcrypt
> +
> +#
> +# Paths and names
> +#
> +LIBXCRYPT_VERSION      := 4.4.28
> +LIBXCRYPT_MD5          := 0b873e641ae201e5e7470cf791c0fe16
> +LIBXCRYPT              := libxcrypt-$(LIBXCRYPT_VERSION)
> +LIBXCRYPT_SUFFIX       := tar.xz
> +LIBXCRYPT_URL          := https://github.com/besser82/libxcrypt/releases/download/v$(LIBXCRYPT_VERSION)/$(LIBXCRYPT).$(LIBXCRYPT_SUFFIX)
> +LIBXCRYPT_SOURCE       := $(SRCDIR)/$(LIBXCRYPT).$(LIBXCRYPT_SUFFIX)
> +LIBXCRYPT_DIR          := $(BUILDDIR)/$(LIBXCRYPT)
> +LIBXCRYPT_LICENSE      := LGPL-2.1-or-later AND BSD-3-Clause AND
> BSD-2-Clause AND 0BSD AND public_domain
> +LIBXCRYPT_LICENSE_MD5  :=
> file://LICENSING;md5=3bb6614cf5880cbf1b9dbd9e3d145e2c
> +
> +#
> ----------------------------------------------------------------------------
> +# Prepare
> +#
> ----------------------------------------------------------------------------
> +
> +#
> +# options
> +#
> +
> +# Hash methods enabled by default.
> +HASH_METHODS := glibc,strong
> +
> +ifdef PTXCONF_LIBXCRYPT_BCRYPT_X
> +HASH_METHODS := $(HASH_METHODS),bcrypt_x
> +endif
> +
> +ifdef PTXCONF_LIBXCRYPT_SHA1CRYPT
> +HASH_METHODS := $(HASH_METHODS),sha1crypt
> +endif
> +
> +ifdef PTXCONF_LIBXCRYPT_SUNMD5
> +HASH_METHODS := $(HASH_METHODS),sunmd5
> +endif
> +
> +ifdef PTXCONF_LIBXCRYPT_NTHASH
> +HASH_METHODS := $(HASH_METHODS),nt
> +endif
> +
> +ifdef PTXCONF_LIBXCRYPT_BSDICRYPT
> +HASH_METHODS := $(HASH_METHODS),bsdicrypt
> +endif
> +
> +ifdef PTXCONF_LIBXCRYPT_BIGCRYPT
> +HASH_METHODS := $(HASH_METHODS),bigcrypt
> +endif
> +
> +#
> +# autoconf
> +#
> +LIBXCRYPT_CONF_TOOL    := autoconf
> +LIBXCRYPT_CONF_OPT     := \
> +       $(CROSS_AUTOCONF_USR) \
> +       --disable-failure-tokens \
> +       --disable-static \
> +       --disable-valgrind \
> +       --enable-obsolete-api=$(call
> ptx/ifdef,PTXCONF_LIBXCRYPT_GLIBC_BINARY_COMPAT,glibc,no) \

Hmm, looks like an extra line break introduced by your MUA/MTA.

Michael

> +       --enable-obsolete-api-enosys=$(call
> ptx/ifdef,PTXCONF_LIBXCRYPT_OBSOLETE_STUBS,yes,no) \
> +       --enable-hashes=$(HASH_METHODS) \
> +       --enable-xcrypt-compat-files
> +
> +#
> ----------------------------------------------------------------------------
> +# Target-Install
> +#
> ----------------------------------------------------------------------------
> +
> +$(STATEDIR)/libxcrypt.targetinstall:
> +       @$(call targetinfo)
> +
> +       @$(call install_init, libxcrypt)
> +       @$(call install_fixup, libxcrypt,PRIORITY,optional)
> +       @$(call install_fixup, libxcrypt,SECTION,base)
> +       @$(call install_fixup, libxcrypt,AUTHOR,"Bjoern Esser
> <bes@pengutronix.de>")
> +       @$(call install_fixup, libxcrypt,DESCRIPTION,Extended crypt library)
> +
> +       @$(call install_lib, libxcrypt, 0, 0, 0644, libcrypt)
> +
> +       @$(call install_finish, libxcrypt)
> +
> +       @$(call touch)
> +
> +# vim: syntax=make
> diff --git a/rules/uclibc.in b/rules/uclibc.in
> index 1fa99eba5..ee9cb0f34 100644
> --- a/rules/uclibc.in
> +++ b/rules/uclibc.in
> @@ -24,12 +24,16 @@ config UCLIBC_C
> 
>           Better not turn this option off..
> 
> +if LIBC_CRYPT_NATIVE_CRYPT
> +
>  config UCLIBC_CRYPT
>         bool
>         prompt "Install libcrypt"
>         help
>           The encryption/decryption library
> 
> +endif
> +
>  config UCLIBC_DL
>         bool
>         prompt "Install libdl"
> -- 
> 2.34.1
> 
> 
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |