Bug 1159097

Summary: OpenSSH 7.9p1-lp151.3.4 source package: Wrong function name used in patch (openssh-7.7p1-gssapi_key_exchange.patch)
Product: [openSUSE] openSUSE Distribution Reporter: Frank Scheiner <scheiner>
Component: NetworkAssignee: Hans Petter Jansson <hpj>
Status: RESOLVED DUPLICATE QA Contact: E-mail List <qa-bugs>
Severity: Normal    
Priority: P5 - None CC: alynx.zhou, mischa.salle
Version: Leap 15.1   
Target Milestone: ---   
Hardware: Other   
OS: Other   
Whiteboard:
Found By: --- Services Priority:
Business Priority: Blocker: ---
Marketing QA Status: --- IT Deployment: ---

Description Frank Scheiner 2019-12-12 11:03:20 UTC
User-Agent:       Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:70.0) Gecko/20100101 Firefox/70.0
Build Identifier: 

During creation of a GSI and HPN enabled OpenSSH package ([1]) for the GridCF's Grid Community Toolkit we (a colleague of mine and me) discovered an oddity in the "openssh-7.7p1-gssapi_key_exchange.patch" which is part of [2] - the package used as basis for [1].

[1]: https://build.opensuse.org/package/show/home:frank_scheiner:gct/gsi-openssh

[2]: http://download.opensuse.org/source/distribution/leap/15.1/repo/oss/src/openssh-7.9p1-lp151.3.4.src.rpm

There we have:

```
1575 +int
1576 +kexgss_server(struct ssh *ssh)
[...]
1603 +       const BIGNUM *p, *g, *pub_key;
[...]
1650 +               DH_set0_pqg(dh, &p, NULL, &g);
[...]
1652 +               packet_put_bignum2((BIGNUM *)p);
1653 +               packet_put_bignum2((BIGNUM *)g);
[...]
1745 +       DH_get0_key(dh, &pub_key, NULL);
[...]
1793 +       packet_put_bignum2(pub_key);
```

Specifically line 1650 looks odd, when comparing the arguments (and their types) to the actual interface of the `DH_set0_pqg()` function on e.g. [3]:

```
int DH_set0_pqg(DH *dh, BIGNUM *p, BIGNUM *q, BIGNUM *g);
```

```
void DH_get0_pqg(const DH *dh,
                  const BIGNUM **p, const BIGNUM **q, const BIGNUM **g);
```
... also from [3] would be a better fit here.

[3]: https://www.openssl.org/docs/man1.1.1/man3/DH_set0_pqg.html

And comparing these specific lines to their counterparts in the OpenSSH 7.9p1 src RPM in Fedora ([4], from the "openssh-7.8p1-gsskex.patch" patch):

```
1390 +int
1391 +kexgss_server(struct ssh *ssh)
[...]
1418 +       const BIGNUM *p, *g, *pub_key;
[...]
1464 +               DH_get0_pqg(dh, &p, NULL, &g);
[...]
1466 +               packet_put_bignum2((BIGNUM *)p);
1467 +               packet_put_bignum2((BIGNUM *)g);
[...]
1559 +       DH_get0_key(dh, &pub_key, NULL);
[...]
1607 +       packet_put_bignum2(pub_key);
```

...it looks like the function name in the openSUSE patch differs and maybe is the result of a typo?

[4]: https://kojipkgs.fedoraproject.org/packages/gsi-openssh/7.9p1/7.fc31/src/gsi-openssh-7.9p1-7.fc31.src.rpm

When compiling with the assumed wrong function name in the openSUSE patch together with our GSI patch(es) (which use the GSSAPI), this creates the following related warnings:

```
gcc -fpie -fstack-protector -pipe -Wall -Wpointer-arith -Wuninitialized -Wsign-compare -Wformat-security -Wsizeof-pointer-memaccess -Wno-pointer-sign -Wno-unused-result -fno-strict-aliasing -ftrapv -fno-builtin-memset -fstack-protector-strong -fPIE   -I. -I.  -D_XOPEN_SOURCE=600 -D_BSD_SOURCE -D_DEFAULT_SOURCE -I/usr/include/editline -DLDAP_DEPRECATED -DOPENSSL_LOAD_CONF -I/usr/include/globus -DSSHDIR=\"/etc/gsissh\" -D_PATH_SSH_PROGRAM=\"/usr/bin/ssh\" -D_PATH_SSH_ASKPASS_DEFAULT=\"/usr/libexec/gsissh/ssh-askpass\" -D_PATH_SFTP_SERVER=\"/usr/libexec/gsissh/sftp-server\" -D_PATH_SSH_KEY_SIGN=\"/usr/libexec/gsissh/ssh-keysign\" -D_PATH_SSH_PKCS11_HELPER=\"/usr/libexec/gsissh/ssh-pkcs11-helper\" -D_PATH_SSH_PIDDIR=\"/run\" -D_PATH_PRIVSEP_CHROOT_DIR=\"/var/lib/empty\" -DHAVE_CONFIG_H -c kexgsss.c -o kexgsss.o
kexgsss.c: In function ‘kexgss_server’:
kexgsss.c:131:32: warning: passing argument 2 of ‘DH_set0_pqg’ from incompatible pointer type [-Wincompatible-pointer-types]
                DH_set0_pqg(dh, &p, NULL, &g);
                                ^
In file included from /usr/include/openssl/dsa.h:31:0,
                 from sshkey.h:33,
                 from kexgsss.c:37:
/usr/include/openssl/dh.h:175:5: note: expected ‘BIGNUM * {aka struct bignum_st *}’ but argument is of type ‘const BIGNUM ** {aka const struct bignum_st **}’
 int DH_set0_pqg(DH *dh, BIGNUM *p, BIGNUM *q, BIGNUM *g);
     ^~~~~~~~~~~
kexgsss.c:131:42: warning: passing argument 4 of ‘DH_set0_pqg’ from incompatible pointer type [-Wincompatible-pointer-types]
                DH_set0_pqg(dh, &p, NULL, &g);
                                          ^
In file included from /usr/include/openssl/dsa.h:31:0,
                 from sshkey.h:33,
                 from kexgsss.c:37:
/usr/include/openssl/dh.h:175:5: note: expected ‘BIGNUM * {aka struct bignum_st *}’ but argument is of type ‘const BIGNUM ** {aka const struct bignum_st **}’
 int DH_set0_pqg(DH *dh, BIGNUM *p, BIGNUM *q, BIGNUM *g);
     ^~~~~~~~~~~
In file included from packet.h:223:0,
                 from kexgsss.c:41:
opacket.h:77:39: warning: passing argument 2 of ‘ssh_packet_put_bignum2’ discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
  ssh_packet_put_bignum2(active_state, (value))
                                       ^
kexgsss.c:275:2: note: in expansion of macro ‘packet_put_bignum2’
  packet_put_bignum2(pub_key);
  ^~~~~~~~~~~~~~~~~~
opacket.h:10:10: note: expected ‘BIGNUM * {aka struct bignum_st *}’ but argument is of type ‘const BIGNUM * {aka const struct bignum_st *}’
 void     ssh_packet_put_bignum2(struct ssh *, BIGNUM * value);
          ^~~~~~~~~~~~~~~~~~~~~~
[...]
```

When using the assumed correct function name, these warnings are gone, but a new warning appears due to passing a const var as non-const argument:

```
[  131s] gcc -fmessage-length=0 -grecord-gcc-switches -O2 -Wall -D_FORTIFY_SOURCE=2 -fstack-protector-strong -funwind-tables -fasynchronous-unwind-tables -fstack-clash-protection -fpie -fstack-protector -pipe -Wall -Wpointer-arith -Wuninitialized -Wsign-compare -Wformat-security -Wsizeof-pointer-memaccess -Wno-pointer-sign -Wno-unused-result -fno-strict-aliasing -D_FORTIFY_SOURCE=2 -ftrapv -fno-builtin-memset -fstack-protector-strong -fPIE   -I. -I.  -D_XOPEN_SOURCE=600 -D_BSD_SOURCE -D_DEFAULT_SOURCE -I/usr/include/editline -DLDAP_DEPRECATED -DOPENSSL_LOAD_CONF -I/usr/include/globus  -DSSHDIR=\"/etc/gsissh\" -D_PATH_SSH_PROGRAM=\"/usr/bin/gsissh\" -D_PATH_SSH_ASKPASS_DEFAULT=\"/usr/lib/gsissh/ssh-askpass\" -D_PATH_SFTP_SERVER=\"/usr/lib/gsissh/sftp-server\" -D_PATH_SSH_KEY_SIGN=\"/usr/lib/gsissh/ssh-keysign\" -D_PATH_SSH_PKCS11_HELPER=\"/usr/lib/gsissh/ssh-pkcs11-helper\" -D_PATH_SSH_PIDDIR=\"/run\" -D_PATH_PRIVSEP_CHROOT_DIR=\"/var/lib/empty\" -DHAVE_CONFIG_H -c ssh.c -o ssh.o
[  131s] In file included from packet.h:223:0,
[  131s]                  from kexgsss.c:41:
[  131s] kexgsss.c: In function 'kexgss_server':
[  131s] opacket.h:77:39: warning: passing argument 2 of 'ssh_packet_put_bignum2' discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
[  131s]   ssh_packet_put_bignum2(active_state, (value))
[  131s]                                        ^
[  131s] kexgsss.c:275:2: note: in expansion of macro 'packet_put_bignum2'
[  131s]   packet_put_bignum2(pub_key);
[  131s]   ^~~~~~~~~~~~~~~~~~
[  131s] opacket.h:10:10: note: expected 'BIGNUM * {aka struct bignum_st *}' but argument is of type 'const BIGNUM * {aka const struct bignum_st *}'
[  131s]  void     ssh_packet_put_bignum2(struct ssh *, BIGNUM * value);
[  131s]           ^~~~~~~~~~~~~~~~~~~~~~
```

What's your opinion on that?

Reproducible: Always
Comment 1 Frank Scheiner 2020-05-07 14:26:05 UTC
This was already fixed before I originally created this bug with:

```
* Fri May 31 2019 Vítězslav Čížek <vcizek@suse.com>
- Fix a crash with GSSAPI key exchange (bsc#1136104)
  * modify openssh-7.7p1-gssapi_key_exchange.patch
```

...in the package "openssh-7.9p1-lp151.4.3.1.src.rpm" from "update".

Corresponding bug is bsc#1136104.

*** This bug has been marked as a duplicate of bug 1136104 ***