[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#773861: signify-openbsd



On 09/01/15 09:02, Eriberto wrote:
> Hi Riley, how are you?

Good! How are you? Thank you for the review.

> I reviewed your package. My considerations:
> 
> 1. d/changelog: upload to experimental, to honor the freeze policy[1].
> 
> [1] https://release.debian.org/jessie/freeze_policy.html

Done.

> 2. d/control: please, be more specific in Homepage field.

Changed to https://github.com/aperezdc/signify

> 3. d/copyright:
> 
> - Please, use 'ISC' only, not generic or official. I understood your
> POV. But all licenses are ISC (even with some differences).

Which ISC license text should I keep?

> - In base64.c we have:
> 
> base64.c: * Portions Copyright (c) 1995 by International Business Machines, Inc.
> 
> This uses the IBM license.

Added to d/copyright.

> - debian/patches/move-manpage.patch have copyright!!!!
> 
> debian/patches/move-manpage.patch:+.\"Copyright (c) 2013 Marc Espie
> <espie@openbsd.org>
> debian/patches/move-manpage.patch:+.\"Copyright (c) 2013 Ted Unangst
> <tedu@openbsd.org>
> 
> You must quote the copyright in d/copyright.

This patch no longer exists, after a recommendation from Jakub Wilk
against using patches to move files.

> - I think that if you want to use 'Files: * / Copyright: 2014 Adrian
> Perez <aperez@igalia.com>', you need to consider the GitHub as main
> site (in d/control too).

I've done it in d/control, but where am I supposed to change the site in
d/copyright?

> - In crypto_api.h, where you found the 2014 year? I found 2013.

Good pickup. I've fixed this in the new d/copyright.

> - In fe25519.c and others (in the same line), the year isn't 2013. I
> think that the year is undetermined. Can you check?

The version of supercop that fe25519.c (and others) were taken from was
released in 2013. However, you are right in saying that these files were
in previous years' versions. It would be difficult to find the exact
year; each older version of supercop would need to be downloaded. Should
I remove the year?

> - In smult_curve25519_ref.c, where you found the year for D. J.?

I didn't find a year, so I thought it would be okay to assume that D. J.
had the same copyright year as Matthew Dempsky. Should I remove the year?

> - Please, add an ENTER in end of file.

Done.

> 4. d/patches/rename-manual.patch: I think that the description must be
> 'from "signify"', not 'to "signify"'.

Even with "from", the sentence is still grammatically correct, but I
admit that it is confusing, so I've replaced 'to "signify"' with 'from
"signify"'

> 5. blhc --all produces:
> 
> # blhc --all signify-openbsd_8-1_amd64.build
> 
> CFLAGS missing (-fPIE): cc -g -O2 -fstack-protector-strong -Wformat
> -Werror=format-security   -Wall -D_FORTIFY_SOURCE=2
> -D'__bounded__(a,b,c)='  -c -o crypto_api.o crypto_api.c
> CFLAGS missing (-fPIE): cc -g -O2 -fstack-protector-strong -Wformat
> -Werror=format-security   -Wall -D_FORTIFY_SOURCE=2
> -D'__bounded__(a,b,c)='  -c -o mod_ed25519.o mod_ed25519.c
> CFLAGS missing (-fPIE): cc -g -O2 -fstack-protector-strong -Wformat
> -Werror=format-security   -Wall -D_FORTIFY_SOURCE=2
> -D'__bounded__(a,b,c)='  -c -o mod_ge25519.o mod_ge25519.c
> CFLAGS missing (-fPIE): cc -g -O2 -fstack-protector-strong -Wformat
> -Werror=format-security   -Wall -D_FORTIFY_SOURCE=2
> -D'__bounded__(a,b,c)='  -c -o fe25519.o fe25519.c
> CFLAGS missing (-fPIE): cc -g -O2 -fstack-protector-strong -Wformat
> -Werror=format-security   -Wall -D_FORTIFY_SOURCE=2
> -D'__bounded__(a,b,c)='  -c -o sc25519.o sc25519.c
> CFLAGS missing (-fPIE): cc -g -O2 -fstack-protector-strong -Wformat
> -Werror=format-security   -Wall -D_FORTIFY_SOURCE=2
> -D'__bounded__(a,b,c)='  -c -o smult_curve25519_ref.o
> smult_curve25519_ref.c
> CFLAGS missing (-fPIE): cc -g -O2 -fstack-protector-strong -Wformat
> -Werror=format-security   -Wall -D_FORTIFY_SOURCE=2
> -D'__bounded__(a,b,c)='  -c -o bcrypt_pbkdf.o bcrypt_pbkdf.c
> CFLAGS missing (-fPIE): cc -g -O2 -fstack-protector-strong -Wformat
> -Werror=format-security   -Wall -D_FORTIFY_SOURCE=2
> -D'__bounded__(a,b,c)='  -c -o timingsafe_bcmp.o timingsafe_bcmp.c
> CFLAGS missing (-fPIE): cc -g -O2 -fstack-protector-strong -Wformat
> -Werror=format-security   -Wall -D_FORTIFY_SOURCE=2
> -D'__bounded__(a,b,c)='  -c -o explicit_bzero.o explicit_bzero.c
> CFLAGS missing (-fPIE): cc -g -O2 -fstack-protector-strong -Wformat
> -Werror=format-security   -Wall -D_FORTIFY_SOURCE=2
> -D'__bounded__(a,b,c)='  -c -o blowfish.o blowfish.c
> CFLAGS missing (-fPIE): cc -g -O2 -fstack-protector-strong -Wformat
> -Werror=format-security   -Wall -D_FORTIFY_SOURCE=2
> -D'__bounded__(a,b,c)='  -c -o base64.o base64.c
> CFLAGS missing (-fPIE): cc -g -O2 -fstack-protector-strong -Wformat
> -Werror=format-security   -Wall -D_FORTIFY_SOURCE=2
> -D'__bounded__(a,b,c)='  -c -o sha2.o sha2.c
> CFLAGS missing (-fPIE): cc -g -O2 -fstack-protector-strong -Wformat
> -Werror=format-security   -Wall -D_FORTIFY_SOURCE=2
> -D'__bounded__(a,b,c)='  -c -o sha256hl.o sha256hl.c
> CFLAGS missing (-fPIE): cc -g -O2 -fstack-protector-strong -Wformat
> -Werror=format-security   -Wall -D_FORTIFY_SOURCE=2
> -D'__bounded__(a,b,c)='  -c -o sha512hl.o sha512hl.c
> CFLAGS missing (-fPIE): cc -g -O2 -fstack-protector-strong -Wformat
> -Werror=format-security   -Wall -D_FORTIFY_SOURCE=2
> -D'__bounded__(a,b,c)='  -c -o signify.o signify.c
> CFLAGS missing (-fPIE): cc -g -O2 -fstack-protector-strong -Wformat
> -Werror=format-security   -Wall -D_FORTIFY_SOURCE=2
> -D'__bounded__(a,b,c)='  -c -o ohash.o ohash.c
> LDFLAGS missing (-fPIE -pie -Wl,-z,now): cc -Wl,-z,relro  -o
> signify-openbsd crypto_api.o mod_ed25519.o mod_ge25519.o fe25519.o
> sc25519.o smult_curve25519_ref.o bcrypt_pbkdf.o timingsafe_bcmp.o
> explicit_bzero.o blowfish.o base64.o sha2.o sha256hl.o sha512hl.o
> signify.o ohash.o -lbsd
> LDFLAGS missing (-fPIE -pie -Wl,-z,now): cc -Wl,-z,relro  -o
> signify-openbsd crypto_api.o mod_ed25519.o mod_ge25519.o fe25519.o
> sc25519.o smult_curve25519_ref.o bcrypt_pbkdf.o timingsafe_bcmp.o
> explicit_bzero.o blowfish.o base64.o sha2.o sha256hl.o sha512hl.o
> signify.o ohash.o -lbsd
> 
> 
> To fix, add to d/rules[2]:
> 
> export DEB_BUILD_MAINT_OPTIONS = hardening=+all
> 
> [2] https://wiki.debian.org/HardeningWalkthrough#Selecting_security_hardening_options

Done. blhc now produces no output.

> 6. Your package doesn't build in a fresh jail. I tested using cowbuilder:
> 
> # cowbuilder --build /jaula-sid/sponsor/signify-openbsd/signify-openbsd_8-1.dsc
> 
> [...]
> 
> make[1]: Entering directory '/tmp/buildd/signify-openbsd-8'
> /bin/sh: 1: pkg-config: not found
> Makefile:52: *** libbsd is not installed or version is older than 0.7.  Stop.
> make[1]: Leaving directory '/tmp/buildd/signify-openbsd-8'
> dh_auto_clean: make -j1 distclean returned exit code 2
> debian/rules:6: recipe for target 'clean' failed
> make: *** [clean] Error 2
> dpkg-buildpackage: error: fakeroot debian/rules clean gave error exit status 2
> E: Failed autobuilding of package
> 
> 
> You need to add pkg-config to Build-Depends.

Done. The package now builds in a cowbuilder chroot.


Reply to: