Bug 244181

Summary: unzip: Add passphrase and GLIBC build support
Product: Base System Reporter: Mingye Wang <arthur200126>
Component: binAssignee: Yoshihiro Takahashi <nyan>
Status: Closed FIXED    
Severity: Affects Some People CC: ak, des, nyan, tom, ypankov
Priority: --- Keywords: needs-qa, patch
Version: CURRENTFlags: koobs: maintainer-feedback? (des)
koobs: maintainer-feedback? (ak)
nyan: mfc-stable13+
nyan: mfc-stable12+
koobs: mfc-stable11?
Hardware: Any   
OS: Any   
See Also: https://github.com/libarchive/libarchive/issues/1138
Attachments:
Description Flags
Support glibc builds
none
Add passphrase support
none
Mention ZIPX in unzip(1)
none
Add passphrase support (v2)
none
Add passphrase support (v3)
none
Add passphrase support none

Description Mingye Wang 2020-02-17 03:37:56 UTC
Created attachment 211703 [details]
Support glibc builds

I would like to add the attached series of patches to unzip:

1. glibc support, since some people on libarchive's GitHub page is showing interest in this replacement util.
2. passsphrase support, both in the unsafe command-line entry form (-P) and in the safe getpassphrase(3) form.
3. minor edit to the manpage, just to remind people that we can decompress fancy ZIPX archives if libarchive can.

I thought about adding the filename iconv stuff from info-zip's unzip beta 6.10b, but doing stuff like iconv_open is a bit too much for me now. It *is* a nice feature though, at least according to the Linux users who came up with unzip-iconv in the first place. I might just add it sometime.

The patch is based against FreeBSD GitHub src tree. The last commit in usr.bin/unzip should be 7551d83.
Comment 1 Mingye Wang 2020-02-17 03:38:21 UTC
Created attachment 211704 [details]
Add passphrase support
Comment 2 Mingye Wang 2020-02-17 03:38:46 UTC
Created attachment 211705 [details]
Mention ZIPX in unzip(1)
Comment 3 Alex Kozlov freebsd_committer 2020-02-17 14:10:27 UTC
The 'support glibc' and 'mention zipx' patches looks alright, but I think you forgot to free passphrase_buf in 'add passphrase' patch.
Comment 4 Mingye Wang 2020-02-18 09:26:00 UTC
Created attachment 211738 [details]
Add passphrase support (v2)

Oops! Here is the patch with the free() added.
Comment 5 Mingye Wang 2020-02-18 10:38:21 UTC
Expect a passphrase v3 -- it simply doesn't make sense to free in main() when the only cause of malloc() is planted by unzip().
Comment 6 Yuri Pankov freebsd_committer 2020-02-18 11:14:03 UTC
You might want to open this at https://reviews.freebsd.org so it's easier to comment on the code.
Comment 7 Mingye Wang 2020-02-19 03:50:13 UTC
Created attachment 211753 [details]
Add passphrase support (v3)
Comment 8 Alex Kozlov freebsd_committer 2020-03-24 09:30:50 UTC
Created attachment 212674 [details]
Add passphrase support

It seems that unzip fails to build with the last passphrase support patch. I fixed it a little, please check it out.
Comment 9 Thomas Hurst 2020-04-10 15:32:12 UTC
+		memset(passphrase_buf, 0, PPBUFF_SIZE);
+		free(passphrase_buf);

This should probably be using explicit_bzero() to prevent the compiler from optimizing it away.

tar(1) has the same issue.
Comment 10 Alex Kozlov freebsd_committer 2020-04-12 18:59:38 UTC
The explicit_bzero(3) is not portable. So it can't be used in libarchive and it's also absent in NetBSD, which is (still ?) upstream for our unzip.
Comment 11 Yuri Pankov 2020-04-12 19:43:13 UTC
What about memset_s(), which is in C11?
Comment 12 Thomas Hurst 2020-04-13 00:06:58 UTC
Looks like NetBSD wants explicit_memset(), which also isn't portable.
Comment 13 Conrad Meyer freebsd_committer 2020-04-13 00:51:25 UTC
readpassphrase() in the same patch is no more portable than explicit_bzero(), so you should just use explicit_bzero.
Comment 14 Alex Kozlov freebsd_committer 2020-04-13 06:57:26 UTC
Yes, memset_s() should work.

The libarchive's cpio and bsdtar use portable version from libarchive_fe if necessary (see use.bin/tar/Makefile). I left readpassphrase() because Free/Open/NetBSD have it. Can be easily changed back to lafe_readpassphrase().
Comment 15 Yoshihiro Takahashi freebsd_committer 2021-02-23 13:44:47 UTC
*** Bug 253726 has been marked as a duplicate of this bug. ***
Comment 16 Yoshihiro Takahashi freebsd_committer 2021-02-23 14:01:56 UTC
I found a problem with your patch to add passphrase support.
In passphrase_callback(), passphrase_buf is (re)defined as a local variable,
so the global passphrase_buf is not changed here.

I have fixed it.  Please see https://reviews.freebsd.org/D28892

BTW, I except GLIBC part at this time.  I think that it should be done by
libarchive's repository first.
Comment 17 Alex Kozlov freebsd_committer 2021-02-23 15:02:45 UTC
Thanks. That was silly braino. As for GLIBC, as I wrote in 253726, it needs more work and testing. The NetBSD opted for use defines with getpass/getpass_r/readpassphrase but I'm not sure it's the best variant or even if it will work on all/most linux distros.
Comment 18 commit-hook freebsd_committer 2021-09-25 16:39:41 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=a4724ff48108840416c83f10e15d666ac8d78937

commit a4724ff48108840416c83f10e15d666ac8d78937
Author:     Yoshihiro Takahashi <nyan@FreeBSD.org>
AuthorDate: 2021-09-25 16:32:42 +0000
Commit:     Yoshihiro Takahashi <nyan@FreeBSD.org>
CommitDate: 2021-09-25 16:32:42 +0000

    unzip: sync with NetBSD upstream to add passphrase support

    - Add support for password protected zip archives.
      We use memset_s() rather than explicit_bzero() for more portable
      (See PR).
    - Use success/failure macro in exit()
    - Mention ZIPX format in unzip(1)

    Submitted by:   Mingye Wang and Alex Kozlov (ak@)
    PR:             244181
    Reviewed by:    mizhka
    Obtained from:  NetBSD
    MFC after:      1 week
    Differential Revision:  https://reviews.freebsd.org/D28892

 usr.bin/unzip/unzip.1 | 10 ++++++--
 usr.bin/unzip/unzip.c | 66 ++++++++++++++++++++++++++++++++++++++++++++-------
 2 files changed, 66 insertions(+), 10 deletions(-)
Comment 19 commit-hook freebsd_committer 2021-10-02 11:57:53 UTC
A commit in branch stable/13 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=cea130c0b8f5a32b0e4a22befb89bad73f8663c2

commit cea130c0b8f5a32b0e4a22befb89bad73f8663c2
Author:     Yoshihiro Takahashi <nyan@FreeBSD.org>
AuthorDate: 2021-09-25 16:32:42 +0000
Commit:     Yoshihiro Takahashi <nyan@FreeBSD.org>
CommitDate: 2021-10-02 11:56:19 +0000

    unzip: sync with NetBSD upstream to add passphrase support

    - Add support for password protected zip archives.
      We use memset_s() rather than explicit_bzero() for more portable
      (See PR).
    - Use success/failure macro in exit()
    - Mention ZIPX format in unzip(1)

    Submitted by:   Mingye Wang and Alex Kozlov (ak@)
    PR:             244181
    Reviewed by:    mizhka
    Obtained from:  NetBSD
    Differential Revision:  https://reviews.freebsd.org/D28892

    (cherry picked from commit a4724ff48108840416c83f10e15d666ac8d78937)

 usr.bin/unzip/unzip.1 | 10 ++++++--
 usr.bin/unzip/unzip.c | 66 ++++++++++++++++++++++++++++++++++++++++++++-------
 2 files changed, 66 insertions(+), 10 deletions(-)
Comment 20 commit-hook freebsd_committer 2021-10-02 11:58:55 UTC
A commit in branch stable/12 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=970c3982cd6a05efe9b4666a8a7f98670f18f36e

commit 970c3982cd6a05efe9b4666a8a7f98670f18f36e
Author:     Yoshihiro Takahashi <nyan@FreeBSD.org>
AuthorDate: 2021-09-25 16:32:42 +0000
Commit:     Yoshihiro Takahashi <nyan@FreeBSD.org>
CommitDate: 2021-10-02 11:57:24 +0000

    unzip: sync with NetBSD upstream to add passphrase support

    - Add support for password protected zip archives.
      We use memset_s() rather than explicit_bzero() for more portable
      (See PR).
    - Use success/failure macro in exit()
    - Mention ZIPX format in unzip(1)

    Submitted by:   Mingye Wang and Alex Kozlov (ak@)
    PR:             244181
    Reviewed by:    mizhka
    Obtained from:  NetBSD
    Differential Revision:  https://reviews.freebsd.org/D28892

    (cherry picked from commit a4724ff48108840416c83f10e15d666ac8d78937)

 usr.bin/unzip/unzip.1 | 10 ++++++--
 usr.bin/unzip/unzip.c | 66 ++++++++++++++++++++++++++++++++++++++++++++-------
 2 files changed, 66 insertions(+), 10 deletions(-)
Comment 21 Yoshihiro Takahashi freebsd_committer 2021-10-02 12:01:55 UTC
Committed.