Bug 244181

Summary: unzip: Add passphrase and GLIBC build support
Product: Base System Reporter: Mingye Wang <arthur200126>
Component: binAssignee: freebsd-bugs mailing list <bugs>
Status: Open ---    
Severity: Affects Some People CC: ak, des, tom, ypankov
Priority: --- Keywords: needs-qa, patch
Version: CURRENTFlags: koobs: maintainer-feedback? (des)
koobs: maintainer-feedback? (ak)
koobs: 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().