Bug#982705: openssh: FTBFS: sha2.h:57:16: error: redefinition of ‘struct _SHA2_CTX’
On Mon, Feb 15, 2021 at 11:31:45AM +0100, Andreas Henriksson wrote: > On Mon, Feb 15, 2021 at 09:41:30AM +, Colin Watson wrote: > > FWIW, I think your patch in > > https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=982705#25 is correct > > (even if I prefer to take a different approach as a workaround for the > > packaging) and could be forwarded upstream. Would you mind doing that? > > I normally prefer people to forward their own patches where possible so > > that there's no doubt about copyright/licensing intent or whatever. > > Agreed. The patch is fixing stuff in the non-portable version though > and I couldn't figure out how to contribute to that. The only > contribution info I could find lead to donating money to openbsd. IME: just send it as a bug on bugzilla.mindrot.org and they sort out which branch to apply it to. -- Colin Watson (he/him) [cjwat...@debian.org]
Bug#982705: openssh: FTBFS: sha2.h:57:16: error: redefinition of ‘struct _SHA2_CTX’
On Mon, Feb 15, 2021 at 09:41:30AM +, Colin Watson wrote: [...] > FWIW, I think your patch in > https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=982705#25 is correct > (even if I prefer to take a different approach as a workaround for the > packaging) and could be forwarded upstream. Would you mind doing that? > I normally prefer people to forward their own patches where possible so > that there's no doubt about copyright/licensing intent or whatever. Agreed. The patch is fixing stuff in the non-portable version though and I couldn't figure out how to contribute to that. The only contribution info I could find lead to donating money to openbsd. I did however send a mail yesterday with the patch attached directly to https://github.com/djmdjm who's seems to have been the one touching the relevant code according to https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/usr.bin/ssh/sk-usbhid.c (and seems also being active in https://github.com/openssh/openssh-portable ). YOLO and I'm moving on. Regards, Andreas Henriksson
Bug#982705: openssh: FTBFS: sha2.h:57:16: error: redefinition of ‘struct _SHA2_CTX’
On Mon, Feb 15, 2021 at 01:52:59AM +0100, Andreas Henriksson wrote: > On Sun, Feb 14, 2021 at 01:05:11PM +, Colin Watson wrote: > > How about this approach instead? Given the choice between a > > packaging-only change and one that requires another patch against > > upstream, I have a slight preference for the packaging-only change as > > long as it's basically reasonable, which I think this is. It just > > overrides configure's automatic detection and tells it that sha2.h isn't > > available. Builds cleanly and doesn't seem to incur any new direct > > dependencies. > > Whatever works and feel free to choose the way you as maintainer > prefers as far as I'm concerned! :) Right, I'll go ahead and upload this. > FWIW I make similar choices myself, but my definition of preferred > solution is a bit more complicated. Nothing is ever as permanent > as something temporary. It's not uncommon to see a temporary > hack be forgotten about and then not be dropped when it's > no longer needed, just to come back later and bite you in the ass. > So while I agree with your rule in general, I go for patches when > there's a big chance that the patch will stop apply when upstream > fixes this. Then it's hard to miss that it should be dropped when > the package is updated the next time However there's no guarantee > that will happen with my patch, so it's really up to you to go > with your gut feeling. Yeah, I agree with your more nuanced version of this too. FWIW, I think your patch in https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=982705#25 is correct (even if I prefer to take a different approach as a workaround for the packaging) and could be forwarded upstream. Would you mind doing that? I normally prefer people to forward their own patches where possible so that there's no doubt about copyright/licensing intent or whatever. -- Colin Watson (he/him) [cjwat...@debian.org]
Bug#982705: openssh: FTBFS: sha2.h:57:16: error: redefinition of ‘struct _SHA2_CTX’
Hello Colin Watson, On Sun, Feb 14, 2021 at 01:05:11PM +, Colin Watson wrote: > Thanks for digging into this. > > How about this approach instead? Given the choice between a > packaging-only change and one that requires another patch against > upstream, I have a slight preference for the packaging-only change as > long as it's basically reasonable, which I think this is. It just > overrides configure's automatic detection and tells it that sha2.h isn't > available. Builds cleanly and doesn't seem to incur any new direct > dependencies. Whatever works and feel free to choose the way you as maintainer prefers as far as I'm concerned! :) FWIW I make similar choices myself, but my definition of preferred solution is a bit more complicated. Nothing is ever as permanent as something temporary. It's not uncommon to see a temporary hack be forgotten about and then not be dropped when it's no longer needed, just to come back later and bite you in the ass. So while I agree with your rule in general, I go for patches when there's a big chance that the patch will stop apply when upstream fixes this. Then it's hard to miss that it should be dropped when the package is updated the next time However there's no guarantee that will happen with my patch, so it's really up to you to go with your gut feeling. And apart from that, my autotools knowledge simply isn't as good as yours to come up with your solution (even though I definitely have seen similar things used in the past now that you point it out). > > diff --git a/debian/rules b/debian/rules > index 73a53c309..44bac00f1 100755 > --- a/debian/rules > +++ b/debian/rules > @@ -65,6 +65,10 @@ ifeq ($(DEB_HOST_ARCH_OS),hurd) > confflags += --with-libs=-lcrypt > endif > > +# Avoid using libmd even if it's installed; see > +# https://bugs.debian.org/982705. > +confflags += ac_cv_header_sha2_h=false > + > # Everything above here is common to the deb and udeb builds. > confflags_udeb := $(confflags) > > > Thanks, > > -- > Colin Watson (he/him) [cjwat...@debian.org] Regards, Andreas Henriksson
Bug#982705: openssh: FTBFS: sha2.h:57:16: error: redefinition of ‘struct _SHA2_CTX’
On Sun, Feb 14, 2021 at 12:49:29PM +0100, Andreas Henriksson wrote: > Attached is a possibly upstreamable patch that solves our problem > (but the base problem still exists in the code for anyone wishing to > build with openssl disabled). Thanks for digging into this. How about this approach instead? Given the choice between a packaging-only change and one that requires another patch against upstream, I have a slight preference for the packaging-only change as long as it's basically reasonable, which I think this is. It just overrides configure's automatic detection and tells it that sha2.h isn't available. Builds cleanly and doesn't seem to incur any new direct dependencies. diff --git a/debian/rules b/debian/rules index 73a53c309..44bac00f1 100755 --- a/debian/rules +++ b/debian/rules @@ -65,6 +65,10 @@ ifeq ($(DEB_HOST_ARCH_OS),hurd) confflags += --with-libs=-lcrypt endif +# Avoid using libmd even if it's installed; see +# https://bugs.debian.org/982705. +confflags += ac_cv_header_sha2_h=false + # Everything above here is common to the deb and udeb builds. confflags_udeb := $(confflags) Thanks, -- Colin Watson (he/him) [cjwat...@debian.org]
Bug#982705: openssh: FTBFS: sha2.h:57:16: error: redefinition of ‘struct _SHA2_CTX’
Control: tags -1 + patch Hi again, Attached is a possibly upstreamable patch that solves our problem (but the base problem still exists in the code for anyone wishing to build with openssl disabled). See description in patch itself. Regards, Andreas Henriksson Description: sk-usbhid.c: Only include sha2.h if building without openssl Author: Andreas Henriksson Bug-Debian: https://bugs.debian.org/982705 There are many sha2.h and including both the openbsd-compat/sha2.h and the (libmd) /usr/include/sha2.h causes build problems. Other files like hash.c etc only includes the sha2.h if building without openssl. It seems like the code in sk-usbhid.c also doesn't really need to include it since it prefers using openssl already, so just reorder the includes similar to hash.c and others to avoid hitting this problem. (The underlying problem likely still needs to be resolved for anyone who wishes to actually build without openssl though.) Forwarded: TODO Last-Update: 2021-02-14 --- openssh-8.4p1.orig/sk-usbhid.c +++ openssh-8.4p1/sk-usbhid.c @@ -26,9 +26,6 @@ #include #include #include -#ifdef HAVE_SHA2_H -#include -#endif #ifdef WITH_OPENSSL #include @@ -37,6 +34,10 @@ #include #include #include +#else +# ifdef HAVE_SHA2_H +# include +# endif #endif /* WITH_OPENSSL */ #include
Bug#982705: openssh: FTBFS: sha2.h:57:16: error: redefinition of ‘struct _SHA2_CTX’
Hi,
On Sun, Feb 14, 2021 at 09:18:25AM +0100, Andreas Henriksson wrote:
> On Sun, Feb 14, 2021 at 08:32:58AM +0100, Andreas Henriksson wrote:
> > Hello,
> >
> > On Sat, Feb 13, 2021 at 06:04:32PM +0100, Lucas Nussbaum wrote:
> > > Source: openssh
> > > Version: 1:8.4p1-3
> > > Severity: serious
> > > Justification: FTBFS on amd64
[...]
I'm attaching a patch (which can be droppen in debian/patches and
appended to series) that adds libmd checking to configure.ac and sets
the required defines to make the build pass.
It likely needs additional work, see FIXMEs. Consider this a PoC. Only
compile-tested (so might not actually work at runtime).
Someone who knows what the intended purpose of the existing code was and
has motivation to untangle the mess called autotools should work out a
real patch.
Also, the SHA*Update etc. implemented by openbsd-compat/sha2.* doesn't
seem to even be called anywhere, so maybe it can just be removed
instead?
If we want a debian-only patch it might be easier to just do:
AC_DEFINE([_SSHSHA2_H_], [1], [Disable openbsd-compat/sha2.h])
Someone should really discuss with whoever does the openbsd-compat stuff
how they see the situation and what they think is the best way to handle
this.
Regards,
Andreas Henriksson
Index: openssh-8.4p1/configure.ac
===
--- openssh-8.4p1.orig/configure.ac
+++ openssh-8.4p1/configure.ac
@@ -1973,6 +1973,20 @@ AC_CHECK_FUNCS([ \
warn \
])
+# FIXME: Possible redefinition of HAVE_SHA* if they where already found
+# in AC_CHECK_FUNCS above?
+# FIXME: This should add -lmd to LDFLAGS instead of relying on something
+# else to pull it in (or even better use pkg-config --{cflags,libs} libmd).
+AC_CHECK_LIB([md], [SHA256Update], [
+ AC_DEFINE([HAVE_SHA256UPDATE], [1], [libmd has SHA256Update])
+ ], [])
+AC_CHECK_LIB([md], [SHA384Update], [
+ AC_DEFINE([HAVE_SHA384UPDATE], [1], [libmd has SHA384Update])
+ ], [])
+AC_CHECK_LIB([md], [SHA512Update], [
+ AC_DEFINE([HAVE_SHA512UPDATE], [1], [libmd has SHA512Update])
+ ], [])
+
AC_CHECK_DECLS([bzero, memmem])
dnl Wide character support.
Bug#982705: openssh: FTBFS: sha2.h:57:16: error: redefinition of ‘struct _SHA2_CTX’
On Sun, Feb 14, 2021 at 08:32:58AM +0100, Andreas Henriksson wrote: > Hello, > > On Sat, Feb 13, 2021 at 06:04:32PM +0100, Lucas Nussbaum wrote: > > Source: openssh > > Version: 1:8.4p1-3 > > Severity: serious > > Justification: FTBFS on amd64 [...] > (Which in turn makes me wonder if something changed on the libmd side?) So looked around a bit more here Apparently this is the reason libmd-dev is installed: openssh b-d libedit-dev libedit-dev dep libbsd-dev libbsd-dev dep libmd-dev Both libmd-dev and libbsd-dev seems to have had recent changes. >From libbsd (0.11.1-1) changelog: ``` - Switch from embedded hashing function implementations to use libmd. Add libmd-dev to Build-Depends and libbsd-dev Depends. ``` I have not yet looked up what the configure results where for the last successful openssh build, but I suspect that the situation back then was that SHA256Update et.al. where not found and the openssh compat sha2.h was used, but this was not a problem because libmd-dev (which provides /usr/lib/sha2.h) was not pulled in back then either... Simply introducing an opennsh `Build-Conflicts: libmd-dev` will not work now that we have a dependency chain which will pull it in via libedit-dev. I suspect the way out of this is to simply improve the openssh configure now... Regards, Andreas Henriksson
Bug#982705: openssh: FTBFS: sha2.h:57:16: error: redefinition of ‘struct _SHA2_CTX’
Hello,
On Sat, Feb 13, 2021 at 06:04:32PM +0100, Lucas Nussbaum wrote:
> Source: openssh
> Version: 1:8.4p1-3
> Severity: serious
> Justification: FTBFS on amd64
[...]
> > In file included from ../../sk-usbhid.c:30:
> > /usr/include/sha2.h:57:16: error: redefinition of ‘struct _SHA2_CTX’
[...]
> > ../../openbsd-compat/sha2.h:66:16: note: originally defined here
[...]
This problem seems to be caused by configure not finding the
SHA{256,384,512}Update functions and thus not defining HAVE_*
for them to make openbsd-compat/sha2.h ifndef's bail out.
The build log says:
```
checking for SHA256Update... no
checking for SHA384Update... no
checking for SHA512Update... no
```
More info on why is in config.log :
```
configure:11580: checking for SHA256Update
configure:11580: cc -o conftest -g -O2 -ffile-prefix-map=/tmp/openssh-8.4p1=.
-fstack-protector-strong -Wformat -Werror=format-security -pipe
-Wno-error=format-truncation -Wall -Wextra -Wpointer-arith -Wuninitialized
-Wsign-compare -Wformat-security -Wsizeof-pointer-memaccess -Wno-pointer-sign
-Wno-unused-parameter -Wno-unused-result -Wimplicit-fallthrough
-fno-strict-aliasing -D_FORTIFY_SOURCE=2 -ftrapv -fno-builtin-memset
-fstack-protector-strong -Wdate-time -D_FORTIFY_SOURCE=2 -g -O2
-ffile-prefix-map=/tmp/openssh-8.4p1=. -fstack-protector-strong -Wformat
-Werror=format-security -DSSH_EXTRAVERSION=\"Debian-3\" -Wdate-time
-D_FORTIFY_SOURCE=2 -D_XOPEN_SOURCE=600 -D_BSD_SOURCE -D_DEFAULT_SOURCE
-I/usr/include/editline -Wl,-z,relro -Wl,-z,now -Wl,-z,relro -Wl,-z,now
-Wl,-z,noexecstack -fstack-protector-strong -Wl,--as-needed -Wl,-z,relro
-Wl,-z,now conftest.c -lutil -lz >&5
: warning: missing terminating " character
/usr/bin/ld: /tmp/cc7rTcJW.o: in function `main':
./debian/build-deb/conftest.c:153: undefined reference to `SHA256Update'
collect2: error: ld returned 1 exit status
```
Seems like some linker flag (-lmd) is missing to make the test program
succeed. OpenSSH uses AC_CHECK_FUNCS to check for SHA256Update, etc.
This macro doesn't have any way to pass in -lmd as far as I can tell
(Which in turn makes me wonder if something changed on the libmd side?)
Regards,
Andreas Henriksson
Bug#982705: openssh: FTBFS: sha2.h:57:16: error: redefinition of ‘struct _SHA2_CTX’
Source: openssh
Version: 1:8.4p1-3
Severity: serious
Justification: FTBFS on amd64
Tags: bullseye sid ftbfs
Usertags: ftbfs-20210213 ftbfs-bullseye
Hi,
During a rebuild of all packages in sid, your package failed to build
on amd64.
Relevant part (hopefully):
> cc -g -O2 -ffile-prefix-map=/<>=. -fstack-protector-strong
> -Wformat -Werror=format-security -pipe -Wno-error=format-truncation -Wall
> -Wextra -Wpointer-arith -Wuninitialized -Wsign-compare -Wformat-security
> -Wsizeof-pointer-memaccess -Wno-pointer-sign -Wno-unused-parameter
> -Wno-unused-result -Wimplicit-fallthrough -fno-strict-aliasing
> -D_FORTIFY_SOURCE=2 -ftrapv -fno-builtin-memset -fstack-protector-strong
> -Wdate-time -D_FORTIFY_SOURCE=2 -g -O2 -ffile-prefix-map=/<>=.
> -fstack-protector-strong -Wformat -Werror=format-security
> -DSSH_EXTRAVERSION=\"Debian-3\" -I. -I../.. -Wdate-time -D_FORTIFY_SOURCE=2
> -D_XOPEN_SOURCE=600 -D_BSD_SOURCE -D_DEFAULT_SOURCE -I/usr/include/editline
> -isystem /usr/include/mit-krb5 -isystem /usr/include/mit-krb5
> -DSSHDIR=\"/etc/ssh\" -D_PATH_SSH_PROGRAM=\"/usr/bin/ssh\"
> -D_PATH_SSH_ASKPASS_DEFAULT=\"/usr/bin/ssh-askpass\"
> -D_PATH_SFTP_SERVER=\"/usr/lib/openssh/sftp-server\"
> -D_PATH_SSH_KEY_SIGN=\"/usr/lib/openssh/ssh-keysign\"
> -D_PATH_SSH_PKCS11_HELPER=\"/usr/lib/openssh/ssh-pkcs11-helper\"
> -D_PATH_SSH_SK_HELPER=\"/usr/lib/openssh/ssh-sk-helper\"
> -D_PATH_SSH_PIDDIR=\"/run\" -D_PATH_PRIVSEP_CHROOT_DIR=\"/run/sshd\"
> -DHAVE_CONFIG_H -c ../../sk-usbhid.c -o sk-usbhid.o
> In file included from ../../sk-usbhid.c:30:
> /usr/include/sha2.h:57:16: error: redefinition of ‘struct _SHA2_CTX’
>57 | typedef struct _SHA2_CTX {
> |^
> In file included from ../../openbsd-compat/openbsd-compat.h:46,
> from ../../includes.h:174,
> from ../../sk-usbhid.c:19:
> ../../openbsd-compat/sha2.h:66:16: note: originally defined here
>66 | typedef struct _SHA2_CTX {
> |^
> In file included from ../../sk-usbhid.c:30:
> /usr/include/sha2.h:64:3: error: conflicting types for ‘SHA2_CTX’
>64 | } SHA2_CTX;
> | ^~~~
> In file included from ../../openbsd-compat/openbsd-compat.h:46,
> from ../../includes.h:174,
> from ../../sk-usbhid.c:19:
> ../../openbsd-compat/sha2.h:73:3: note: previous declaration of ‘SHA2_CTX’
> was here
>73 | } SHA2_CTX;
> | ^~~~
> In file included from ../../sk-usbhid.c:30:
> /usr/include/sha2.h:70:6: error: conflicting types for ‘SHA256Init’
>70 | void SHA256Init(SHA2_CTX *);
> | ^~
> In file included from ../../openbsd-compat/openbsd-compat.h:46,
> from ../../includes.h:174,
> from ../../sk-usbhid.c:19:
> ../../openbsd-compat/sha2.h:96:6: note: previous declaration of ‘SHA256Init’
> was here
>96 | void SHA256Init(SHA2_CTX *);
> | ^~
> In file included from ../../sk-usbhid.c:30:
> /usr/include/sha2.h:72:6: error: conflicting types for ‘SHA256Update’
>72 | void SHA256Update(SHA2_CTX *, const uint8_t *, size_t);
> | ^~~~
> In file included from ../../openbsd-compat/openbsd-compat.h:46,
> from ../../includes.h:174,
> from ../../sk-usbhid.c:19:
> ../../openbsd-compat/sha2.h:98:6: note: previous declaration of
> ‘SHA256Update’ was here
>98 | void SHA256Update(SHA2_CTX *, const u_int8_t *, size_t)
> | ^~~~
> In file included from ../../sk-usbhid.c:30:
> /usr/include/sha2.h:73:6: error: conflicting types for ‘SHA256Pad’
>73 | void SHA256Pad(SHA2_CTX *);
> | ^
> In file included from ../../openbsd-compat/openbsd-compat.h:46,
> from ../../includes.h:174,
> from ../../sk-usbhid.c:19:
> ../../openbsd-compat/sha2.h:100:6: note: previous declaration of ‘SHA256Pad’
> was here
> 100 | void SHA256Pad(SHA2_CTX *);
> | ^
> In file included from ../../sk-usbhid.c:30:
> /usr/include/sha2.h:74:6: error: conflicting types for ‘SHA256Final’
>74 | void SHA256Final(uint8_t [SHA256_DIGEST_LENGTH], SHA2_CTX *);
> | ^~~
> In file included from ../../openbsd-compat/openbsd-compat.h:46,
> from ../../includes.h:174,
> from ../../sk-usbhid.c:19:
> ../../openbsd-compat/sha2.h:101:6: note: previous declaration of
> ‘SHA256Final’ was here
> 101 | void SHA256Final(u_int8_t [SHA256_DIGEST_LENGTH], SHA2_CTX *)
> | ^~~
> In file included from ../../sk-usbhid.c:30:
> /usr/include/sha2.h:75:7: error: conflicting types for ‘SHA256End’
>75 | char *SHA256End(SHA2_CTX *, char *);
> | ^
> In file included from ../../openbsd-compat/openbsd-compat.h:46,
> from ../../includes.h:174,
> from ../../sk-usbhid.c:19:
> ../../openbsd-compat/sha2.h:103:7: note: previous declaration of ‘SHA256End’
>
