| Message ID | 80f93982-dac6-4d4e-b9eb-9a5d09710a9c@app.fastmail.com |
|---|---|
| Headers | show |
| Series | Remove libcrypt | expand |
On 21/09/23 17:48, Zack Weinberg wrote: > In https://sourceware.org/pipermail/libc-alpha/2023-September/151664.html > Adhemerval suggested that now would be a good time to complete the > removal of libcrypt. This patch series does just that. If you would > rather not apply it by hand, you can get it from the zack/remove-libcrypt > branch (with somewhat different commit messages -- I'll fix that > before merging). In fact I also sent patchset to remove libcrypt just a couple of hours before yours [1]. > > Patch #1 was compile-tested on x86_64-linux with both --enable-crypt > and the default --disable-crypt; furthermore, in the --enable-crypt > configuration, I ran "make xcheck subdirs='crypt locale'" with no > failures. Patch #2 was only compile tested. For patch #3, which only > touches the manual, I did 'make info html' and inspected the output. > (I don't have TeX on this computer so I couldn't run 'make pdf'.) > Finally, after patch #4 I ran the complete testsuite ('make check' > only) and found no new failures. 26 tests are failing on my machine > due to probable environment issues, but they were all failing on trunk > before I started making changes, and none of them appear to have > anything to do with this patchset. I really don't see any value of changing the md5 implementation glibc uses on localedef, current implementation does not have any red flag (such as alloca usage), it is used solely for integrity (which in theory could be any other algorithm, albeit changing it might add some compatibility issues); and for this change I think we should the code as similar as possible (avoid any internal API change). I also see no point in adding extra costly md5 tests, since the integrity support is not really implemented in glibc. If you really think changing the md5 implementation for localedef integrity would be really useful, it would be better to be proposed in a different patch. > > stdio-common/Versions says that __snprintf is exported as > GLIBC_PRIVATE because libcrypt uses it. I doubt that was the only > ancillary library using that symbol, but I don't know how to find > other uses, so I left the export in place; it can always be removed > later. The GLIBC_PRIVATE as just used between glibc installed shared objects, so any other external users probing on GLIBC_PRIVATE was never supported. > > There are a few files of test data that mention the crypt directory > and/or its contents, e.g. benchtests/strcoll-inputs/filelist#en_US.UTF-8. > As best I can tell, the intent of these is just to have a big pile of > strings and it doesn't matter whether the named files exist, so I > haven't altered them. > > I seem to have a slightly different version of autoconf than the one > that was last used to regenerate the top-level configure script. If > that's a problem, let me know. > > I deleted the paragraph at the beginning of crypt.texi about legal > restrictions on cryptographic software, because after this patchset > the only cryptographic code in glibc itself will be the MD5 > implementation used by localedef (see first patch in this series), > which is not exposed to users of the library, and the DES > implementation in sunrpc/, which is also slated for removal (right?) > If this paragraph should be preserved, please let me know. > I slight more inclined to go with my patch, mainly because it tries to avoid changing internal APIs and pack the whole removal in only one patch (I also proposed the sparc crypt optimization for last release, but I postpone to send while libcrypt was actually removed). [1] https://patchwork.sourceware.org/project/glibc/list/?series=24938
On Wed, Sep 27, 2023, at 9:58 AM, Adhemerval Zanella Netto wrote: > On 21/09/23 17:48, Zack Weinberg wrote: >> In >> https://sourceware.org/pipermail/libc-alpha/2023-September/151664.html >> Adhemerval suggested that now would be a good time to complete the >> removal of libcrypt. This patch series does just that. If you would >> rather not apply it by hand, you can get it from the zack/remove- >> libcrypt branch (with somewhat different commit messages -- I'll fix >> that before merging). > > In fact I also sent patchset to remove libcrypt just a couple of hours > before yours [1]. Yes, I saw that only after posting my patch set. I didn't realize, from your message saying that you thought it was a good time to proceed, that you intended to work on it yourself. > I really don't see any value of changing the md5 implementation glibc > uses on localedef, current implementation does not have any red flag > (such as alloca usage), it is used solely for integrity (which in > theory could be any other algorithm, albeit changing it might add some > compatibility issues); and for this change I think we should the code > as similar as possible (avoid any internal API change). Fair enough. > I also see no point in adding extra costly md5 tests, since the > integrity support is not really implemented in glibc. That test isn't new, I just moved it from the crypt directory. It looks like your patch leaves md5.c in the crypt directory? What did you do with the tests in there? >> stdio-common/Versions says that __snprintf is exported as >> GLIBC_PRIVATE because libcrypt uses it. I doubt that was the only >> ancillary library using that symbol, but I don't know how to find >> other uses, so I left the export in place; it can always be >> removed later. > > The GLIBC_PRIVATE as just used between glibc installed shared objects, > so any other external users probing on GLIBC_PRIVATE was never > supported. I meant, I don't know how to check whether there are any other uses of __snprintf@GLIBC_PRIVATE *within glibc* after removing libcrypt. > I slight more inclined to go with my patch, mainly because it tries to > avoid changing internal APIs and pack the whole removal in only one > patch (I also proposed the sparc crypt optimization for last release, > but I postpone to send while libcrypt was actually removed). The only piece of my patchset that I think is really worth keeping, is the changes I made to the documentation. How about you go ahead and merge your patchset and then I'll resubmit the doc changes on top of that? Also I'll send a patch moving md5.c to locale/ so we can completely remove the crypt directory (one fewer top-level directory to walk over in the build makes a measurable difference in how long a null incremental build takes). zw
On 27/09/23 15:05, Zack Weinberg wrote: > On Wed, Sep 27, 2023, at 9:58 AM, Adhemerval Zanella Netto wrote: >> On 21/09/23 17:48, Zack Weinberg wrote: >>> In >>> https://sourceware.org/pipermail/libc-alpha/2023-September/151664.html >>> Adhemerval suggested that now would be a good time to complete the >>> removal of libcrypt. This patch series does just that. If you would >>> rather not apply it by hand, you can get it from the zack/remove- >>> libcrypt branch (with somewhat different commit messages -- I'll fix >>> that before merging). >> >> In fact I also sent patchset to remove libcrypt just a couple of hours >> before yours [1]. > > Yes, I saw that only after posting my patch set. I didn't realize, from > your message saying that you thought it was a good time to proceed, that > you intended to work on it yourself. > >> I really don't see any value of changing the md5 implementation glibc >> uses on localedef, current implementation does not have any red flag >> (such as alloca usage), it is used solely for integrity (which in >> theory could be any other algorithm, albeit changing it might add some >> compatibility issues); and for this change I think we should the code >> as similar as possible (avoid any internal API change). > > Fair enough. > >> I also see no point in adding extra costly md5 tests, since the >> integrity support is not really implemented in glibc. > > That test isn't new, I just moved it from the crypt directory. It looks > like your patch leaves md5.c in the crypt directory? What did you do > with the tests in there? I removed them entirely, my patch moves the md5.c and the md5-block.c to locale/programs (with some minor changes to remove the md5_stream function that is not used by localedef) since the integrity check is not really used anywhere. > >>> stdio-common/Versions says that __snprintf is exported as >>> GLIBC_PRIVATE because libcrypt uses it. I doubt that was the only >>> ancillary library using that symbol, but I don't know how to find >>> other uses, so I left the export in place; it can always be >>> removed later. >> >> The GLIBC_PRIVATE as just used between glibc installed shared objects, >> so any other external users probing on GLIBC_PRIVATE was never >> supported. > > I meant, I don't know how to check whether there are any other uses of > __snprintf@GLIBC_PRIVATE *within glibc* after removing libcrypt. Afaiu it should be caught in built time if any other shared library still uses it. > >> I slight more inclined to go with my patch, mainly because it tries to >> avoid changing internal APIs and pack the whole removal in only one >> patch (I also proposed the sparc crypt optimization for last release, >> but I postpone to send while libcrypt was actually removed). > > The only piece of my patchset that I think is really worth keeping, is > the changes I made to the documentation. How about you go ahead and > merge your patchset and then I'll resubmit the doc changes on top of > that? Also I'll send a patch moving md5.c to locale/ so we can > completely remove the crypt directory (one fewer top-level directory to > walk over in the build makes a measurable difference in how long a null > incremental build takes). Joseph asked to remove the shlib-versions entries, so what about I add your documentation on top my patch and send a v2?
On Wed, Sep 27, 2023, at 2:19 PM, Adhemerval Zanella Netto wrote: >> The only piece of my patchset that I think is really worth keeping, is >> the changes I made to the documentation. How about you go ahead and >> merge your patchset and then I'll resubmit the doc changes on top of >> that? Also I'll send a patch moving md5.c to locale/ so we can >> completely remove the crypt directory (one fewer top-level directory to >> walk over in the build makes a measurable difference in how long a null >> incremental build takes). > > Joseph asked to remove the shlib-versions entries, so what about I add > your documentation on top my patch and send a v2? If you don't mind doing that work, please go ahead. zw
In https://sourceware.org/pipermail/libc-alpha/2023-September/151664.html Adhemerval suggested that now would be a good time to complete the removal of libcrypt. This patch series does just that. If you would rather not apply it by hand, you can get it from the zack/remove-libcrypt branch (with somewhat different commit messages -- I'll fix that before merging). Patch #1 was compile-tested on x86_64-linux with both --enable-crypt and the default --disable-crypt; furthermore, in the --enable-crypt configuration, I ran "make xcheck subdirs='crypt locale'" with no failures. Patch #2 was only compile tested. For patch #3, which only touches the manual, I did 'make info html' and inspected the output. (I don't have TeX on this computer so I couldn't run 'make pdf'.) Finally, after patch #4 I ran the complete testsuite ('make check' only) and found no new failures. 26 tests are failing on my machine due to probable environment issues, but they were all failing on trunk before I started making changes, and none of them appear to have anything to do with this patchset. stdio-common/Versions says that __snprintf is exported as GLIBC_PRIVATE because libcrypt uses it. I doubt that was the only ancillary library using that symbol, but I don't know how to find other uses, so I left the export in place; it can always be removed later. There are a few files of test data that mention the crypt directory and/or its contents, e.g. benchtests/strcoll-inputs/filelist#en_US.UTF-8. As best I can tell, the intent of these is just to have a big pile of strings and it doesn't matter whether the named files exist, so I haven't altered them. I seem to have a slightly different version of autoconf than the one that was last used to regenerate the top-level configure script. If that's a problem, let me know. I deleted the paragraph at the beginning of crypt.texi about legal restrictions on cryptographic software, because after this patchset the only cryptographic code in glibc itself will be the MD5 implementation used by localedef (see first patch in this series), which is not exposed to users of the library, and the DES implementation in sunrpc/, which is also slated for removal (right?) If this paragraph should be preserved, please let me know. Zack Weinberg (4): Import Solar Designer's public domain MD5 for use by localedef. Remove --enable-crypt and --enable-nss-crypt configure options. Remove documentation of passphrase-hashing functions. Remove all of the remaining libcrypt code. CONTRIBUTED-BY | 15 - INSTALL | 13 - Makeconfig | 5 - NEWS | 16 +- SHARED-FILES | 2 - config.make.in | 4 - configure | 247 ++--- configure.ac | 64 -- conform/Makefile | 5 - crypt/Makefile | 69 -- crypt/README.ufc-crypt | 135 --- crypt/Versions | 5 - crypt/badsalttest.c | 54 - crypt/cert.c | 135 --- crypt/cert.input | 171 ---- crypt/crypt-entry.c | 183 ---- crypt/crypt-private.h | 76 -- crypt/crypt.c | 115 --- crypt/crypt.h | 70 -- crypt/crypt_util.c | 946 ------------------ crypt/md5-block.c | 166 --- crypt/md5-crypt.c | 331 ------ crypt/md5.c | 257 ----- crypt/md5.h | 146 --- crypt/md5c-test.c | 18 - crypt/sha256-block.c | 98 -- crypt/sha256-crypt.c | 423 -------- crypt/sha256.c | 193 ---- crypt/sha256.h | 69 -- crypt/sha256c-test.c | 61 -- crypt/sha256test.c | 102 -- crypt/sha512-block.c | 105 -- crypt/sha512-crypt.c | 445 -------- crypt/sha512.c | 221 ---- crypt/sha512.h | 72 -- crypt/sha512c-test.c | 63 -- crypt/sha512test.c | 113 --- crypt/speeds.c | 153 --- crypt/ufc-crypt.h | 28 - crypt/ufc.c | 54 - elf/Makefile | 38 - elf/tst-linkall-static.c | 6 - include/crypt.h | 3 - locale/Makefile | 18 +- locale/locarchive.h | 4 +- locale/md5.c | 281 ++++++ locale/md5.h | 45 + locale/programs/locarchive.c | 10 +- locale/programs/locfile.c | 6 +- .../md5test-giant.c => locale/tst-md5-giant.c | 43 +- crypt/md5test.c => locale/tst-md5.c | 22 +- manual/contrib.texi | 2 +- manual/crypt.texi | 234 +---- manual/examples/genpass.c | 59 -- manual/examples/testpass.c | 67 -- manual/install.texi | 13 - manual/users.texi | 4 +- posix/unistd.h | 10 - scripts/build-many-glibcs.py | 9 +- scripts/documented.sh | 2 +- shlib-versions | 3 - stdio-common/Versions | 2 +- sysdeps/generic/fips-private.h | 36 - sysdeps/generic/libcrypt.abilist | 0 sysdeps/mach/Makefile | 4 +- sysdeps/mach/hurd/i386/libcrypt.abilist | 7 - sysdeps/mach/hurd/x86_64/libcrypt.abilist | 2 - .../sparc/sparc32/sparcv9/multiarch/Makefile | 8 - .../sparc32/sparcv9/multiarch/md5-block.c | 1 - .../sparc32/sparcv9/multiarch/md5-crop.S | 1 - .../sparc32/sparcv9/multiarch/sha256-block.c | 1 - .../sparc32/sparcv9/multiarch/sha256-crop.S | 1 - .../sparc32/sparcv9/multiarch/sha512-block.c | 1 - .../sparc32/sparcv9/multiarch/sha512-crop.S | 1 - sysdeps/sparc/sparc64/multiarch/Makefile | 8 - sysdeps/sparc/sparc64/multiarch/md5-block.c | 29 - sysdeps/sparc/sparc64/multiarch/md5-crop.S | 109 -- .../sparc/sparc64/multiarch/sha256-block.c | 32 - sysdeps/sparc/sparc64/multiarch/sha256-crop.S | 100 -- .../sparc/sparc64/multiarch/sha512-block.c | 32 - sysdeps/sparc/sparc64/multiarch/sha512-crop.S | 130 --- .../unix/sysv/linux/aarch64/libcrypt.abilist | 7 - .../unix/sysv/linux/alpha/libcrypt.abilist | 7 - sysdeps/unix/sysv/linux/alpha/shlib-versions | 1 - sysdeps/unix/sysv/linux/arc/libcrypt.abilist | 2 - sysdeps/unix/sysv/linux/arm/Makefile | 4 - .../unix/sysv/linux/arm/be/libcrypt.abilist | 7 - .../unix/sysv/linux/arm/le/libcrypt.abilist | 7 - sysdeps/unix/sysv/linux/csky/libcrypt.abilist | 2 - sysdeps/unix/sysv/linux/fips-private.h | 74 -- sysdeps/unix/sysv/linux/hppa/libcrypt.abilist | 7 - sysdeps/unix/sysv/linux/i386/libcrypt.abilist | 7 - sysdeps/unix/sysv/linux/ia64/libcrypt.abilist | 7 - .../linux/loongarch/lp64/libcrypt.abilist | 2 - .../sysv/linux/m68k/coldfire/libcrypt.abilist | 7 - .../sysv/linux/m68k/m680x0/libcrypt.abilist | 7 - .../sysv/linux/microblaze/be/libcrypt.abilist | 7 - .../sysv/linux/microblaze/le/libcrypt.abilist | 7 - .../sysv/linux/mips/mips32/libcrypt.abilist | 7 - .../sysv/linux/mips/mips64/libcrypt.abilist | 7 - .../unix/sysv/linux/nios2/libcrypt.abilist | 7 - sysdeps/unix/sysv/linux/or1k/libcrypt.abilist | 2 - .../linux/powerpc/powerpc32/libcrypt.abilist | 7 - .../powerpc/powerpc64/be/libcrypt.abilist | 7 - .../powerpc/powerpc64/le/libcrypt.abilist | 7 - .../sysv/linux/riscv/rv32/libcrypt.abilist | 2 - .../sysv/linux/riscv/rv64/libcrypt.abilist | 7 - .../sysv/linux/s390/s390-32/libcrypt.abilist | 7 - .../sysv/linux/s390/s390-64/libcrypt.abilist | 7 - .../unix/sysv/linux/sh/be/libcrypt.abilist | 7 - .../unix/sysv/linux/sh/le/libcrypt.abilist | 7 - .../sysv/linux/sparc/sparc32/libcrypt.abilist | 7 - .../sysv/linux/sparc/sparc64/libcrypt.abilist | 7 - .../sysv/linux/x86_64/64/libcrypt.abilist | 7 - .../sysv/linux/x86_64/x32/libcrypt.abilist | 7 - 115 files changed, 492 insertions(+), 6611 deletions(-) delete mode 100644 crypt/Makefile delete mode 100644 crypt/README.ufc-crypt delete mode 100644 crypt/Versions delete mode 100644 crypt/badsalttest.c delete mode 100644 crypt/cert.c delete mode 100644 crypt/cert.input delete mode 100644 crypt/crypt-entry.c delete mode 100644 crypt/crypt-private.h delete mode 100644 crypt/crypt.c delete mode 100644 crypt/crypt.h delete mode 100644 crypt/crypt_util.c delete mode 100644 crypt/md5-block.c delete mode 100644 crypt/md5-crypt.c delete mode 100644 crypt/md5.c delete mode 100644 crypt/md5.h delete mode 100644 crypt/md5c-test.c delete mode 100644 crypt/sha256-block.c delete mode 100644 crypt/sha256-crypt.c delete mode 100644 crypt/sha256.c delete mode 100644 crypt/sha256.h delete mode 100644 crypt/sha256c-test.c delete mode 100644 crypt/sha256test.c delete mode 100644 crypt/sha512-block.c delete mode 100644 crypt/sha512-crypt.c delete mode 100644 crypt/sha512.c delete mode 100644 crypt/sha512.h delete mode 100644 crypt/sha512c-test.c delete mode 100644 crypt/sha512test.c delete mode 100644 crypt/speeds.c delete mode 100644 crypt/ufc-crypt.h delete mode 100644 crypt/ufc.c delete mode 100644 include/crypt.h create mode 100644 locale/md5.c create mode 100644 locale/md5.h rename crypt/md5test-giant.c => locale/tst-md5-giant.c (77%) rename crypt/md5test.c => locale/tst-md5.c (78%) delete mode 100644 manual/examples/genpass.c delete mode 100644 manual/examples/testpass.c delete mode 100644 sysdeps/generic/fips-private.h delete mode 100644 sysdeps/generic/libcrypt.abilist delete mode 100644 sysdeps/mach/hurd/i386/libcrypt.abilist delete mode 100644 sysdeps/mach/hurd/x86_64/libcrypt.abilist delete mode 100644 sysdeps/sparc/sparc32/sparcv9/multiarch/md5-block.c delete mode 100644 sysdeps/sparc/sparc32/sparcv9/multiarch/md5-crop.S delete mode 100644 sysdeps/sparc/sparc32/sparcv9/multiarch/sha256-block.c delete mode 100644 sysdeps/sparc/sparc32/sparcv9/multiarch/sha256-crop.S delete mode 100644 sysdeps/sparc/sparc32/sparcv9/multiarch/sha512-block.c delete mode 100644 sysdeps/sparc/sparc32/sparcv9/multiarch/sha512-crop.S delete mode 100644 sysdeps/sparc/sparc64/multiarch/md5-block.c delete mode 100644 sysdeps/sparc/sparc64/multiarch/md5-crop.S delete mode 100644 sysdeps/sparc/sparc64/multiarch/sha256-block.c delete mode 100644 sysdeps/sparc/sparc64/multiarch/sha256-crop.S delete mode 100644 sysdeps/sparc/sparc64/multiarch/sha512-block.c delete mode 100644 sysdeps/sparc/sparc64/multiarch/sha512-crop.S delete mode 100644 sysdeps/unix/sysv/linux/aarch64/libcrypt.abilist delete mode 100644 sysdeps/unix/sysv/linux/alpha/libcrypt.abilist delete mode 100644 sysdeps/unix/sysv/linux/arc/libcrypt.abilist delete mode 100644 sysdeps/unix/sysv/linux/arm/be/libcrypt.abilist delete mode 100644 sysdeps/unix/sysv/linux/arm/le/libcrypt.abilist delete mode 100644 sysdeps/unix/sysv/linux/csky/libcrypt.abilist delete mode 100644 sysdeps/unix/sysv/linux/fips-private.h delete mode 100644 sysdeps/unix/sysv/linux/hppa/libcrypt.abilist delete mode 100644 sysdeps/unix/sysv/linux/i386/libcrypt.abilist delete mode 100644 sysdeps/unix/sysv/linux/ia64/libcrypt.abilist delete mode 100644 sysdeps/unix/sysv/linux/loongarch/lp64/libcrypt.abilist delete mode 100644 sysdeps/unix/sysv/linux/m68k/coldfire/libcrypt.abilist delete mode 100644 sysdeps/unix/sysv/linux/m68k/m680x0/libcrypt.abilist delete mode 100644 sysdeps/unix/sysv/linux/microblaze/be/libcrypt.abilist delete mode 100644 sysdeps/unix/sysv/linux/microblaze/le/libcrypt.abilist delete mode 100644 sysdeps/unix/sysv/linux/mips/mips32/libcrypt.abilist delete mode 100644 sysdeps/unix/sysv/linux/mips/mips64/libcrypt.abilist delete mode 100644 sysdeps/unix/sysv/linux/nios2/libcrypt.abilist delete mode 100644 sysdeps/unix/sysv/linux/or1k/libcrypt.abilist delete mode 100644 sysdeps/unix/sysv/linux/powerpc/powerpc32/libcrypt.abilist delete mode 100644 sysdeps/unix/sysv/linux/powerpc/powerpc64/be/libcrypt.abilist delete mode 100644 sysdeps/unix/sysv/linux/powerpc/powerpc64/le/libcrypt.abilist delete mode 100644 sysdeps/unix/sysv/linux/riscv/rv32/libcrypt.abilist delete mode 100644 sysdeps/unix/sysv/linux/riscv/rv64/libcrypt.abilist delete mode 100644 sysdeps/unix/sysv/linux/s390/s390-32/libcrypt.abilist delete mode 100644 sysdeps/unix/sysv/linux/s390/s390-64/libcrypt.abilist delete mode 100644 sysdeps/unix/sysv/linux/sh/be/libcrypt.abilist delete mode 100644 sysdeps/unix/sysv/linux/sh/le/libcrypt.abilist delete mode 100644 sysdeps/unix/sysv/linux/sparc/sparc32/libcrypt.abilist delete mode 100644 sysdeps/unix/sysv/linux/sparc/sparc64/libcrypt.abilist delete mode 100644 sysdeps/unix/sysv/linux/x86_64/64/libcrypt.abilist delete mode 100644 sysdeps/unix/sysv/linux/x86_64/x32/libcrypt.abilist