Discussion:
Call for testing: OpenSSH 7.9
Damien Miller
2018-10-11 03:54:21 UTC
Permalink
Hi,

OpenSSH 7.9p1 is almost ready for release, so we would appreciate testing
on as many platforms and systems as possible. This is a bugfix release.

Snapshot releases for portable OpenSSH are available from
http://www.mindrot.org/openssh_snap/

The OpenBSD version is available in CVS HEAD:
http://www.openbsd.org/anoncvs.html

Portable OpenSSH is also available via git using the
instructions at http://www.openssh.com/portable.html#cvs
At https://anongit.mindrot.org/openssh.git/ or via a mirror at Github:
https://github.com/openssh/openssh-portable

Running the regression tests supplied with Portable OpenSSH does not
require installation and is a simply:

$ ./configure && make tests

Live testing on suitable non-production systems is also appreciated.
Please send reports of success or failure to
openssh-unix-***@mindrot.org. Security bugs should be reported
directly to ***@openssh.com.

Below is a summary of changes. More detail may be found in the ChangeLog
in the portable OpenSSH tarballs.

Thanks to the many people who contributed to this release.

Potentially-incompatible changes
================================

This release includes a number of changes that may affect existing
configurations:

* ssh(1), sshd(8): the setting of the new CASignatureAlgorithms
option (see below) bans the use of DSA keys as certificate
authorities.

* sshd(8): the authentication success/failure log message has
changed format slightly. It now includes the certificate
fingerprint (previously it included only key ID and CA key
fingerprint).

Changes since OpenSSH 7.8
=========================

This is primarily a bugfix release.

New Features
------------

* ssh(1), sshd(8): allow most port numbers to be specified using
service names from getservbyname(3) (typically /etc/services).

* ssh(1): allow the IdentityAgent configuration directive to accept
environment variable names. This supports the use of multiple
agent sockets without needing to use fixed paths.

* sshd(8): support signalling sessions via the SSH protocol.
A limited subset of signals is supported and only for login or
command sessions (i.e. not subsystems) that were not subject to
a forced command via authorized_keys or sshd_config. bz#1424

* ssh(1): support "ssh -Q sig" to list supported signature options.
Also "ssh -Q help" to show the full set of supported queries.

* ssh(1), sshd(8): add a CASignatureAlgorithms option for the
client and server configs to allow control over which signature
formats are allowed for CAs to sign certificates. For example,
this allows banning CAs that sign certificates using the RSA-SHA1
signature algorithm.

* sshd(8), ssh-keygen(1): allow key revocation lists (KRLs) to
revoke keys specified by SHA256 hash.

* ssh-keygen(1): allow creation of key revocation lists directly
from base64-encoded SHA256 fingerprints. This supports revoking
keys using only the information contained in sshd(8)
authentication log messages.

Bugfixes
--------

* ssh(1), ssh-keygen(1): avoid spurious "invalid format" errors when
attempting to load PEM private keys while using an incorrect
passphrase. bz#2901

* sshd(8): when a channel closed message is received from a client,
close the stderr file descriptor at the same time stdout is
closed. This avoids stuck processes if they were waiting for
stderr to close and were insensitive to stdin/out closing. bz#2863

* ssh(1): allow ForwardX11Timeout=0 to disable the untrusted X11
forwarding timeout and support X11 forwarding indefinitely.
Previously the behaviour of ForwardX11Timeout=0 was undefined.

* sshd(8): when compiled with GSSAPI support, cache supported method
OIDs regardless of whether GSSAPI authentication is enabled in the
main section of sshd_config. This avoids sandbox violations if
GSSAPI authentication was later enabled in a Match block. bz#2107

* sshd(8): do not fail closed when configured with a text key
revocation list that contains a too-short key. bz#2897

* ssh(1): treat connections with ProxyJump specified the same as
ones with a ProxyCommand set with regards to hostname
canonicalisation (i.e. don't try to canonicalise the hostname
unless CanonicalizeHostname is set to 'always'). bz#2896

* ssh(1): fix regression in OpenSSH 7.8 that could prevent public-
key authentication using certificates hosted in a ssh-agent(1)
or against sshd(8) from OpenSSH <7.8.

Portability
-----------

* All: support building against the openssl-1.1 API. The
openssl-1.0 API will remain supported at least until OpenSSL
terminates security patch support for that API version.

* sshd(8): allow the futex(2) syscall in the Linux seccomp sandbox;
apparently required by some glibc/OpenSSL combinations.

* sshd(8): handle getgrouplist(3) returning more than
_SC_NGROUPS_MAX groups. Some platforms consider this limit more
as a guideline.

Reporting Bugs:
===============

- Please read http://www.openssh.com/report.html
Security bugs should be reported directly to ***@openssh.com

OpenSSH is brought to you by Markus Friedl, Niels Provos, Theo de
Raadt, Kevin Steves, Damien Miller, Darren Tucker, Jason McIntyre,
Tim Rice and Ben Lindstrom.
Hisashi T Fujinaka
2018-10-11 05:16:42 UTC
Permalink
NetBSD-8-amd64 and NetBSD-current-amd64 both quit in configure at:

config.status: error: cannot find input file: `config.h.in'

--
Hisashi T Fujinaka - ***@twofifty.com
BSEE + BSChem + BAEnglish + MSCS + $2.50 = coffee
Damien Miller
2018-10-11 06:36:39 UTC
Permalink
On Wed, 10 Oct 2018, Hisashi T Fujinaka wrote:

> NetBSD-8-amd64 and NetBSD-current-amd64 both quit in configure at:
>
> config.status: error: cannot find input file: `config.h.in'

were you using the snapshots or checking out from git?

If you were doing the latter then you probably need to run autoreconf
first.

-d
Hisashi T Fujinaka
2018-10-11 15:44:55 UTC
Permalink
On Thu, 11 Oct 2018, Damien Miller wrote:

> On Wed, 10 Oct 2018, Hisashi T Fujinaka wrote:
>
>> NetBSD-8-amd64 and NetBSD-current-amd64 both quit in configure at:
>>
>> config.status: error: cannot find input file: `config.h.in'
>
> were you using the snapshots or checking out from git?
>
> If you were doing the latter then you probably need to run autoreconf
> first.

D'oh. I keep forgetting that and start with autoconf. Did I miss it in
the docs AGAIN?

--
Hisashi T Fujinaka - ***@twofifty.com
BSEE + BSChem + BAEnglish + MSCS + $2.50 = coffee
Darren Tucker
2018-10-11 06:39:52 UTC
Permalink
On 11 October 2018 at 16:16, Hisashi T Fujinaka <***@twofifty.com> wrote:
> NetBSD-8-amd64 and NetBSD-current-amd64 both quit in configure at:
>
> config.status: error: cannot find input file: `config.h.in'

If you are checking out from git yourself you need to run "autoreconf"
to rebuild the autoconf files, including that one, before running
configure. (Although presumably you did something to create
configure?) If you're using a snapshot that should already have
those files.

--
Darren Tucker (dtucker at dtucker.net)
GPG key 11EAA6FA / A86E 3E07 5B19 5880 E860 37F4 9357 ECEF 11EA A6FA (new)
Good judgement comes with experience. Unfortunately, the experience
usually comes from bad judgement.
Adam Eijdenberg
2018-10-11 06:53:07 UTC
Permalink
On Thu, Oct 11, 2018 at 2:58 PM Damien Miller <***@mindrot.org> wrote:
> $ ./configure && make tests

Succeeds on my Ubuntu 18.04 (with OpenSSL 1.1.0g).
The Doctor
2018-10-11 13:14:58 UTC
Permalink
Reporting in!

FreeBsd 11.2 on

Openssl 1.0.2

and openssl 1.1.1

are working and all tests passed.

Running them without issue so far.

--
Member - Liberal International This is doctor@@nl2k.ab.ca Ici doctor@@nl2k.ab.ca
Yahweh, Queen & country!Never Satan President Republic!Beware AntiChrist rising!
https://www.empire.kred/ROOTNK?t=94a1f39b Look at Psalms 14 and 53 on Atheism
Love much. Earth has enough of bitter in it. -Ella Wheeler Wilcox
Val Baranov
2018-10-11 17:42:38 UTC
Permalink
"all test passed" for AIX 7.1 TL5 SP2 (7100-05-02-1832). OpenSSL 1.0.2p.

Val
Michael Felt
2018-10-12 12:17:34 UTC
Permalink
Also passed on my AIX system:


OpenSSH_7.9p1-snap20181012, OpenSSL 1.0.2j  26 Sep 2016
***@x065:[/data/prj/openbsd/openssh/openssh-7.9.10.12]oslevel -s
xl5300-07-00-0000
***@x065:[/data/prj/openbsd/openssh/openssh-7.9.10.12]xlc -qversion
IBM XL C/C++ for AIX, V11.1 (5724-X13)
Version: 11.01.0000.0020

FYI: the messages (to stderr) during make

"kludge-fd_set.c", line 28.1: 1506-356 (W) Compilation unit is empty.
"glob.c", line 94.9: 1506-236 (W) Macro name TILDE has been redefined.
"glob.c", line 94.9: 1506-358 (I) "TILDE" is defined on line 250 of
/usr/include/sys/ioctl.h.
"/usr/include/syms.h", line 288.9: 1506-236 (W) Macro name T_NULL has
been redefined.
"/usr/include/syms.h", line 288.9: 1506-358 (I) "T_NULL" is defined on
line 150 of /usr/include/arpa/onameser_compat.h.
"/usr/include/sys/file.h", line 132.9: 1506-236 (W) Macro name LOCK_SH
has been redefined.
"/usr/include/sys/file.h", line 132.9: 1506-358 (I) "LOCK_SH" is defined
on line 149 of ../openbsd-compat/bsd-misc.h.
"/usr/include/sys/file.h", line 133.9: 1506-236 (W) Macro name LOCK_EX
has been redefined.
"/usr/include/sys/file.h", line 133.9: 1506-358 (I) "LOCK_EX" is defined
on line 150 of ../openbsd-compat/bsd-misc.h.
"/usr/include/sys/file.h", line 134.9: 1506-236 (W) Macro name LOCK_NB
has been redefined.
"/usr/include/sys/file.h", line 134.9: 1506-358 (I) "LOCK_NB" is defined
on line 151 of ../openbsd-compat/bsd-misc.h.
"/usr/include/sys/file.h", line 135.9: 1506-236 (W) Macro name LOCK_UN
has been redefined.
"/usr/include/sys/file.h", line 135.9: 1506-358 (I) "LOCK_UN" is defined
on line 152 of ../openbsd-compat/bsd-misc.h.
ar: Creating an archive file libopenbsd-compat.a.
"ssh-pkcs11.c", line 615.30: 1506-068 (W) Operation between types
"unsigned long(*)(struct _CK_FUNCTION_LIST**)" and "void*" is not allowed.
ar: Creating an archive file libssh.a.
"mux.c", line 1831.44: 1506-280 (W) Function argument assignment between
types "unsigned int*" and "int*" is not allowed.
"platform.c", line 194.44: 1506-280 (W) Function argument assignment
between types "char*" and "const char*" is not allowed.
"auth.c", line 261.40: 1506-280 (W) Function argument assignment between
types "struct sshbuf*" and "struct sshbuf**" is not allowed.
"auth.c", line 365.21: 1506-280 (W) Function argument assignment between
types "struct sshbuf*" and "struct sshbuf**" is not allowed.
"monitor.c", line 1132.36: 1506-280 (W) Function argument assignment
between types "unsigned int*" and "enum mm_keytype*" is not allowed.
     407  1500-010: (W) WARNING in monitor_child_postauth: Infinite
loop.  Program may not stop.
"monitor_wrap.c", line 415.36: 1506-280 (W) Function argument assignment
between types "unsigned int*" and "int*" is not allowed.
"monitor_wrap.c", line 475.36: 1506-280 (W) Function argument assignment
between types "unsigned int*" and "int*" is not allowed.
"monitor_wrap.c", line 578.36: 1506-280 (W) Function argument assignment
between types "unsigned int*" and "int*" is not allowed.
"monitor_wrap.c", line 863.36: 1506-280 (W) Function argument assignment
between types "unsigned int*" and "int*" is not allowed.
"loginrec.c", line 470.13: 1506-280 (W) Function argument assignment
between types "struct sshbuf*" and "struct sshbuf**" is not allowed.
    1644  1500-010: (W) WARNING in sftp_server_main: Infinite loop. 
Program may not stop.
     329  1500-010: (W) WARNING in main: Infinite loop.  Program may not
stop.
    1324  1500-010: (W) WARNING in main: Infinite loop.  Program may not
stop.


Also note, 'make tests' does not test utf8 (not installed by default):

BUILDDIR=`pwd`; \
TEST_SSH_SCP="${BUILDDIR}/scp"; \
TEST_SSH_SSH="${BUILDDIR}/ssh"; \
TEST_SSH_SSHD="${BUILDDIR}/sshd"; \
TEST_SSH_SSHAGENT="${BUILDDIR}/ssh-agent"; \
TEST_SSH_SSHADD="${BUILDDIR}/ssh-add"; \
TEST_SSH_SSHKEYGEN="${BUILDDIR}/ssh-keygen"; \
TEST_SSH_SSHPKCS11HELPER="${BUILDDIR}/ssh-pkcs11-helper"; \
TEST_SSH_SSHKEYSCAN="${BUILDDIR}/ssh-keyscan"; \
TEST_SSH_SFTP="${BUILDDIR}/sftp"; \
TEST_SSH_SFTPSERVER="${BUILDDIR}/sftp-server"; \
TEST_SSH_PLINK="plink"; \
TEST_SSH_PUTTYGEN="puttygen"; \
TEST_SSH_CONCH="conch"; \
TEST_SSH_IPV6="yes" ; \
TEST_SSH_UTF8="no" ; \
TEST_SSH_ECC="yes" ; \
cd ./regress || exit $?; \
make \

...


On 10/11/2018 7:42 PM, Val Baranov wrote:
> "all test passed" for AIX 7.1 TL5 SP2 (7100-05-02-1832). OpenSSL 1.0.2p.
>
> Val
>
>
> _______________________________________________
> openssh-unix-dev mailing list
> openssh-unix-***@mindrot.org
> https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev
Hisashi T Fujinaka
2018-10-11 23:28:23 UTC
Permalink
OK, netbsd-8 i386 & x86 passed.
netbsd-current x86 also passed now!

--
Hisashi T Fujinaka - ***@twofifty.com
BSEE + BSChem + BAEnglish + MSCS + $2.50 = coffee
Corinna Vinschen
2018-10-12 13:46:13 UTC
Permalink
On Oct 11 14:54, Damien Miller wrote:
> Hi,
>
> OpenSSH 7.9p1 is almost ready for release, so we would appreciate testing
> on as many platforms and systems as possible. This is a bugfix release.

All tests pass on current Cygwin 2.11.1.


Thanks,
Corinna

--
Corinna Vinschen
Cygwin Maintainer
Red Hat
Jakub Jelen
2018-10-12 13:47:57 UTC
Permalink
On Thu, 2018-10-11 at 14:54 +1100, Damien Miller wrote:
> Hi,
>
> OpenSSH 7.9p1 is almost ready for release, so we would appreciate
> testing
> on as many platforms and systems as possible. This is a bugfix
> release.

The latest snapshot is still using OPENSSL_config() which was
deprecated in OpenSSL 1.1.0:

openssl-compat.c: In function 'ssh_OpenSSL_add_all_algorithms':
openssl-compat.c:78:2: warning: 'OPENSSL_config' is deprecated [-
Wdeprecated-declarations]
OPENSSL_config(NULL);
^~~~~~~~~~~~~~
In file included from /usr/include/openssl/opensslconf.h:42,
from /usr/include/openssl/engine.h:19,
from openssl-compat.c:26:
/usr/include/openssl/conf.h:92:1: note: declared here
DEPRECATEDIN_1_1_0(void OPENSSL_config(const char *config_name))
^~~~~~~~~~~~~~~~~~


Something like this can be used to properly initialize new OpenSSL
versions:


@@ -70,12 +70,19 @@ ssh_compatible_openssl(long headerver, long libver)
void
ssh_OpenSSL_add_all_algorithms(void)
{
+#if OPENSSL_VERSION_NUMBER < 0x10100000L
OpenSSL_add_all_algorithms();

/* Enable use of crypto hardware */
ENGINE_load_builtin_engines();
+#if OPENSSL_VERSION_NUMBER < 0x10001000L
ENGINE_register_all_complete();
+#endif
OPENSSL_config(NULL);
+#else
+ OPENSSL_init_crypto(OPENSSL_INIT_ADD_ALL_DIGESTS |
+ OPENSSL_INIT_ADD_ALL_DIGESTS | OPENSSL_INIT_LOAD_CONFIG,
NULL);
+#endif
}
#endif


Regards,
--
Jakub Jelen
Software Engineer
Security Technologies
Red Hat, Inc.
Damien Miller
2018-10-14 23:18:52 UTC
Permalink
On Fri, 12 Oct 2018, Jakub Jelen wrote:

> Something like this can be used to properly initialize new OpenSSL
> versions:
>
> @@ -70,12 +70,19 @@ ssh_compatible_openssl(long headerver, long libver)
> void
> ssh_OpenSSL_add_all_algorithms(void)
> {
> +#if OPENSSL_VERSION_NUMBER < 0x10100000L
> OpenSSL_add_all_algorithms();
>
> /* Enable use of crypto hardware */
> ENGINE_load_builtin_engines();
> +#if OPENSSL_VERSION_NUMBER < 0x10001000L
> ENGINE_register_all_complete();
> +#endif
> OPENSSL_config(NULL);
> +#else
> + OPENSSL_init_crypto(OPENSSL_INIT_ADD_ALL_DIGESTS |
> + OPENSSL_INIT_ADD_ALL_DIGESTS | OPENSSL_INIT_LOAD_CONFIG,
> NULL);
> +#endif

I don't think the #ifs match the #endifs properly here - it leaves
the OPENSSL_init_crypto() call inside a #if OPENSSL_VERSION_NUMBER <
0x10100000L...

IMO this is simpler and better preserves the current flow of the code.
OpenSSL_add_all_algorithms() isn't marked as deprecated in the OpenSSL
headers, is used elsewhere in OpenSSH and is still used in most of
OpenSSL's demos/*, so I don't see any need to skip that ATM.

diff --git a/openbsd-compat/openssl-compat.c b/openbsd-compat/openssl-compat.c
index 259fccbe..762358f0 100644
--- a/openbsd-compat/openssl-compat.c
+++ b/openbsd-compat/openssl-compat.c
@@ -75,7 +75,13 @@ ssh_OpenSSL_add_all_algorithms(void)
/* Enable use of crypto hardware */
ENGINE_load_builtin_engines();
ENGINE_register_all_complete();
+
+#if OPENSSL_VERSION_NUMBER < 0x10001000L
OPENSSL_config(NULL);
+#else
+ OPENSSL_init_crypto(OPENSSL_INIT_ADD_ALL_CIPHERS |
+ OPENSSL_INIT_ADD_ALL_DIGESTS | OPENSSL_INIT_LOAD_CONFIG);
+#endif
}
#endif
Corinna Vinschen
2018-10-15 06:32:40 UTC
Permalink
On Oct 15 10:18, Damien Miller wrote:
> On Fri, 12 Oct 2018, Jakub Jelen wrote:
>
> > Something like this can be used to properly initialize new OpenSSL
> > versions:
> >
> > @@ -70,12 +70,19 @@ ssh_compatible_openssl(long headerver, long libver)
> > void
> > ssh_OpenSSL_add_all_algorithms(void)
> > {
> > +#if OPENSSL_VERSION_NUMBER < 0x10100000L
> > OpenSSL_add_all_algorithms();
> >
> > /* Enable use of crypto hardware */
> > ENGINE_load_builtin_engines();
> > +#if OPENSSL_VERSION_NUMBER < 0x10001000L
> > ENGINE_register_all_complete();
> > +#endif
> > OPENSSL_config(NULL);
> > +#else
> > + OPENSSL_init_crypto(OPENSSL_INIT_ADD_ALL_DIGESTS |
> > + OPENSSL_INIT_ADD_ALL_DIGESTS | OPENSSL_INIT_LOAD_CONFIG,
> > NULL);
> > +#endif
>
> I don't think the #ifs match the #endifs properly here - it leaves
> the OPENSSL_init_crypto() call inside a #if OPENSSL_VERSION_NUMBER <
> 0x10100000L...

#if bracketing is correct, afaics:

#if OPENSSL_VERSION_NUMBER < 0x10100000L
#if OPENSSL_VERSION_NUMBER < 0x10001000L
#endif
#else
#endif

There's only one OPENSSL_INIT_ADD_ALL_DIGESTS too many.


HTH,
Corinna

--
Corinna Vinschen
Cygwin Maintainer
Red Hat
Jakub Jelen
2018-10-16 10:04:56 UTC
Permalink
On Mon, 2018-10-15 at 08:32 +0200, Corinna Vinschen wrote:
> On Oct 15 10:18, Damien Miller wrote:
> > On Fri, 12 Oct 2018, Jakub Jelen wrote:
> >
> > > Something like this can be used to properly initialize new
> > > OpenSSL
> > > versions:
> > >
> > > @@ -70,12 +70,19 @@ ssh_compatible_openssl(long headerver, long
> > > libver)
> > > void
> > > ssh_OpenSSL_add_all_algorithms(void)
> > > {
> > > +#if OPENSSL_VERSION_NUMBER < 0x10100000L
> > > OpenSSL_add_all_algorithms();
> > >
> > > /* Enable use of crypto hardware */
> > > ENGINE_load_builtin_engines();
> > > +#if OPENSSL_VERSION_NUMBER < 0x10001000L
> > > ENGINE_register_all_complete();
> > > +#endif
> > > OPENSSL_config(NULL);
> > > +#else
> > > + OPENSSL_init_crypto(OPENSSL_INIT_ADD_ALL_DIGESTS |
> > > + OPENSSL_INIT_ADD_ALL_DIGESTS | OPENSSL_INIT_LOAD_CONFIG,
> > > NULL);
> > > +#endif
> >
> > I don't think the #ifs match the #endifs properly here - it leaves
> > the OPENSSL_init_crypto() call inside a #if OPENSSL_VERSION_NUMBER
> > <
> > 0x10100000L...
>
> #if bracketing is correct, afaics:
>
> #if OPENSSL_VERSION_NUMBER < 0x10100000L
> #if OPENSSL_VERSION_NUMBER < 0x10001000L
> #endif
> #else
> #endif

You are right.

> There's only one OPENSSL_INIT_ADD_ALL_DIGESTS too many.

Good catch. The one of them should probably have been
OPENSSL_INIT_ENGINE_ALL_BUILTIN.

The OpenSSL_add_all_algorithms() is described as deprecated in the
official documentation [1] and matches the functionality of the new
call OPENSSL_init_crypto().

[1]
https://www.openssl.org/docs/man1.1.0/crypto/OpenSSL_add_all_algorithms.html


--
Jakub Jelen
Software Engineer
Security Technologies
Red Hat, Inc.
Jakub Jelen
2018-10-16 18:13:39 UTC
Permalink
On Mon, 2018-10-15 at 10:18 +1100, Damien Miller wrote:
> On Fri, 12 Oct 2018, Jakub Jelen wrote:
>
> > Something like this can be used to properly initialize new OpenSSL
> > versions:
> >
> > @@ -70,12 +70,19 @@ ssh_compatible_openssl(long headerver, long
> > libver)
> > void
> > ssh_OpenSSL_add_all_algorithms(void)
> > {
> > +#if OPENSSL_VERSION_NUMBER < 0x10100000L
> > OpenSSL_add_all_algorithms();
> >
> > /* Enable use of crypto hardware */
> > ENGINE_load_builtin_engines();
> > +#if OPENSSL_VERSION_NUMBER < 0x10001000L
> > ENGINE_register_all_complete();
> > +#endif
> > OPENSSL_config(NULL);
> > +#else
> > + OPENSSL_init_crypto(OPENSSL_INIT_ADD_ALL_DIGESTS |
> > + OPENSSL_INIT_ADD_ALL_DIGESTS | OPENSSL_INIT_LOAD_CONFIG,
> > NULL);
> > +#endif
>
> I don't think the #ifs match the #endifs properly here - it leaves
> the OPENSSL_init_crypto() call inside a #if OPENSSL_VERSION_NUMBER <
> 0x10100000L...
>
> IMO this is simpler and better preserves the current flow of the
> code.
> OpenSSL_add_all_algorithms() isn't marked as deprecated in the
> OpenSSL
> headers, is used elsewhere in OpenSSH and is still used in most of
> OpenSSL's demos/*, so I don't see any need to skip that ATM.
>
> diff --git a/openbsd-compat/openssl-compat.c b/openbsd-
> compat/openssl-compat.c
> index 259fccbe..762358f0 100644
> --- a/openbsd-compat/openssl-compat.c
> +++ b/openbsd-compat/openssl-compat.c
> @@ -75,7 +75,13 @@ ssh_OpenSSL_add_all_algorithms(void)
> /* Enable use of crypto hardware */
> ENGINE_load_builtin_engines();
> ENGINE_register_all_complete();
> +
> +#if OPENSSL_VERSION_NUMBER < 0x10001000L
> OPENSSL_config(NULL);
> +#else
> + OPENSSL_init_crypto(OPENSSL_INIT_ADD_ALL_CIPHERS |
> + OPENSSL_INIT_ADD_ALL_DIGESTS | OPENSSL_INIT_LOAD_CONFIG);
> +#endif
> }
> #endif

The version in the last snap 20181017 (master commit [1]) is actually
missing the last (NULL) argument so the master/snap does not compile at
all now with new OpenSSL.

[1] https://github.com/openssh/openssh-portable/commit/4e23deef

Regards,
--
Jakub Jelen
Software Engineer
Security Technologies
Red Hat, Inc.
Jakub Jelen
2018-10-16 18:43:08 UTC
Permalink
On Tue, 2018-10-16 at 20:13 +0200, Jakub Jelen wrote:
> On Mon, 2018-10-15 at 10:18 +1100, Damien Miller wrote:
> > On Fri, 12 Oct 2018, Jakub Jelen wrote:
> >
> > > Something like this can be used to properly initialize new
> > > OpenSSL
> > > versions:
> > >
> > > @@ -70,12 +70,19 @@ ssh_compatible_openssl(long headerver, long
> > > libver)
> > > void
> > > ssh_OpenSSL_add_all_algorithms(void)
> > > {
> > > +#if OPENSSL_VERSION_NUMBER < 0x10100000L
> > > OpenSSL_add_all_algorithms();
> > >
> > > /* Enable use of crypto hardware */
> > > ENGINE_load_builtin_engines();
> > > +#if OPENSSL_VERSION_NUMBER < 0x10001000L
> > > ENGINE_register_all_complete();
> > > +#endif
> > > OPENSSL_config(NULL);
> > > +#else
> > > + OPENSSL_init_crypto(OPENSSL_INIT_ADD_ALL_DIGESTS |
> > > + OPENSSL_INIT_ADD_ALL_DIGESTS | OPENSSL_INIT_LOAD_CONFIG,
> > > NULL);
> > > +#endif
> >
> > I don't think the #ifs match the #endifs properly here - it leaves
> > the OPENSSL_init_crypto() call inside a #if OPENSSL_VERSION_NUMBER
> > <
> > 0x10100000L...
> >
> > IMO this is simpler and better preserves the current flow of the
> > code.
> > OpenSSL_add_all_algorithms() isn't marked as deprecated in the
> > OpenSSL
> > headers, is used elsewhere in OpenSSH and is still used in most of
> > OpenSSL's demos/*, so I don't see any need to skip that ATM.
> >
> > diff --git a/openbsd-compat/openssl-compat.c b/openbsd-
> > compat/openssl-compat.c
> > index 259fccbe..762358f0 100644
> > --- a/openbsd-compat/openssl-compat.c
> > +++ b/openbsd-compat/openssl-compat.c
> > @@ -75,7 +75,13 @@ ssh_OpenSSL_add_all_algorithms(void)
> > /* Enable use of crypto hardware */
> > ENGINE_load_builtin_engines();
> > ENGINE_register_all_complete();
> > +
> > +#if OPENSSL_VERSION_NUMBER < 0x10001000L
> > OPENSSL_config(NULL);
> > +#else
> > + OPENSSL_init_crypto(OPENSSL_INIT_ADD_ALL_CIPHERS |
> > + OPENSSL_INIT_ADD_ALL_DIGESTS | OPENSSL_INIT_LOAD_CONFIG);
> > +#endif
> > }
> > #endif
>
> The version in the last snap 20181017 (master commit [1]) is actually
> missing the last (NULL) argument so the master/snap does not compile
> at
> all now with new OpenSSL.
>
> [1] https://github.com/openssh/openssh-portable/commit/4e23deef

After fixing the missing argument above, all the tests pass for me on
Fedora 28 with OpenSSL 1.1.0

Thanks,
Jakub
Predrag Zecevic
2018-10-12 15:07:09 UTC
Permalink
On Thu, 2018-10-11 at 14:54 +1100, Damien Miller wrote:

> Hi,
>
> OpenSSH 7.9p1 is almost ready for release, so we would appreciate
> testing
> on as many platforms and systems as possible. This is a bugfix
> release.

Compilation has passed on OpenIndiana /hipster (GCC 8; linked against
self compiled OpenSSL and MIT Kerberos 5):

$ ssh -V
OpenSSH_7.9p1-snap20181013, OpenSSL 1.1.1  11 Sep 2018

With best regards.
Predrag Zečević
Larry Ploetz
2018-10-14 18:13:02 UTC
Permalink
All tests passed on Mac OS Mojave 10.14.1 beta

bash5-5.0$ uname -v
Darwin Kernel Version 18.2.0: Thu Sep 27 21:18:44 PDT 2018; root:xnu-4903.220.48~70/RELEASE_X86_64
bash5-5.0$ openssl version
OpenSSL 1.1.1 11 Sep 2018

--
Larry
Zev Weiss
2018-10-15 03:59:15 UTC
Permalink
On Wed, Oct 10, 2018 at 10:54:21PM CDT, Damien Miller wrote:
>
>Live testing on suitable non-production systems is also appreciated.
>Please send reports of success or failure to
>openssh-unix-***@mindrot.org. Security bugs should be reported
>directly to ***@openssh.com.
>

With git revision 797cdd9c8, all tests passed with on Void Linux, though
I hit a number of -wformat-trunction warnings during compilation (gcc
8.2).

fmt_scaled.c: In function 'fmt_scaled':
fmt_scaled.c:272:52: warning: '%1lld' directive output may be truncated writing between 1 and 17 bytes into a region of size between 0 and 5 [-Wformat-truncation=]
(void)snprintf(result, FMT_SCALED_STRSIZE, "%lld.%1lld%c",
^~~~~
fmt_scaled.c:272:46: note: directive argument in the range [-9007199254740991, 9]
(void)snprintf(result, FMT_SCALED_STRSIZE, "%lld.%1lld%c",
^~~~~~~~~~~~~~
In file included from /usr/include/stdio.h:873,
from /usr/include/bsd/libutil.h:49,
from ../includes.h:141,
from fmt_scaled.c:41:
/usr/include/bits/stdio2.h:67:10: note: '__builtin___snprintf_chk' output between 5 and 40 bytes into a destination of size 7
return __builtin___snprintf_chk (__s, __n, __USE_FORTIFY_LEVEL - 1,
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
__bos (__s), __fmt, __va_arg_pack ());
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

From my reading of the preceding code (lines 249-258), it looks to me
like the value of fract should be squarely in [0, 9] at that point, so
either I'm missing something subtle or gcc's value-range analysis is
confused. In reading the rest of fmt_scaled() I noticed it does call
llabs(number) without any range checks, which is undefined if number is
LONG_LONG_MIN, though for what it's worth another compile with a test
for that condition inserted before the llabs() call still produced the
same warning. (After some further experimentation with similar code in
a minimized context I'm somewhat more confident in my suspicion of a gcc
bug.)


sshkey.c: In function 'sshkey_format_cert_validity':
sshkey.c:2762:42: warning: '%s' directive output may be truncated writing up to 31 bytes into a region of size between 24 and 55 [-Wformat-truncation=]
snprintf(ret, sizeof(ret), "from %s to %s", from, to);
^~ ~~
In file included from /usr/include/stdio.h:873,
from /usr/include/bsd/libutil.h:49,
from includes.h:141,
from sshkey.c:28:
/usr/include/bits/stdio2.h:67:10: note: '__builtin___snprintf_chk' output between 10 and 72 bytes into a destination of size 64
return __builtin___snprintf_chk (__s, __n, __USE_FORTIFY_LEVEL - 1,
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
__bos (__s), __fmt, __va_arg_pack ());
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

With appropriate knowledge of strftime this one appears safe (at least
within the expected lifespan of the earth), but treating it as an opaque
function writing the 'from' and 'to' buffers the warning seems
legitimate (formatting two 31-byte strings plus 10 more bytes into a
64-byte destination buffer).


hostfile.c: In function 'host_hash':
hostfile.c:151:44: warning: '%s' directive output may be truncated writing up to 511 bytes into a region of size between 509 and 1020 [-Wformat-truncation=]
snprintf(encoded, sizeof(encoded), "%s%s%c%s", HASH_MAGIC, uu_salt,
^~
HASH_DELIM, uu_result);
~~~~~~~~~
In file included from /usr/include/stdio.h:873,
from /usr/include/bsd/libutil.h:49,
from includes.h:141,
from hostfile.c:39:
/usr/include/bits/stdio2.h:67:10: note: '__builtin___snprintf_chk' output between 5 and 1027 bytes into a destination of size 1024
return __builtin___snprintf_chk (__s, __n, __USE_FORTIFY_LEVEL - 1,
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
__bos (__s), __fmt, __va_arg_pack ());
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Likewise, with sufficient introspection into __b64_ntop I think we can
conclude this is safe, but from gcc's perspective it might not be.


sshconnect.c: In function 'check_host_key.constprop':
sshconnect.c:1027:8: warning: '%s' directive output may be truncated writing up to 1023 bytes into a region of size between 773 and 973 [-Wformat-truncation=]
"The authenticity of host '%.200s (%s)' can't be "
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
sshconnect.c:1032:18:
host, ip, msg1, type, fp,
~~~~
sshconnect.c:1028:20: note: format string is defined here
"established%s\n"
^~
In file included from /usr/include/stdio.h:873,
from /usr/include/bsd/libutil.h:49,
from includes.h:141,
from sshconnect.c:16:
/usr/include/bits/stdio2.h:67:10: note: '__builtin___snprintf_chk' output 130 or more bytes (assuming 2377) into a destination of size 1024
return __builtin___snprintf_chk (__s, __n, __USE_FORTIFY_LEVEL - 1,
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
__bos (__s), __fmt, __va_arg_pack ());
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

This one's in a somewhat more complex context and I haven't fully
analyzed it myself.


ssh-agent.c: In function 'main':
ssh-agent.c:1215:48: warning: '/agent.' directive output may be truncated writing 7 bytes into a region of size between 1 and 4096 [-Wformat-truncation=]
snprintf(socket_name, sizeof socket_name, "%s/agent.%ld", socket_dir,
^~~~~~~
ssh-agent.c:1215:45: note: directive argument in the range [-2147483648, 2147483647]
snprintf(socket_name, sizeof socket_name, "%s/agent.%ld", socket_dir,
^~~~~~~~~~~~~~
In file included from /usr/include/stdio.h:873,
from /usr/include/bsd/libutil.h:49,
from includes.h:141,
from ssh-agent.c:37:
/usr/include/bits/stdio2.h:67:10: note: '__builtin___snprintf_chk' output between 9 and 4114 bytes into a destination of size 4096
return __builtin___snprintf_chk (__s, __n, __USE_FORTIFY_LEVEL - 1,
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
__bos (__s), __fmt, __va_arg_pack ());
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Similar to the hostfile.c and sshkey.c ones above, though I think this
one might potentially be ticklable with sufficiently large values of
TMPDIR?


ssh-keygen.c: In function 'do_gen_all_hostkeys.isra.12':
ssh-keygen.c:1074:41: warning: '%s' directive output may be truncated writing up to 1024 bytes into a region of size 1023 [-Wformat-truncation=]
snprintf(comment, sizeof comment, "%s@%s", pw->pw_name,
^~
hostname);
~~~~~~~~
In file included from /usr/include/stdio.h:873,
from /usr/include/bsd/libutil.h:49,
from includes.h:141,
from ssh-keygen.c:15:
/usr/include/bits/stdio2.h:67:10: note: '__builtin___snprintf_chk' output 2 or more bytes (assuming 1026) into a destination of size 1024
return __builtin___snprintf_chk (__s, __n, __USE_FORTIFY_LEVEL - 1,
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
__bos (__s), __fmt, __va_arg_pack ());
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

(And another similar instance at line 2892.) I'm not aware of any
static limit on user name length analogous to NI_MAXHOST, so it seems
like these might need to be dynamically allocated for full generality,
though failing that making the comment buffer a few bytes bigger would
at least mollify the compiler.


ssh-keygen.c:340:34: warning: '%s' directive output may be truncated writing up to 1024 bytes into a region of size 39 [-Wformat-truncation=]
"%u-bit %s, converted by %s@%s from OpenSSH",
^~
ssh-keygen.c:342:19:
pw->pw_name, hostname);
~~~~~~~~
In file included from /usr/include/stdio.h:873,
from /usr/include/bsd/libutil.h:49,
from includes.h:141,
from ssh-keygen.c:15:
/usr/include/bits/stdio2.h:67:10: note: '__builtin___snprintf_chk' output 36 or more bytes (assuming 1060) into a destination of size 61
return __builtin___snprintf_chk (__s, __n, __USE_FORTIFY_LEVEL - 1,
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
__bos (__s), __fmt, __va_arg_pack ());
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

This one is similar to the others in this file, though I'm not sure why
the comment buffer is so much smaller here -- output format
restrictions? (If so, perhaps a "%.60s" printf format or something
instead of a small buffer?)


I also still (as has been happening for me for a while) get a bunch of
-Wimplicit-function-declaration warnings on various arc4random
functions, but I'm guessing that's not openssh's fault.



Zev
Jeff Wieland
2018-10-16 16:27:54 UTC
Permalink
Passes all tests on SPARC Solaris 10, 32 bit + large files,
using Solaris Studio 12.2, and our local build of OpenSSL 1.0.2p.

--
Jeff Wieland, UNIX/Network Systems Administrator
Purdue University IT Infrastructure Services UNIX Platforms
Jeff Wieland
2018-10-16 19:17:51 UTC
Permalink
Jeff Wieland wrote:
> Passes all tests on SPARC Solaris 10, 32 bit + large files,
> using Solaris Studio 12.2, and our local build of OpenSSL 1.0.2p.
>
> --
> Jeff Wieland, UNIX/Network Systems Administrator
> Purdue University IT Infrastructure Services UNIX Platforms
>

It also passed all tests using Solaris Studio 12.4 instead of 12.2.

--
Jeff Wieland, UNIX/Network Systems Administrator
Purdue University IT Infrastructure Services UNIX Platforms
Continue reading on narkive:
Loading...