Re: CVS commit: src/crypto/external/bsd/openssh/dist
On 2021/09/06 23:31, Taylor R Campbell wrote: Date: Mon, 6 Sep 2021 22:32:22 +0900 From: Rin Okuyama On 2021/09/06 22:11, Ryo ONODERA wrote: Module Name:src Committed By: ryoon Date: Mon Sep 6 13:11:34 UTC 2021 Modified Files: src/crypto/external/bsd/openssh/dist: dns.c Log Message: Make no diff to upstream This diff from upstream is intentional. See: http://cvsweb.netbsd.org/bsdweb.cgi/src/crypto/external/bsd/openssh/dist/dns.c#rev1.6 http://cvsweb.netbsd.org/bsdweb.cgi/src/crypto/external/bsd/openssh/dist/dns.c#rev1.7 At the time, I think upstream used memcmp, so anything different from that was a local change. Now our libc has consttime_memequal, and upstream uses a similar function called timingsafe_bcmp, so as long as timingsafe_bcmp is defined in terms of consttime_memequal (and not in terms of memcmp or bcmp), reducing this local diff strikes me as an improvement (speaking as the author of the original local change). Yeah, I agree. I didn't notice the definitions in includes.h. Thanks for your detailed explanation! rin
Re: CVS commit: src/crypto/external/bsd/openssh/dist
> Date: Mon, 6 Sep 2021 22:32:22 +0900 > From: Rin Okuyama > > On 2021/09/06 22:11, Ryo ONODERA wrote: > > Module Name:src > > Committed By: ryoon > > Date: Mon Sep 6 13:11:34 UTC 2021 > > > > Modified Files: > > src/crypto/external/bsd/openssh/dist: dns.c > > > > Log Message: > > Make no diff to upstream > > This diff from upstream is intentional. See: > > http://cvsweb.netbsd.org/bsdweb.cgi/src/crypto/external/bsd/openssh/dist/dns.c#rev1.6 > http://cvsweb.netbsd.org/bsdweb.cgi/src/crypto/external/bsd/openssh/dist/dns.c#rev1.7 At the time, I think upstream used memcmp, so anything different from that was a local change. Now our libc has consttime_memequal, and upstream uses a similar function called timingsafe_bcmp, so as long as timingsafe_bcmp is defined in terms of consttime_memequal (and not in terms of memcmp or bcmp), reducing this local diff strikes me as an improvement (speaking as the author of the original local change).
Re: CVS commit: src/crypto/external/bsd/openssh/dist
Hi, Rin Okuyama writes: > On 2021/09/06 23:11, Ryo ONODERA wrote: >> Hi, >> >> Rin Okuyama writes: >> >>> On 2021/09/06 22:11, Ryo ONODERA wrote: Module Name: src Committed By: ryoon Date: Mon Sep 6 13:11:34 UTC 2021 Modified Files: src/crypto/external/bsd/openssh/dist: dns.c Log Message: Make no diff to upstream To generate a diff of this commit: cvs rdiff -u -r1.20 -r1.21 src/crypto/external/bsd/openssh/dist/dns.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. >>> >>> This diff from upstream is intentional. See: >>> >>> http://cvsweb.netbsd.org/bsdweb.cgi/src/crypto/external/bsd/openssh/dist/dns.c#rev1.6 >>> http://cvsweb.netbsd.org/bsdweb.cgi/src/crypto/external/bsd/openssh/dist/dns.c#rev1.7 >> >> Thanks for your pointer. >> >> >> #define timingsafe_bcmp(a, b, c) (!consttime_memequal((a), (b), (c))) >> >> is in src/crypto/external/bsd/openssh/dist/includes.h. >> >> My change still uses consttime_memequal() practically like >> other places in OpenSSH. > > Ah, I got it. Thanks for explanation, and sorry for the noise! Sorry for my less explanation. I should write more information in the commit message. Thank you. > rin -- Ryo ONODERA // r...@tetera.org PGP fingerprint = 82A2 DC91 76E0 A10A 8ABB FD1B F404 27FA C7D1 15F3
Re: CVS commit: src/crypto/external/bsd/openssh/dist
On 2021/09/06 23:11, Ryo ONODERA wrote: Hi, Rin Okuyama writes: On 2021/09/06 22:11, Ryo ONODERA wrote: Module Name:src Committed By: ryoon Date: Mon Sep 6 13:11:34 UTC 2021 Modified Files: src/crypto/external/bsd/openssh/dist: dns.c Log Message: Make no diff to upstream To generate a diff of this commit: cvs rdiff -u -r1.20 -r1.21 src/crypto/external/bsd/openssh/dist/dns.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. This diff from upstream is intentional. See: http://cvsweb.netbsd.org/bsdweb.cgi/src/crypto/external/bsd/openssh/dist/dns.c#rev1.6 http://cvsweb.netbsd.org/bsdweb.cgi/src/crypto/external/bsd/openssh/dist/dns.c#rev1.7 Thanks for your pointer. #define timingsafe_bcmp(a, b, c) (!consttime_memequal((a), (b), (c))) is in src/crypto/external/bsd/openssh/dist/includes.h. My change still uses consttime_memequal() practically like other places in OpenSSH. Ah, I got it. Thanks for explanation, and sorry for the noise! rin
Re: CVS commit: src/crypto/external/bsd/openssh/dist
Hi, Rin Okuyama writes: > On 2021/09/06 22:11, Ryo ONODERA wrote: >> Module Name: src >> Committed By:ryoon >> Date:Mon Sep 6 13:11:34 UTC 2021 >> >> Modified Files: >> src/crypto/external/bsd/openssh/dist: dns.c >> >> Log Message: >> Make no diff to upstream >> >> >> To generate a diff of this commit: >> cvs rdiff -u -r1.20 -r1.21 src/crypto/external/bsd/openssh/dist/dns.c >> >> Please note that diffs are not public domain; they are subject to the >> copyright notices on the relevant files. > > This diff from upstream is intentional. See: > > http://cvsweb.netbsd.org/bsdweb.cgi/src/crypto/external/bsd/openssh/dist/dns.c#rev1.6 > http://cvsweb.netbsd.org/bsdweb.cgi/src/crypto/external/bsd/openssh/dist/dns.c#rev1.7 Thanks for your pointer. #define timingsafe_bcmp(a, b, c) (!consttime_memequal((a), (b), (c))) is in src/crypto/external/bsd/openssh/dist/includes.h. My change still uses consttime_memequal() practically like other places in OpenSSH. > Thanks, > rin -- Ryo ONODERA // r...@tetera.org PGP fingerprint = 82A2 DC91 76E0 A10A 8ABB FD1B F404 27FA C7D1 15F3
Re: CVS commit: src/crypto/external/bsd/openssh/dist
On 2021/09/06 22:11, Ryo ONODERA wrote: Module Name:src Committed By: ryoon Date: Mon Sep 6 13:11:34 UTC 2021 Modified Files: src/crypto/external/bsd/openssh/dist: dns.c Log Message: Make no diff to upstream To generate a diff of this commit: cvs rdiff -u -r1.20 -r1.21 src/crypto/external/bsd/openssh/dist/dns.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. This diff from upstream is intentional. See: http://cvsweb.netbsd.org/bsdweb.cgi/src/crypto/external/bsd/openssh/dist/dns.c#rev1.6 http://cvsweb.netbsd.org/bsdweb.cgi/src/crypto/external/bsd/openssh/dist/dns.c#rev1.7 Thanks, rin
Re: CVS commit: src/crypto/external/bsd/openssh/dist (reallocarray())
In article <20190309110959.ga3...@primenet.com.au>,
Geoff Wing wrote:
>On Saturday 2019-03-09 07:35 +1100, Christos Zoulas output:
>:Module Name: src
>:Committed By: christos
>:Date: Fri Mar 8 20:34:24 UTC 2019
>:
>:Modified Files:
>: src/crypto/external/bsd/openssh/dist: recallocarray.c
>:Log Message:
>:Replace our buggy recallocarray implementation one with the portable one
>:from OpenBSD.
>
>This is interesting. We still have libc/stdlib/reallocarr{,ay}.c
>which implements recallocarray() in -lc
>Every version of pkgsrc/news/inn I try - self built or downloaded from
>nyftp.netbsd* segvs due to reallocarray() unless I hack around those calls.
>Does this "external/bsd/openssh/..." fix only work for openssh? Or does this
>fix also affect the libc version we have? (Not sure why we are duplicating
>but I'll take any one which works)
>
>[PS not pleased that ISC wanted to use this reallocarray() stuff]
This is re"c"allocarray nor reallocararray() which is in libc.
christos
Re: CVS commit: src/crypto/external/bsd/openssh/dist (reallocarray())
On Saturday 2019-03-09 07:35 +1100, Christos Zoulas output:
:Module Name: src
:Committed By: christos
:Date: Fri Mar 8 20:34:24 UTC 2019
:
:Modified Files:
: src/crypto/external/bsd/openssh/dist: recallocarray.c
:Log Message:
:Replace our buggy recallocarray implementation one with the portable one
:from OpenBSD.
This is interesting. We still have libc/stdlib/reallocarr{,ay}.c
which implements recallocarray() in -lc
Every version of pkgsrc/news/inn I try - self built or downloaded from
nyftp.netbsd* segvs due to reallocarray() unless I hack around those calls.
Does this "external/bsd/openssh/..." fix only work for openssh? Or does this
fix also affect the libc version we have? (Not sure why we are duplicating
but I'll take any one which works)
[PS not pleased that ISC wanted to use this reallocarray() stuff]
Regards,
Geoff
Re: CVS commit: src/crypto/external/bsd/openssh/dist
On Feb 6, 1:26pm, m...@netbsd.org (m...@netbsd.org) wrote:
-- Subject: Re: CVS commit: src/crypto/external/bsd/openssh/dist
| > + const BIGNUM *pub_key;
| > + if ((r = dh_gen_key(kex->dh, kex->we_need * 8)) != 0)
| > + goto out;
| > + DH_get0_key(kex->dh, &pub_key, NULL);
| > + if ((r = sshpkt_start(ssh, SSH2_MSG_KEX_DH_GEX_INIT)) != 0 ||
| > + (r = sshpkt_put_bignum2(ssh, pub_key)) != 0 ||
| > + (r = sshpkt_send(ssh)) != 0) {
| > goto out;
| > + }
| > + }
| > debug("SSH2_MSG_KEX_DH_GEX_INIT sent");
| > #ifdef DEBUG_KEXDH
| > DHparams_print_fp(stderr, kex->dh);
| > @@ -134,10 +140,12 @@ input_kex_dh_gex_group(int type, u_int32
| > ssh_dispatch_set(ssh, SSH2_MSG_KEX_DH_GEX_REPLY,
&input_kex_dh_gex_reply);
| > r = 0;
| > out:
| > - if (p)
| > + if (r != 0) {
| > BN_clear_free(p);
| > - if (g)
| > BN_clear_free(g);
| > + DH_free(kex->dh);
| > + kex->dh = NULL;
| > + }
| > return r;
|
| BN_clear_free will null deref on this error path I think
void BN_clear_free(BIGNUM *a)
{
int i;
if (a == NULL)
return;
christos
Re: CVS commit: src/crypto/external/bsd/openssh/dist
On Tue, Feb 06, 2018 at 01:26:41PM +, m...@netbsd.org wrote: > BN_clear_free will null deref on this error path I think oops, I"m relying on ctags and went into the heimdal BN_clear_free instead which does this, and not the openssl which does NULL test.
Re: CVS commit: src/crypto/external/bsd/openssh/dist
> + const BIGNUM *pub_key;
> + if ((r = dh_gen_key(kex->dh, kex->we_need * 8)) != 0)
> + goto out;
> + DH_get0_key(kex->dh, &pub_key, NULL);
> + if ((r = sshpkt_start(ssh, SSH2_MSG_KEX_DH_GEX_INIT)) != 0 ||
> + (r = sshpkt_put_bignum2(ssh, pub_key)) != 0 ||
> + (r = sshpkt_send(ssh)) != 0) {
> goto out;
> + }
> + }
> debug("SSH2_MSG_KEX_DH_GEX_INIT sent");
> #ifdef DEBUG_KEXDH
> DHparams_print_fp(stderr, kex->dh);
> @@ -134,10 +140,12 @@ input_kex_dh_gex_group(int type, u_int32
> ssh_dispatch_set(ssh, SSH2_MSG_KEX_DH_GEX_REPLY,
> &input_kex_dh_gex_reply);
> r = 0;
> out:
> - if (p)
> + if (r != 0) {
> BN_clear_free(p);
> - if (g)
> BN_clear_free(g);
> + DH_free(kex->dh);
> + kex->dh = NULL;
> + }
> return r;
BN_clear_free will null deref on this error path I think
Re: CVS commit: src/crypto/external/bsd/openssh/dist
On Feb 6, 11:10am, m...@netbsd.org (m...@netbsd.org) wrote: -- Subject: Re: CVS commit: src/crypto/external/bsd/openssh/dist | On Sun, Feb 04, 2018 at 07:13:50PM -0500, Christos Zoulas wrote: | > - return BN_num_bits(k->rsa->n); | > +#if OPENSSL_VERSION_NUMBER >= 0x1010UL | > + return RSA_bits(k->rsa); | > +#else | > + return BN_num_bits(k->rsa->p); | > +#endif | > case KEY_DSA: | > case KEY_DSA_CERT: | > +#if OPENSSL_VERSION_NUMBER >= 0x1010UL | > + return DSA_bits(k->dsa); | > +#else | > return BN_num_bits(k->dsa->p); | > +#endif | | you put RSA_bits in openssl.old, can use it directly instead of macros? Yes, it seems that the linux patch was done by different people; some tried to maintain backwards compatibility, others did not. I guess the remaining 4 ifdefs in sshkey.c can be removed since the code is already using 1.1 functions unconditionally elsewhere. Please test before you commit if you do this. christos
Re: CVS commit: src/crypto/external/bsd/openssh/dist
On Sun, Feb 04, 2018 at 07:13:50PM -0500, Christos Zoulas wrote: > - return BN_num_bits(k->rsa->n); > +#if OPENSSL_VERSION_NUMBER >= 0x1010UL > + return RSA_bits(k->rsa); > +#else > + return BN_num_bits(k->rsa->p); > +#endif > case KEY_DSA: > case KEY_DSA_CERT: > +#if OPENSSL_VERSION_NUMBER >= 0x1010UL > + return DSA_bits(k->dsa); > +#else > return BN_num_bits(k->dsa->p); > +#endif you put RSA_bits in openssl.old, can use it directly instead of macros?
Re: CVS commit: src/crypto/external/bsd/openssh/dist
On Apr 19, 5:17pm, herb...@mailbox.org ("Herbert J. Skuhra") wrote:
-- Subject: Re: CVS commit: src/crypto/external/bsd/openssh/dist
| I think something went wrong with merging
| crypto/external/bsd/openssh/dist/sshd_config.
|
| r1.20 contains weird lines:
|
| Index: sshd_config
| ===
| RCS file: /cvsroot/src/crypto/external/bsd/openssh/dist/sshd_config,v
| retrieving revision 1.19
| retrieving revision 1.20
| diff -u -r1.19 -r1.20
| --- sshd_config 1 Feb 2017 14:27:37 - 1.19
| +++ sshd_config 18 Apr 2017 18:41:46 - 1.20
| @@ -1,5 +1,5 @@
| -# $NetBSD: sshd_config,v 1.19 2017/02/01 14:27:37 christos Exp $
| -# $OpenBSD: sshd_config,v 1.100 2016/08/15 12:32:04 naddy Exp $
| +# $NetBSD: sshd_config,v 1.20 2017/04/18 18:41:46 christos Exp $
| +# $OpenBSD: sshd_config,v 1.101 2017/03/14 07:19:07 djm Exp $
|
| # This is the sshd server system-wide configuration file. See
| # sshd_config(5) for more information.
| @@ -74,8 +74,11 @@
| #PrintLastLog yes
| #TCPKeepAlive yes
| #UseLogin no
| +<<<<<<< sshd_config
| #UsePrivilegeSeparation sandbox
| UsePam yes
| +===
| +>>>>>>> 1.1.1.15
| #PermitUserEnvironment no
| #Compression delayed
| #ClientAliveInterval 0
|
Thanks, I fixed that exact problem, so I don't know why it is still incorrect.
I will fix it again.
christos
Re: CVS commit: src/crypto/external/bsd/openssh/dist
Christos Zoulas skrev: > > Module Name: src > Committed By: christos > Date: Tue Apr 18 18:41:46 UTC 2017 > > Modified Files: > src/crypto/external/bsd/openssh/dist: addrmatch.c atomicio.c atomicio.h > auth-bsdauth.c auth-krb5.c auth-options.c auth-options.h auth-pam.c > auth-pam.h auth-passwd.c auth-rhosts.c auth-skey.c auth.c auth.h > auth2-chall.c auth2-gss.c auth2-hostbased.c auth2-kbdint.c > auth2-krb5.c auth2-none.c auth2-passwd.c auth2-pubkey.c auth2.c > authfd.c authfd.h authfile.c authfile.h bcrypt_pbkdf.c bitmap.c > bitmap.h blocks.c blowfish.c bufaux.c bufbn.c bufec.c buffer.c > buffer.h canohost.c canohost.h chacha.c channels.c channels.h > cipher-3des1.c cipher-bf1.c cipher-chachapoly.c cipher-ctr-mt.c > cipher.c cipher.h cleanup.c clientloop.c clientloop.h compat.c > compat.h crc32.c crc32.h deattack.c deattack.h dh.c dh.h > digest-libc.c digest-openssl.c dispatch.c dispatch.h dns.c dns.h > ed25519.c fatal.c fe25519.c fmt_scaled.c fmt_scaled.h ge25519.c > getpeereid.c getpeereid.h getrrsetbyname.c getrrsetbyname.h > groupaccess.c groupaccess.h gss-genr.c gss-serv-krb5.c gss-serv.c > hash.c hmac.c hostfile.c hostfile.h includes.h kex.c kex.h > kexc25519.c kexc25519c.c kexc25519s.c kexdh.c kexdhc.c kexdhs.c > kexecdh.c kexecdhc.c kexecdhs.c kexgex.c kexgexc.c kexgexs.c key.c > key.h krl.c ldapauth.c ldapauth.h log.c log.h mac.c mac.h match.c > match.h md-sha256.c misc.c misc.h moduli.5 moduli.c monitor.c > monitor.h monitor_fdpass.c monitor_fdpass.h monitor_wrap.c > monitor_wrap.h msg.c msg.h mux.c myproposal.h namespace.h nchan.c > opacket.c opacket.h packet.c packet.h pathnames.h pkcs11.h > poly1305.c progressmeter.c progressmeter.h random.h readconf.c > readconf.h readpass.c readpassphrase.3 readpassphrase.c > readpassphrase.h rsa.c rsa.h sandbox-rlimit.c sc25519.c scp.1 scp.c > servconf.c servconf.h serverloop.c serverloop.h session.c session.h > sftp-client.c sftp-client.h sftp-common.c sftp-common.h sftp-glob.c > sftp-server-main.c sftp-server.8 sftp-server.c sftp.1 sftp.c sftp.h > smult_curve25519_ref.c ssh-add.1 ssh-add.c ssh-agent.1 ssh-agent.c > ssh-dss.c ssh-ecdsa.c ssh-ed25519.c ssh-gss.h ssh-keygen.1 > ssh-keygen.c ssh-keyscan.1 ssh-keyscan.c ssh-keysign.8 > ssh-keysign.c ssh-pkcs11-client.c ssh-pkcs11-helper.8 > ssh-pkcs11-helper.c ssh-pkcs11.c ssh-pkcs11.h ssh-rsa.c ssh.1 ssh.c > ssh.h ssh1.h ssh2.h ssh_api.c ssh_api.h ssh_config.5 > sshbuf-getput-basic.c sshbuf-getput-crypto.c sshbuf-misc.c sshbuf.c > sshconnect.c sshconnect.h sshconnect1.c sshconnect2.c sshd.8 sshd.c > sshd_config sshd_config.5 ssherr.c sshkey.c sshkey.h sshlogin.c > sshlogin.h sshpty.c sshpty.h sshtty.c ttymodes.c ttymodes.h > uidswap.c uidswap.h umac.c umac.h utf8.c uuencode.c uuencode.h > verify.c version.h xmalloc.c xmalloc.h > > Log Message: > merge conflicts I think something went wrong with merging crypto/external/bsd/openssh/dist/sshd_config. r1.20 contains weird lines: Index: sshd_config === RCS file: /cvsroot/src/crypto/external/bsd/openssh/dist/sshd_config,v retrieving revision 1.19 retrieving revision 1.20 diff -u -r1.19 -r1.20 --- sshd_config 1 Feb 2017 14:27:37 - 1.19 +++ sshd_config 18 Apr 2017 18:41:46 - 1.20 @@ -1,5 +1,5 @@ -# $NetBSD: sshd_config,v 1.19 2017/02/01 14:27:37 christos Exp $ -# $OpenBSD: sshd_config,v 1.100 2016/08/15 12:32:04 naddy Exp $ +# $NetBSD: sshd_config,v 1.20 2017/04/18 18:41:46 christos Exp $ +# $OpenBSD: sshd_config,v 1.101 2017/03/14 07:19:07 djm Exp $ # This is the sshd server system-wide configuration file. See # sshd_config(5) for more information. @@ -74,8 +74,11 @@ #PrintLastLog yes #TCPKeepAlive yes #UseLogin no +<<< sshd_config #UsePrivilegeSeparation sandbox UsePam yes +=== +>>> 1.1.1.15 #PermitUserEnvironment no #Compression delayed #ClientAliveInterval 0 Thanks. -- Herbert
Re: CVS commit: src/crypto/external/bsd/openssh/dist
On Sun, 06 Oct 2013, Jean-Yves Migeon wrote: Modified Files: src/crypto/external/bsd/openssh/dist: ssh_config Log Message: Enable VerifyHostKeyDNS (SSHFP records verification) from DNS for hosts under NetBSD.org domain. Thank you. I think this is an improvement. Notified on netbsd-users@, no objection after a week -- committed. Please discuss such things in the relevant tech-* list (tech-net or tech-userlevel in this case, I suppose). +# NetBSD.org DNS provides SSHFP records - use them when possible +Host *.netbsd.org *.NetBSD.org +VerifyHostKeyDNS ask I have been running similar configuration for some time, but with with "VerifyHostKeyDNS yes" (not "ask"), and I have had no problems. The difference between "yes" and "ask" arises only when the ssh client can be sure that the DNS answer was secured by DNSSEC; in such a case, "yes" means accept the result silently, while "ask" means ask the user (the first time). If the DNS answer was not secured by DNSSEC, then both "yes" and "ask" end up asking the user. By the way, I think that's a bug in ssh that the Host patterns are case sensitive. --apb (Alan Barrett)
Re: CVS commit: src/crypto/external/bsd/openssh/dist
> On Mon Jan 03 2011 at 18:55:42 +, Arnaud Ysmal wrote: > > Module Name: src > > Committed By: stacktic > > Date: Mon Jan 3 18:55:42 UTC 2011 > > > > Modified Files: > > src/crypto/external/bsd/openssh/dist: sshconnect2.c > > > > Log Message: > > Fixed strvisx usage > > Didn't you fix that already once before? Can the fix be upstreamed? Yes I did. There is no problem upstream, they are using strnvis, which takes the length of the destination buffer as a parameter, while we are using strvisx, which takes the length of the input buffer. This fix should be part of the fixes we merge for each new version of openssh.
Re: CVS commit: src/crypto/external/bsd/openssh/dist
On Mon Jan 03 2011 at 18:55:42 +, Arnaud Ysmal wrote: > Module Name: src > Committed By: stacktic > Date: Mon Jan 3 18:55:42 UTC 2011 > > Modified Files: > src/crypto/external/bsd/openssh/dist: sshconnect2.c > > Log Message: > Fixed strvisx usage Didn't you fix that already once before? Can the fix be upstreamed? -- älä karot toivorikkauttas, kyl rätei ja lumpui piisaa
