Re: OpenSSH: RSA/SHA1 disabled by default

2021-09-07 Thread Damien Miller
On Tue, 7 Sep 2021, Martijn van Duren wrote:

> On Mon, 2021-08-30 at 10:08 +1000, Damien Miller wrote:
> > Hi,
> > 
> > RSA/SHA1, a.k.a the "ssh-rsa" signature type is now disabled by default
> > in OpenSSH.
> > 
> > While The SSH protocol confusingly uses overlapping names for key and
> > signature algorithms, this does not stop the use of RSA keys and there
> > is no need to regenerate "ssh-rsa" keys - most servers released in the
> > last five years will automatically negotiate the use of RSA/SHA-256/512
> > signatures.
> > 
> > This has been coming for a long time, but I do expect it will be
> > distruptive for some people as there are likely to be some devices
> > out there that cannot be upgraded to support the safer algorithms.
> > 
> > In these cases, it is possible to selectively re-enable RSA/SHA1
> > support by specifying PubkeyAcceptedAlgorithms=+ssh-rsa in the
> > ssh_config(5) or sshd_config(5) for the endpoint.
> > 
> > Please report any problems here, to bugs@ or to openssh@
> > 
> > Thanks,
> > Damien
> > 
> Just did an update to the latest snapshot and this breaks connection
> to one of the older hosts I still need to connect to from time to time.
> 
> Reverting this diff fixes the issue for me.
> 
> According to -G it should work:
> 
> $ ssh -G -oPubkeyAcceptedAlgorithms=ssh-rsa 10.255.3.242 | grep -i 
> PubkeyAcceptedAlgorithms
> pubkeyacceptedalgorithms ssh-rsa
> 
> But when trying it for real I get the following:
> martijn$ ssh - -oPubkeyAcceptedAlgorithms=ssh-rsa x.x.x.x
> OpenSSH_8.7, LibreSSL 3.4.0
[snip]
> Unable to negotiate with x.x.x.x port 22: no matching host key type found. 
> Their offer: ssh-rsa,ssh-dss
> 
> Same difference when using -oPubkeyAcceptedAlgorithms=+ssh-rsa, or
> placing it in the ssh_config(5).

This is a case of the host key algorithm not matching, so you
should use HostKeyAlgorithms=+ssh-rsa - I'll make sure to mention
this in the release notes.

PubkeyAcceptedAlgorithms is for user authentication. Generally,
you should use the "Option=+algorithm" form rather than just
"Option=algorithm" - the former adds the algorithm to the end of
the list, so if the destination upgrades its crypto then you're
not stuck using the old algorithm.

-d




snaps: scp uses SFTP protocol by default

2021-09-05 Thread Damien Miller
Hi,

Just letting you know that the snaps rolling out now have scp defaulting
to use the SFTP protocol by default. We hope to keep this change in the
next release, so please report any problems you encounter either here
(tech@), to bugs@ or to openssh@.

One thing to be aware of: copying to/from a remote path that includes a
~user/ prefix requires a SFTP protocol extension that is only available
in OpenSSH 8.7 and later. If you need this, and upgrading the server is
not an option, then you should force the use of the scp/rcp protocol
using the -O flag.

Thanks,
Damien



OpenSSH: RSA/SHA1 disabled by default

2021-08-29 Thread Damien Miller
Hi,

RSA/SHA1, a.k.a the "ssh-rsa" signature type is now disabled by default
in OpenSSH.

While The SSH protocol confusingly uses overlapping names for key and
signature algorithms, this does not stop the use of RSA keys and there
is no need to regenerate "ssh-rsa" keys - most servers released in the
last five years will automatically negotiate the use of RSA/SHA-256/512
signatures.

This has been coming for a long time, but I do expect it will be
distruptive for some people as there are likely to be some devices
out there that cannot be upgraded to support the safer algorithms.

In these cases, it is possible to selectively re-enable RSA/SHA1
support by specifying PubkeyAcceptedAlgorithms=+ssh-rsa in the
ssh_config(5) or sshd_config(5) for the endpoint.

Please report any problems here, to bugs@ or to openssh@

Thanks,
Damien

-- Forwarded message --
Date: Mon, 30 Aug 2021 09:53:10
From: Damien Miller 
To: source-chan...@cvs.openbsd.org
Subject: CVS: cvs.openbsd.org: src

CVSROOT:/cvs
Module name:src
Changes by: d...@cvs.openbsd.org2021/08/29 17:53:10

Modified files:
usr.bin/ssh: myproposal.h 

Log message:
After years of forewarning, disable the RSA/SHA-1 signature algorithm
by default. It is feasible to create colliding SHA1 hashes, so we
need to deprecate its use.

RSA/SHA-256/512 remains available and will be transparently selected
instead of RSA/SHA1 for most SSH servers released in the last five+
years. There is no need to regenerate RSA keys.

The use of RSA/SHA1 can be re-enabled by adding "ssh-rsa" to the
PubkeyAcceptedAlgorithms directives on the client and server.

ok dtucker deraadt



Re: ssh match.c: Remove always true condition

2021-08-19 Thread Damien Miller
On Thu, 19 Aug 2021, Martin Vahlensieck wrote:

> Ping.
> 
> On Tue, Aug 10, 2021 at 04:33:52PM +0200, Martin Vahlensieck wrote:
> > Ping, diff reattached with extra context for easier review.
> > 
> > On Wed, Jul 21, 2021 at 12:10:31PM +0200, Martin Vahlensieck wrote:
> > > Hi
> > > 
> > > After the last commit where consecutive `*' are folded, *pattern is
> > > never '*' here.

I prefer to leave this as-is because it documents the intention of the
code and because there's still more work to do in fixing recursion
problems in that function.

-d



Re: scp(1) changes in snaps

2021-08-07 Thread Damien Miller
On Fri, 6 Aug 2021, Christian Weisgerber wrote:

> Damien Miller:
> 
> > Just a head-up: snaps currently contain a set of changes[1] to
> > make scp(1) use the SFTP protocol by default.
> 
> > Please report any incompatibilities or bugs that you encounter here
> > (tech@), to bugs@ or to openssh@.
> 
> An obvious cosmetic difference is that relative paths are prefixed
> with the home directory of the remote source in the progress bar:
> 
> $ scp lorvorc:foo /dev/null
> /home/naddy/foo   100% 4099 1.6MB/s   00:00   
>  
> $ scp -O lorvorc:foo /dev/null
> foo   100% 4099 3.7MB/s   00:00   
>  
> 
> I don't know if we care.

Yeah, I'm inclined to leave the full paths unless there's a good argument
for truncating them.

-d



scp(1) changes in snaps

2021-08-05 Thread Damien Miller
Hi,

Just a head-up: snaps currently contain a set of changes[1] to
make scp(1) use the SFTP protocol by default. This has a number of
advantages, mostly relating to the improved security that comes from
avoiding the use of a protocol that shambled out of the 1980s (SCP/RCP).

A certain amount in incompatibility is to be expected: the SCP/RCP
protocol implicitly uses the remote shell for glob expansion, and this
can make quoting/escpaing problematic as it gets expanded twice, by both
the local and remote shells. SFTP-based scp(1) avoids this and functions
a lot more like what you'd typically expect. Given this, I consider this
an improvement and so I'm leaning towards not trying to make the new
code bug-compatible with SCP/RCP quoting.

Note that the code supporting scp -3 over SFTP is very new and not very
well-tested. So if you are a user of this feature then please give it a
try.

Please report any incompatibilities or bugs that you encounter here
(tech@), to bugs@ or to openssh@.

Thanks,
Damien

[1] https://github.com/djmdjm/openssh-wip/pull/7



ssh/sshd configuration parsing

2021-06-08 Thread Damien Miller
Hi,

I just committed some changes to ssh/sshd configuration parsing that
have been in snaps for the last few days. These changes switch parsing
from using a naive tokeniser to one that better follows shell-style
rules for quoting and comments.

This does make config parsing stricter in a number of cases, e.g. it
was previously possible for sshd_config to have a AllowUsers option
alone on a line with no arguments (it was pretty nonsensical to do so,
since it had zero effect), but the new parser will reject this as well
as a few other weird cases.

The benefits of the new code are better handling of quoted strings,
e.g. with escaped quotes and a fix for a regression caused by adding
support for comments in ssh_config a few releases ago.

These changes do touch something that is likely used in ways that I
haven't thought of and the regress tests don't cover :) If you spot
weirdness, regressions or your previously-valid configurations do not
parse afterwards, then please let let bugs@ or openssh@ know ASAP.

Thanks,
Damien



Re: ssh: zap unused family parameter from ssh_connect_direct()

2020-10-11 Thread Damien Miller
ok djm

On Sun, 11 Oct 2020, Klemens Nanni wrote:

> CVS log shows that the following commit removed usage of it:
> 
>   sshconnect.c
>   revision 1.241
>   date: 2013/10/16 02:31:46;  author: djm;  state: Exp;  lines: +29 -45;
>   Implement client-side hostname canonicalisation to allow an explicit
>   search path of domain suffixes to use to convert unqualified host names
>   to fully-qualified ones for host key matching.
>   [...]
> 
> So it is unused ever since in the only call chain:
> ssh(1) main() -> ssh_connect() -> ssh_connect_direct().
> 
> I came here after reading the code when ssh(1)'s `-4' would not effect
> jump hosts, i.e. `-J' or `ProxyJump'... only to find out later that I
> didn't read the manual properly in the first place:
> 
>   -J destination
>   [...]
>   Note that configuration directives supplied on the command-line
>   generally apply to the destination host and not any specified
>   jump hosts.  Use ~/.ssh/config to specify configuration for jump
>   hosts.
> 
> Compiles and works fine as before.
> Feedback? Objections? OK?
> 
> 
> Index: ssh.c
> ===
> RCS file: /cvs/src/usr.bin/ssh/ssh.c,v
> retrieving revision 1.537
> diff -u -p -r1.537 ssh.c
> --- ssh.c 3 Oct 2020 09:22:26 -   1.537
> +++ ssh.c 10 Oct 2020 00:35:49 -
> @@ -1521,7 +1521,7 @@ main(int ac, char **av)
>  
>   /* Open a connection to the remote host. */
>   if (ssh_connect(ssh, host, host_arg, addrs, , options.port,
> - options.address_family, options.connection_attempts,
> + options.connection_attempts,
>   _ms, options.tcp_keep_alive) != 0)
>   exit(255);
>  
> Index: sshconnect.c
> ===
> RCS file: /cvs/src/usr.bin/ssh/sshconnect.c,v
> retrieving revision 1.339
> diff -u -p -r1.339 sshconnect.c
> --- sshconnect.c  7 Oct 2020 02:26:28 -   1.339
> +++ sshconnect.c  10 Oct 2020 00:35:47 -
> @@ -420,8 +420,8 @@ fail:
>   */
>  static int
>  ssh_connect_direct(struct ssh *ssh, const char *host, struct addrinfo *aitop,
> -struct sockaddr_storage *hostaddr, u_short port, int family,
> -int connection_attempts, int *timeout_ms, int want_keepalive)
> +struct sockaddr_storage *hostaddr, u_short port, int connection_attempts,
> +int *timeout_ms, int want_keepalive)
>  {
>   int on = 1, saved_timeout_ms = *timeout_ms;
>   int oerrno, sock = -1, attempt;
> @@ -511,13 +511,13 @@ ssh_connect_direct(struct ssh *ssh, cons
>  int
>  ssh_connect(struct ssh *ssh, const char *host, const char *host_arg,
>  struct addrinfo *addrs, struct sockaddr_storage *hostaddr, u_short port,
> -int family, int connection_attempts, int *timeout_ms, int want_keepalive)
> +int connection_attempts, int *timeout_ms, int want_keepalive)
>  {
>   int in, out;
>  
>   if (options.proxy_command == NULL) {
>   return ssh_connect_direct(ssh, host, addrs, hostaddr, port,
> - family, connection_attempts, timeout_ms, want_keepalive);
> + connection_attempts, timeout_ms, want_keepalive);
>   } else if (strcmp(options.proxy_command, "-") == 0) {
>   if ((in = dup(STDIN_FILENO)) == -1 ||
>   (out = dup(STDOUT_FILENO)) == -1) {
> Index: sshconnect.h
> ===
> RCS file: /cvs/src/usr.bin/ssh/sshconnect.h,v
> retrieving revision 1.42
> diff -u -p -r1.42 sshconnect.h
> --- sshconnect.h  7 Oct 2020 02:22:23 -   1.42
> +++ sshconnect.h  10 Oct 2020 00:36:25 -
> @@ -35,7 +35,7 @@ struct ssh;
>  
>  int   ssh_connect(struct ssh *, const char *, const char *,
>   struct addrinfo *, struct sockaddr_storage *, u_short,
> - int, int, int *, int);
> + int, int *, int);
>  void  ssh_kill_proxy_command(void);
>  
>  void  ssh_login(struct ssh *, Sensitive *, const char *,
> 
> 



Re: sync libfido2 with upstream

2020-08-20 Thread Damien Miller
On Mon, 17 Aug 2020, Damien Miller wrote:

> On Mon, 10 Aug 2020, Damien Miller wrote:
> 
> > Hi,
> > 
> > This syncs libfido2 with the current state of upstream. It includes
> > a few new APIs that I want to use in OpenSSH to improve FIDO token
> > support (require-PIN and fixing some corner-case bugs around multiple
> > inserted tokens).
> > 
> > ok?
> > 
> > (major crank for ABI change)
> 
> So I pounced on the new API a bit too soon and before it stabilised.
> There have been a couple more changes upstream that I need.
> 
> Sorry for the unneccessary churn.
> 
> ok?

I'd like to commit this. Ok?

(to be clear - nothing in src/ yet uses the APIs affected by this change)

Index: README.openbsd
===
RCS file: /cvs/src/lib/libfido2/README.openbsd,v
retrieving revision 1.3
diff -u -p -r1.3 README.openbsd
--- README.openbsd  11 Aug 2020 08:44:53 -  1.3
+++ README.openbsd  21 Aug 2020 01:42:03 -
@@ -1,4 +1,4 @@
-This is an import of https://github.com/Yubico/libfido2 2fa20b889 (20200810)
+This is an import of https://github.com/Yubico/libfido2 46710ac06 (20200815)
 
 Local changes:
 
Index: shlib_version
===
RCS file: /cvs/src/lib/libfido2/shlib_version,v
retrieving revision 1.4
diff -u -p -r1.4 shlib_version
--- shlib_version   11 Aug 2020 08:44:53 -  1.4
+++ shlib_version   21 Aug 2020 01:42:03 -
@@ -1,2 +1,2 @@
-major=3
+major=4
 minor=0
Index: man/fido_dev_get_touch_begin.3
===
RCS file: /cvs/src/lib/libfido2/man/fido_dev_get_touch_begin.3,v
retrieving revision 1.1
diff -u -p -r1.1 fido_dev_get_touch_begin.3
--- man/fido_dev_get_touch_begin.3  11 Aug 2020 08:44:53 -  1.1
+++ man/fido_dev_get_touch_begin.3  21 Aug 2020 01:42:03 -
@@ -14,7 +14,7 @@
 .Ft int
 .Fn fido_dev_get_touch_begin "fido_dev_t *dev"
 .Ft int
-.Fn fido_dev_get_touch_status "fido_dev_t *dev" "int *touched" "int *pin_set" 
"int ms"
+.Fn fido_dev_get_touch_status "fido_dev_t *dev" "int *touched" "int ms"
 .Sh DESCRIPTION
 The functions described in this page allow an application to
 asynchronously wait for touch on a FIDO authenticator.
Index: man/fido_dev_open.3
===
RCS file: /cvs/src/lib/libfido2/man/fido_dev_open.3,v
retrieving revision 1.4
diff -u -p -r1.4 fido_dev_open.3
--- man/fido_dev_open.3 11 Aug 2020 08:44:53 -  1.4
+++ man/fido_dev_open.3 21 Aug 2020 01:42:03 -
@@ -16,6 +16,7 @@
 .Nm fido_dev_is_fido2 ,
 .Nm fido_dev_supports_cred_prot ,
 .Nm fido_dev_supports_pin ,
+.Nm fido_dev_has_pin ,
 .Nm fido_dev_protocol ,
 .Nm fido_dev_build ,
 .Nm fido_dev_flags ,
@@ -44,6 +45,8 @@
 .Fn fido_dev_supports_cred_prot "const fido_dev_t *dev"
 .Ft bool
 .Fn fido_dev_supports_pin "const fido_dev_t *dev"
+.Ft bool
+.Fn fido_dev_has_pin "const fido_dev_t *dev"
 .Ft uint8_t
 .Fn fido_dev_protocol "const fido_dev_t *dev"
 .Ft uint8_t
@@ -137,6 +140,14 @@ function returns
 if
 .Fa dev
 supports FIDO 2.0 Client PINs.
+.Pp
+The
+.Fn fido_dev_has_pin
+function returns
+.Dv true
+if
+.Fa dev
+has a FIDO 2.0 Client PIN set.
 .Pp
 The
 .Fn fido_dev_protocol
Index: src/dev.c
===
RCS file: /cvs/src/lib/libfido2/src/dev.c,v
retrieving revision 1.3
diff -u -p -r1.3 dev.c
--- src/dev.c   11 Aug 2020 08:44:53 -  1.3
+++ src/dev.c   21 Aug 2020 01:42:03 -
@@ -123,30 +123,27 @@ static void
 fido_dev_set_flags(fido_dev_t *dev, const fido_cbor_info_t *info)
 {
char * const*ptr;
+   const bool  *val;
size_t   len;
 
ptr = fido_cbor_info_extensions_ptr(info);
len = fido_cbor_info_extensions_len(info);
 
-   for (size_t i = 0; i < len; i++) {
-   if (strcmp(ptr[i], "credProtect") == 0) {
-   dev->flags |= FIDO_DEV_SUPPORTS_CRED_PROT;
-   }
-   }
+   for (size_t i = 0; i < len; i++)
+   if (strcmp(ptr[i], "credProtect") == 0)
+   dev->flags |= FIDO_DEV_CRED_PROT;
 
ptr = fido_cbor_info_options_name_ptr(info);
+   val = fido_cbor_info_options_value_ptr(info);
len = fido_cbor_info_options_len(info);
 
-   for (size_t i = 0; i < len; i++) {
-   /*
-* clientPin: PIN supported and set;
-* noclientPin: PIN supported but not set.
-*/
-   if (strcmp(ptr[i], "clientPin") == 0 ||
-   strcmp(ptr[i], "noclientPin") == 0) {
-   dev->

Re: sync libfido2 with upstream

2020-08-17 Thread Damien Miller
On Mon, 10 Aug 2020, Damien Miller wrote:

> Hi,
> 
> This syncs libfido2 with the current state of upstream. It includes
> a few new APIs that I want to use in OpenSSH to improve FIDO token
> support (require-PIN and fixing some corner-case bugs around multiple
> inserted tokens).
> 
> ok?
> 
> (major crank for ABI change)

So I pounced on the new API a bit too soon and before it stabilised.
There have been a couple more changes upstream that I need.

Sorry for the unneccessary churn.

ok?

-d

Index: README.openbsd
===
RCS file: /cvs/src/lib/libfido2/README.openbsd,v
retrieving revision 1.3
diff -u -p -r1.3 README.openbsd
--- README.openbsd  11 Aug 2020 08:44:53 -  1.3
+++ README.openbsd  17 Aug 2020 06:13:36 -
@@ -1,4 +1,4 @@
-This is an import of https://github.com/Yubico/libfido2 2fa20b889 (20200810)
+This is an import of https://github.com/Yubico/libfido2 46710ac06 (20200810)
 
 Local changes:
 
Index: shlib_version
===
RCS file: /cvs/src/lib/libfido2/shlib_version,v
retrieving revision 1.4
diff -u -p -r1.4 shlib_version
--- shlib_version   11 Aug 2020 08:44:53 -  1.4
+++ shlib_version   17 Aug 2020 06:13:36 -
@@ -1,2 +1,2 @@
-major=3
+major=4
 minor=0
Index: man/fido_dev_get_touch_begin.3
===
RCS file: /cvs/src/lib/libfido2/man/fido_dev_get_touch_begin.3,v
retrieving revision 1.1
diff -u -p -r1.1 fido_dev_get_touch_begin.3
--- man/fido_dev_get_touch_begin.3  11 Aug 2020 08:44:53 -  1.1
+++ man/fido_dev_get_touch_begin.3  17 Aug 2020 06:13:36 -
@@ -14,7 +14,7 @@
 .Ft int
 .Fn fido_dev_get_touch_begin "fido_dev_t *dev"
 .Ft int
-.Fn fido_dev_get_touch_status "fido_dev_t *dev" "int *touched" "int *pin_set" 
"int ms"
+.Fn fido_dev_get_touch_status "fido_dev_t *dev" "int *touched" "int ms"
 .Sh DESCRIPTION
 The functions described in this page allow an application to
 asynchronously wait for touch on a FIDO authenticator.
Index: man/fido_dev_open.3
===
RCS file: /cvs/src/lib/libfido2/man/fido_dev_open.3,v
retrieving revision 1.4
diff -u -p -r1.4 fido_dev_open.3
--- man/fido_dev_open.3 11 Aug 2020 08:44:53 -  1.4
+++ man/fido_dev_open.3 17 Aug 2020 06:13:36 -
@@ -16,6 +16,7 @@
 .Nm fido_dev_is_fido2 ,
 .Nm fido_dev_supports_cred_prot ,
 .Nm fido_dev_supports_pin ,
+.Nm fido_dev_has_pin ,
 .Nm fido_dev_protocol ,
 .Nm fido_dev_build ,
 .Nm fido_dev_flags ,
@@ -44,6 +45,8 @@
 .Fn fido_dev_supports_cred_prot "const fido_dev_t *dev"
 .Ft bool
 .Fn fido_dev_supports_pin "const fido_dev_t *dev"
+.Ft bool
+.Fn fido_dev_has_pin "const fido_dev_t *dev"
 .Ft uint8_t
 .Fn fido_dev_protocol "const fido_dev_t *dev"
 .Ft uint8_t
@@ -137,6 +140,14 @@ function returns
 if
 .Fa dev
 supports FIDO 2.0 Client PINs.
+.Pp
+The
+.Fn fido_dev_has_pin
+function returns
+.Dv true
+if
+.Fa dev
+has a FIDO 2.0 Client PIN set.
 .Pp
 The
 .Fn fido_dev_protocol
Index: src/dev.c
===
RCS file: /cvs/src/lib/libfido2/src/dev.c,v
retrieving revision 1.3
diff -u -p -r1.3 dev.c
--- src/dev.c   11 Aug 2020 08:44:53 -  1.3
+++ src/dev.c   17 Aug 2020 06:13:36 -
@@ -123,30 +123,27 @@ static void
 fido_dev_set_flags(fido_dev_t *dev, const fido_cbor_info_t *info)
 {
char * const*ptr;
+   const bool  *val;
size_t   len;
 
ptr = fido_cbor_info_extensions_ptr(info);
len = fido_cbor_info_extensions_len(info);
 
-   for (size_t i = 0; i < len; i++) {
-   if (strcmp(ptr[i], "credProtect") == 0) {
-   dev->flags |= FIDO_DEV_SUPPORTS_CRED_PROT;
-   }
-   }
+   for (size_t i = 0; i < len; i++)
+   if (strcmp(ptr[i], "credProtect") == 0)
+   dev->flags |= FIDO_DEV_CRED_PROT;
 
ptr = fido_cbor_info_options_name_ptr(info);
+   val = fido_cbor_info_options_value_ptr(info);
len = fido_cbor_info_options_len(info);
 
-   for (size_t i = 0; i < len; i++) {
-   /*
-* clientPin: PIN supported and set;
-* noclientPin: PIN supported but not set.
-*/
-   if (strcmp(ptr[i], "clientPin") == 0 ||
-   strcmp(ptr[i], "noclientPin") == 0) {
-   dev->flags |= FIDO_DEV_SUPPORTS_PIN;
+   for (size_t i = 0; i < len; i++)
+   if (strcmp(ptr[i], "clientPin") == 0) {
+   if (val[i] == true)
+   dev->flags |= FIDO_DEV_PIN_SET;
+

Re: Fwd: explicit_bzero vs. alternatives

2020-08-10 Thread Damien Miller
On Mon, 10 Aug 2020, Amit Kulkarni wrote:

> moving to tech@
> 
> -- Forwarded message -
> From: Philipp Klaus Krause 
> Date: Mon, Aug 10, 2020 at 4:34 AM
> Subject: explicit_bzero vs. alternatives
> To: 
> 
> 
> OpenBSD has the explicit_bzero function to reliably (i.e. even if not
> observable in the C abstract machine) overwrite memory with zeroes.
> 
> WG14 is currently considering adding similar functionality to C2X.
> 
> Considered options include:
> 
> * A function like explicit_bzero or memset_explicit, that overwrites the
> memory with a known value.
> * A function like memclear, that overwrites the memory in an
> implementation-defined manner, possibly using random data.
> 
> Is there a rationale why OpenBSD went with their explicit_bzero design?
> Were alternatives considered and rejected?

We went with explict_bzero because our only use-case for this was
safe erasure that could not be elided by the compiler.

I don't see any need for explicit_memset() - if anything depends on
the overwritten value then simple memset() should be sufficient as
the compiler should detect the dependency and refuse to elide the
memset() to begin with.

Likewise, I can see no benefit for overwriting with random data. Doing
this is always going to be more expensive and more likely to leak
secrets, e.g. the length of cleared objects.

Hopefully C2X is taking a more broad approach to this problem than
considering new library calls. Over-eager optimisation (especially when
done at link-time over the whole program) is a major for anyone trying
to write safe C code.

-d



sync libfido2 with upstream

2020-08-09 Thread Damien Miller
Hi,

This syncs libfido2 with the current state of upstream. It includes
a few new APIs that I want to use in OpenSSH to improve FIDO token
support (require-PIN and fixing some corner-case bugs around multiple
inserted tokens).

ok?

(major crank for ABI change)

Index: Makefile
===
RCS file: /cvs/src/lib/libfido2/Makefile,v
retrieving revision 1.5
diff -u -p -r1.5 Makefile
--- Makefile7 Feb 2020 00:57:49 -   1.5
+++ Makefile10 Aug 2020 02:02:50 -
@@ -30,7 +30,7 @@ MAN+= fido_cbor_info_new.3 fido_cred_exc
 MAN+=  fido_cred_set_authdata.3 fido_cred_verify.3 fido_credman_metadata_new.3
 MAN+=  fido_dev_get_assert.3 fido_dev_info_manifest.3 fido_dev_make_cred.3
 MAN+=  fido_dev_open.3 fido_dev_set_io_functions.3 fido_dev_set_pin.3
-MAN+=  fido_init.3 fido_strerr.3 rs256_pk_new.3
+MAN+=  fido_init.3 fido_strerr.3 rs256_pk_new.3 fido_dev_get_touch_begin.3
 
 includes:
@for i in $(HDRS); do \
Index: README.openbsd
===
RCS file: /cvs/src/lib/libfido2/README.openbsd,v
retrieving revision 1.2
diff -u -p -r1.2 README.openbsd
--- README.openbsd  7 Feb 2020 00:57:49 -   1.2
+++ README.openbsd  10 Aug 2020 02:02:50 -
@@ -1,4 +1,4 @@
-This is an import of https://github.com/Yubico/libfido2 780ad3c25 (20120123)
+This is an import of https://github.com/Yubico/libfido2 2fa20b889c (20200810)
 
 Local changes:
 
Index: shlib_version
===
RCS file: /cvs/src/lib/libfido2/shlib_version,v
retrieving revision 1.3
diff -u -p -r1.3 shlib_version
--- shlib_version   7 Feb 2020 00:57:49 -   1.3
+++ shlib_version   10 Aug 2020 02:02:50 -
@@ -1,2 +1,2 @@
-major=2
+major=3
 minor=0
Index: man/fido_assert_new.3
===
RCS file: /cvs/src/lib/libfido2/man/fido_assert_new.3,v
retrieving revision 1.3
diff -u -p -r1.3 fido_assert_new.3
--- man/fido_assert_new.3   7 Feb 2020 00:57:49 -   1.3
+++ man/fido_assert_new.3   10 Aug 2020 02:02:50 -
@@ -9,6 +9,7 @@
 .Nm fido_assert_new ,
 .Nm fido_assert_free ,
 .Nm fido_assert_count ,
+.Nm fido_assert_rp_id ,
 .Nm fido_assert_user_display_name ,
 .Nm fido_assert_user_icon ,
 .Nm fido_assert_user_name ,
@@ -17,12 +18,15 @@
 .Nm fido_assert_hmac_secret_ptr ,
 .Nm fido_assert_user_id_ptr ,
 .Nm fido_assert_sig_ptr ,
+.Nm fido_assert_id_ptr ,
 .Nm fido_assert_authdata_len ,
 .Nm fido_assert_clientdata_hash_len ,
 .Nm fido_assert_hmac_secret_len ,
 .Nm fido_assert_user_id_len ,
 .Nm fido_assert_sig_len ,
-.Nm fido_assert_sigcount
+.Nm fido_assert_id_len ,
+.Nm fido_assert_sigcount ,
+.Nm fido_assert_flags
 .Nd FIDO 2 assertion API
 .Sh SYNOPSIS
 .In fido.h
@@ -33,6 +37,8 @@
 .Ft size_t
 .Fn fido_assert_count "const fido_assert_t *assert"
 .Ft const char *
+.Fn fido_assert_rp_id "const fido_assert_t *assert"
+.Ft const char *
 .Fn fido_assert_user_display_name "const fido_assert_t *assert" "size_t idx"
 .Ft const char *
 .Fn fido_assert_user_icon "const fido_assert_t *assert" "size_t idx"
@@ -48,6 +54,8 @@
 .Fn fido_assert_user_id_ptr "const fido_assert_t *assert" "size_t idx"
 .Ft const unsigned char *
 .Fn fido_assert_sig_ptr "const fido_assert_t *assert" "size_t idx"
+.Ft const unsigned char *
+.Fn fido_assert_id_ptr "const fido_assert_t *assert" "size_t idx"
 .Ft size_t
 .Fn fido_assert_authdata_len "const fido_assert_t *assert" "size_t idx"
 .Ft size_t
@@ -58,8 +66,12 @@
 .Fn fido_assert_user_id_len "const fido_assert_t *assert" "size_t idx"
 .Ft size_t
 .Fn fido_assert_sig_len "const fido_assert_t *assert" "size_t idx"
+.Ft size_t
+.Fn fido_assert_id_len "const fido_assert_t *assert" "size_t idx"
 .Ft uint32_t
 .Fn fido_assert_sigcount "const fido_assert_t *assert" "size_t idx"
+.Ft uint8_t
+.Fn fido_assert_flags "const fido_assert_t *assert" "size_t idx"
 .Sh DESCRIPTION
 FIDO 2 assertions are abstracted in
 .Em libfido2
@@ -110,6 +122,12 @@ function returns the number of statement
 .Fa assert .
 .Pp
 The
+.Fn fido_assert_rp_id
+function returns a pointer to a NUL-terminated string holding the
+relying party ID of
+.Fa assert .
+.Pp
+The
 .Fn fido_assert_user_display_name ,
 .Fn fido_assert_user_icon ,
 and
@@ -126,10 +144,11 @@ The
 .Fn fido_assert_user_id_ptr ,
 .Fn fido_assert_authdata_ptr ,
 .Fn fido_assert_hmac_secret_ptr ,
+.Fn fido_assert_sig_ptr ,
 and
-.Fn fido_assert_sig_ptr
+.Fn fido_assert_id_ptr
 functions return pointers to the user ID, authenticator data,
-hmac-secret, and signature attributes of statement
+hmac-secret, signature, and credential ID attributes of statement
 .Fa idx
 in
 .Fa assert .
@@ -137,14 +156,22 @@ The
 .Fn fido_assert_user_id_len ,
 .Fn fido_assert_authdata_len ,
 .Fn fido_assert_hmac_secret_len ,
+.Fn fido_assert_sig_len ,
 and
-.Fn fido_assert_sig_len
+.Fn fido_assert_id_len
 functions can be used to retrieve the 

sync libcbor to 0.7.0

2020-07-29 Thread Damien Miller
Hi,

This syncs lib/libcbor from our v0.5.0+patches to the released v0.7.0

AFAIK the changes are mostly inconsequential to the current uses
in-tree (there is a stack exhaustion fix that is worth having), but
being at an actual release rather than a frankenpatch will make future
updates a bit easier.

Major bump because of one API removal.

ok?

Index: Makefile
===
RCS file: /cvs/src/lib/libcbor/Makefile,v
retrieving revision 1.2
diff -u -p -r1.2 Makefile
--- Makefile15 Nov 2019 03:19:39 -  1.2
+++ Makefile30 Jul 2020 04:31:37 -
@@ -10,6 +10,7 @@ SRCS= cbor.c
 
 WARNINGS=yes
 CDIAGFLAGS+=   -Wall -Wextra -Wno-unused-parameter
+CDIAGFLAGS+=   -Wno-missing-field-initializers
 #CDIAGFLAGS+=  -Werror
 
 # cbor/
Index: README.openbsd
===
RCS file: /cvs/src/lib/libcbor/README.openbsd,v
retrieving revision 1.1
diff -u -p -r1.1 README.openbsd
--- README.openbsd  14 Nov 2019 21:11:34 -  1.1
+++ README.openbsd  30 Jul 2020 04:31:37 -
@@ -1,4 +1,4 @@
-This is an import of https://github.com/pjk/libcbor v0.5.0
+This is an import of https://github.com/pjk/libcbor v0.7.0
 
 Apart from README.md and LICENSE.md, only the src/ directory has been
 imported.
Index: shlib_version
===
RCS file: /cvs/src/lib/libcbor/shlib_version,v
retrieving revision 1.3
diff -u -p -r1.3 shlib_version
--- shlib_version   28 Nov 2019 21:18:16 -  1.3
+++ shlib_version   30 Jul 2020 04:31:37 -
@@ -1,2 +1,2 @@
-major=0
-minor=6
+major=1
+minor=0
Index: src/allocators.c
===
RCS file: /cvs/src/lib/libcbor/src/allocators.c,v
retrieving revision 1.2
diff -u -p -r1.2 allocators.c
--- src/allocators.c28 Nov 2019 02:58:39 -  1.2
+++ src/allocators.c30 Jul 2020 04:31:37 -
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2014-2019 Pavel Kalvoda 
+ * Copyright (c) 2014-2020 Pavel Kalvoda 
  *
  * libcbor is free software; you can redistribute it and/or modify
  * it under the terms of the MIT license. See LICENSE for details.
Index: src/cbor.c
===
RCS file: /cvs/src/lib/libcbor/src/cbor.c,v
retrieving revision 1.2
diff -u -p -r1.2 cbor.c
--- src/cbor.c  28 Nov 2019 02:58:39 -  1.2
+++ src/cbor.c  30 Jul 2020 04:31:37 -
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2014-2019 Pavel Kalvoda 
+ * Copyright (c) 2014-2020 Pavel Kalvoda 
  *
  * libcbor is free software; you can redistribute it and/or modify
  * it under the terms of the MIT license. See LICENSE for details.
@@ -323,8 +323,7 @@ static void _cbor_nested_describe(cbor_i
   fprintf(out, "%*s[CBOR_TYPE_FLOAT_CTRL] ", indent, " ");
   if (cbor_float_ctrl_is_ctrl(item)) {
 if (cbor_is_bool(item))
-  fprintf(out, "Bool: %s\n",
-  cbor_ctrl_is_bool(item) ? "true" : "false");
+  fprintf(out, "Bool: %s\n", cbor_get_bool(item) ? "true" : "false");
 else if (cbor_is_undef(item))
   fprintf(out, "Undefined\n");
 else if (cbor_is_null(item))
Index: src/cbor.h
===
RCS file: /cvs/src/lib/libcbor/src/cbor.h,v
retrieving revision 1.2
diff -u -p -r1.2 cbor.h
--- src/cbor.h  28 Nov 2019 02:58:39 -  1.2
+++ src/cbor.h  30 Jul 2020 04:31:37 -
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2014-2019 Pavel Kalvoda 
+ * Copyright (c) 2014-2020 Pavel Kalvoda 
  *
  * libcbor is free software; you can redistribute it and/or modify
  * it under the terms of the MIT license. See LICENSE for details.
Index: src/cbor/arrays.c
===
RCS file: /cvs/src/lib/libcbor/src/cbor/arrays.c,v
retrieving revision 1.2
diff -u -p -r1.2 arrays.c
--- src/cbor/arrays.c   28 Nov 2019 02:58:39 -  1.2
+++ src/cbor/arrays.c   30 Jul 2020 04:31:37 -
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2014-2019 Pavel Kalvoda 
+ * Copyright (c) 2014-2020 Pavel Kalvoda 
  *
  * libcbor is free software; you can redistribute it and/or modify
  * it under the terms of the MIT license. See LICENSE for details.
Index: src/cbor/arrays.h
===
RCS file: /cvs/src/lib/libcbor/src/cbor/arrays.h,v
retrieving revision 1.2
diff -u -p -r1.2 arrays.h
--- src/cbor/arrays.h   28 Nov 2019 02:58:39 -  1.2
+++ src/cbor/arrays.h   30 Jul 2020 04:31:37 -
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2014-2019 Pavel Kalvoda 
+ * Copyright (c) 2014-2020 Pavel Kalvoda 
  *
  * libcbor is free software; you can redistribute it and/or modify
  * it under the terms of the MIT license. See LICENSE for details.
Index: src/cbor/bytestrings.c
===
RCS file: 

Re: ssh: Register tun channel cleanup callback when ExitOnForwardFailure is set

2020-02-17 Thread Damien Miller
On Mon, 17 Feb 2020, Remi Pommarel wrote:

> When remote side fails to create tun (e.g. tun device is already opened)
> it notifies the client with an SSH2_MSG_CHANNEL_OPEN_FAILURE message and
> channel is marked dead on client side. But because tun forward channel
> is not an interactive channel it has no cleanup callback and is kept in
> a Zombie state until ssh is manually terminated.
> 
> This makes "ssh -Nw" to not fail and exit in such situation even if
> ExitOnForwardFailure is set.
> 
> This patch registers a cleanup callback to tun forward channel if
> ExitOnForwardFailure is set so that "ssh -Nw" exit directly when
> remote fails to established the tunnel correctly on its side.

Please try this patch. It handles tunnel forwarding failures via
the same logic as remote forwards.

diff --git a/clientloop.c b/clientloop.c
index f7ce3a2..f35ccfb 100644
--- a/clientloop.c
+++ b/clientloop.c
@@ -1641,7 +1641,7 @@ client_request_agent(struct ssh *ssh, const char 
*request_type, int rchan)
 
 char *
 client_request_tun_fwd(struct ssh *ssh, int tun_mode,
-int local_tun, int remote_tun)
+int local_tun, int remote_tun, channel_open_fn *cb, void *cbctx)
 {
Channel *c;
int r, fd;
@@ -1663,6 +1663,9 @@ client_request_tun_fwd(struct ssh *ssh, int tun_mode,
CHAN_TCP_WINDOW_DEFAULT, CHAN_TCP_PACKET_DEFAULT, 0, "tun", 1);
c->datagram = 1;
 
+   if (cb != NULL)
+   channel_register_open_confirm(ssh, c->self, cb, cbctx);
+
if ((r = sshpkt_start(ssh, SSH2_MSG_CHANNEL_OPEN)) != 0 ||
(r = sshpkt_put_cstring(ssh, "t...@openssh.com")) != 0 ||
(r = sshpkt_put_u32(ssh, c->self)) != 0 ||
diff --git a/clientloop.h b/clientloop.h
index bf79c87..4604e7c 100644
--- a/clientloop.h
+++ b/clientloop.h
@@ -46,7 +46,8 @@ intclient_x11_get_proto(struct ssh *, const char *, const 
char *,
 voidclient_global_request_reply_fwd(int, u_int32_t, void *);
 voidclient_session2_setup(struct ssh *, int, int, int,
const char *, struct termios *, int, struct sshbuf *, char **);
-char*client_request_tun_fwd(struct ssh *, int, int, int);
+char*client_request_tun_fwd(struct ssh *, int, int, int,
+channel_open_fn *, void *);
 voidclient_stop_mux(void);
 
 /* Escape filter for protocol 2 sessions */
diff --git a/ssh.c b/ssh.c
index 314b3c5..37a693c 100644
--- a/ssh.c
+++ b/ssh.c
@@ -175,7 +175,7 @@ struct sshbuf *command;
 int subsystem_flag = 0;
 
 /* # of replies received for global requests */
-static int remote_forward_confirms_received = 0;
+static int forward_confirms_pending = -1;
 
 /* mux.c */
 extern int muxserver_sock;
@@ -1640,6 +1640,16 @@ fork_postauth(void)
fatal("daemon() failed: %.200s", strerror(errno));
 }
 
+static void
+forwarding_success(void)
+{
+   if (forward_confirms_pending > 0 && --forward_confirms_pending == 0) {
+   debug("All forwarding requests processed");
+   if (fork_after_authentication_flag)
+   fork_postauth();
+   }
+}
+
 /* Callback for remote forward global requests */
 static void
 ssh_confirm_remote_forward(struct ssh *ssh, int type, u_int32_t seq, void 
*ctxt)
@@ -1699,11 +1709,7 @@ ssh_confirm_remote_forward(struct ssh *ssh, int type, 
u_int32_t seq, void *ctxt)
"for listen port %d", rfwd->listen_port);
}
}
-   if (++remote_forward_confirms_received == options.num_remote_forwards) {
-   debug("All remote forwarding requests processed");
-   if (fork_after_authentication_flag)
-   fork_postauth();
-   }
+   forwarding_success();
 }
 
 static void
@@ -1720,6 +1726,19 @@ ssh_stdio_confirm(struct ssh *ssh, int id, int success, 
void *arg)
fatal("stdio forwarding failed");
 }
 
+static void
+ssh_tun_confirm(struct ssh *ssh, int id, int success, void *arg)
+{
+   if (!success) {
+   error("Tunnel forwarding failed");
+   if (options.exit_on_forward_failure)
+   cleanup_exit(255);
+   }
+
+   debug("%s: tunnel forward established, id=%d", __func__, id);
+   forwarding_success();
+}
+
 static void
 ssh_init_stdio_forwarding(struct ssh *ssh)
 {
@@ -1783,32 +1802,29 @@ ssh_init_forwarding(struct ssh *ssh, char **ifname)
options.remote_forwards[i].connect_path :
options.remote_forwards[i].connect_host,
options.remote_forwards[i].connect_port);
-   options.remote_forwards[i].handle =
+   if ((options.remote_forwards[i].handle =
channel_request_remote_forwarding(ssh,
-   _forwards[i]);
-   if (options.remote_forwards[i].handle < 0) {
-   if (options.exit_on_forward_failure)
-   fatal("Could not request remote forwarding.");
-   else
- 

Add #define for RFC8622 IPTOS_DSCP_LE codepoint

2020-01-25 Thread Damien Miller
Hi,

This adds a #define for the "lower effort" DSCP code point specified
by https://tools.ietf.org/html/rfc8622

People have asked to be able to use this OpenSSH for "don't care"
traffic.

ok?

Index: sys/netinet/ip.h
===
RCS file: /cvs/src/sys/netinet/ip.h,v
retrieving revision 1.18
diff -u -p -r1.18 ip.h
--- sys/netinet/ip.h8 Aug 2017 18:25:31 -   1.18
+++ sys/netinet/ip.h25 Jan 2020 12:34:27 -
@@ -98,6 +98,7 @@ struct ip {
  * Definitions for DiffServ Codepoints as per RFC2474
  */
 #defineIPTOS_DSCP_CS0  0x00
+#defineIPTOS_DSCP_LE   0x01
 #defineIPTOS_DSCP_CS1  0x20
 #defineIPTOS_DSCP_AF11 0x28
 #defineIPTOS_DSCP_AF12 0x30



Re: sshd proctitle [Re: CVS: cvs.openbsd.org: src]

2020-01-24 Thread Damien Miller
On Fri, 24 Jan 2020, Stuart Henderson wrote:

> That works - etc/rc.d/sshd diff to match as follows:
> 
> Index: sshd
> ===
> RCS file: /cvs/src/etc/rc.d/sshd,v
> retrieving revision 1.5
> diff -u -p -r1.5 sshd
> --- sshd  22 Jan 2020 13:14:51 -  1.5
> +++ sshd  24 Jan 2020 11:59:52 -
> @@ -6,7 +6,7 @@ daemon="/usr/sbin/sshd"
>  
>  . /etc/rc.d/rc.subr
>  
> -pexp="sshd: \[listener\].*"
> +pexp="sshd: ${daemon}${daemon_flags:+ ${daemon_flags}} \[listener\].*"
>  
>  rc_reload() {
>   ${daemon} ${daemon_flags} -t && pkill -HUP -xf "${pexp}"

ok djm



Re: sshd proctitle [Re: CVS: cvs.openbsd.org: src]

2020-01-24 Thread Damien Miller
On Fri, 24 Jan 2020, Antoine Jacoutot wrote:

> Great :-)
> Ok aja 

committed, the proctitle looks like this now in case the rc scripts need
further tweaking:

$ pgrep -lf sshd
12844 sshd: /usr/sbin/sshd -f /etc/ssh/sshd_config [listener] 0 of 10-100 
startups



Re: sshd proctitle [Re: CVS: cvs.openbsd.org: src]

2020-01-23 Thread Damien Miller
On Thu, 23 Jan 2020, Damien Miller wrote:

> On Thu, 23 Jan 2020, Damien Miller wrote:
> 
> > What information would you like there? We could put the first N listen
> > addrs in the proctitle if that would help.
> 
> Maybe like this:
> 
> 63817 ??  S0:00.05 sshd: [listen] on [0.0.0.0]:22, [::]:22, 0 of 
> 10-100

antoine@ said this was not sufficient, so please try the following:

63817 ??  I0:00.09 sshd: /usr/sbin/sshd [listener] 0 of 10-100 startups


diff --git a/sshd.c b/sshd.c
index f6139fe..b7ed0f3 100644
--- a/sshd.c
+++ b/sshd.c
@@ -240,6 +240,8 @@ void destroy_sensitive_data(void);
 void demote_sensitive_data(void);
 static void do_ssh2_kex(struct ssh *);
 
+static char *listener_proctitle;
+
 /*
  * Close all listening sockets
  */
@@ -1032,9 +1034,9 @@ server_accept_loop(int *sock_in, int *sock_out, int 
*newsock, int *config_s)
 */
for (;;) {
if (ostartups != startups) {
-   setproctitle("[listener] %d of %d-%d startups",
-   startups, options.max_startups_begin,
-   options.max_startups);
+   setproctitle("%s [listener] %d of %d-%d startups",
+   listener_proctitle, startups,
+   options.max_startups_begin, options.max_startups);
ostartups = startups;
}
if (received_sighup) {
@@ -1347,6 +1349,41 @@ accumulate_host_timing_secret(struct sshbuf *server_cfg,
sshbuf_free(buf);
 }
 
+static void
+xextendf(char **s, const char *sep, const char *fmt, ...)
+__attribute__((__format__ (printf, 3, 4))) __attribute__((__nonnull__ 
(3)));
+static void
+xextendf(char **sp, const char *sep, const char *fmt, ...)
+{
+   va_list ap;
+   char *tmp1, *tmp2;
+
+   va_start(ap, fmt);
+   xvasprintf(, fmt, ap);
+   va_end(ap);
+
+   if (*sp == NULL || **sp == '\0') {
+   free(*sp);
+   *sp = tmp1;
+   return;
+   }
+   xasprintf(, "%s%s%s", *sp, sep, tmp1);
+   free(tmp1);
+   free(*sp);
+   *sp = tmp2;
+}
+
+static char *
+prepare_proctitle(int ac, char **av)
+{
+   char *ret = NULL;
+   int i;
+
+   for (i = 0; i < ac; i++)
+   xextendf(, " ", "%s", av[i]);
+   return ret;
+}
+
 /*
  * Main program for the daemon.
  */
@@ -1774,6 +1811,7 @@ main(int ac, char **av)
rexec_argv[rexec_argc] = "-R";
rexec_argv[rexec_argc + 1] = NULL;
}
+   listener_proctitle = prepare_proctitle(ac, av);
 
/* Ensure that umask disallows at least group and world write */
new_umask = umask(0077) | 0022;



Re: sshd proctitle [Re: CVS: cvs.openbsd.org: src]

2020-01-22 Thread Damien Miller
On Thu, 23 Jan 2020, Damien Miller wrote:

> On Wed, 22 Jan 2020, Stuart Henderson wrote:
> 
> > On 2020/01/21 15:39, Damien Miller wrote:
> > > CVSROOT:  /cvs
> > > Module name:  src
> > > Changes by:   d...@cvs.openbsd.org2020/01/21 15:39:57
> > > 
> > > Modified files:
> > >   usr.bin/ssh: sshd.c 
> > > 
> > > Log message:
> > > expose the number of currently-authenticating connections
> > > along with the MaxStartups limit in the proctitle;
> > > suggestion from Philipp Marek, w/ feedback from Craig Miskell
> > > ok dtucker@
> > > 
> > 
> > It's nice to have this information visible, but it brings some problems.
> > You can't now distinguish between multiple sshd processes (e.g. if you
> > run several on different ports it's hard to figure out which one to
> > signal if needed).
> 
> How could you discern between different sshd processes before? Just the
> command-line args?
> 
> What information would you like there? We could put the first N listen
> addrs in the proctitle if that would help.

Maybe like this:

63817 ??  S0:00.05 sshd: [listen] on [0.0.0.0]:22, [::]:22, 0 of 10-100

ok?

diff --git a/sshd.c b/sshd.c
index ec644c9..15014d1 100644
--- a/sshd.c
+++ b/sshd.c
@@ -240,6 +240,9 @@ void destroy_sensitive_data(void);
 void demote_sensitive_data(void);
 static void do_ssh2_kex(struct ssh *);
 
+/* Listen info for proctitle */
+static char *proctitle_listen_addr;
+
 /*
  * Close all listening sockets
  */
@@ -913,7 +916,7 @@ listen_on_addrs(struct listenaddr *la)
 {
int ret, listen_sock;
struct addrinfo *ai;
-   char ntop[NI_MAXHOST], strport[NI_MAXSERV];
+   char *cp, ntop[NI_MAXHOST], strport[NI_MAXSERV];
 
for (ai = la->addrs; ai; ai = ai->ai_next) {
if (ai->ai_family != AF_INET && ai->ai_family != AF_INET6)
@@ -973,6 +976,15 @@ listen_on_addrs(struct listenaddr *la)
ntop, strport,
la->rdomain == NULL ? "" : " rdomain ",
la->rdomain == NULL ? "" : la->rdomain);
+   if (num_listen_socks < 3) {
+   cp = proctitle_listen_addr;
+   xasprintf(_listen_addr, "%s%s[%s]:%s%s%s",
+   cp == NULL ? "" : cp, cp == NULL ? "" : ", ",
+   ntop, strport,
+   la->rdomain == NULL ? "" : " rdomain ",
+   la->rdomain == NULL ? "" : la->rdomain);
+   free(cp);
+   }
}
 }
 
@@ -1030,7 +1042,9 @@ server_accept_loop(int *sock_in, int *sock_out, int 
*newsock, int *config_s)
 */
for (;;) {
if (ostartups != startups) {
-   setproctitle("[listener] %d of %d-%d startups",
+   setproctitle("[listen] on %s%s, "
+   "%d of %d-%d startups", proctitle_listen_addr,
+   num_listen_socks > 3 ? " [...]" : "",
startups, options.max_startups_begin,
options.max_startups);
ostartups = startups;



Re: sshd proctitle [Re: CVS: cvs.openbsd.org: src]

2020-01-22 Thread Damien Miller
On Wed, 22 Jan 2020, Stuart Henderson wrote:

> On 2020/01/21 15:39, Damien Miller wrote:
> > CVSROOT:/cvs
> > Module name:src
> > Changes by: d...@cvs.openbsd.org2020/01/21 15:39:57
> > 
> > Modified files:
> > usr.bin/ssh: sshd.c 
> > 
> > Log message:
> > expose the number of currently-authenticating connections
> > along with the MaxStartups limit in the proctitle;
> > suggestion from Philipp Marek, w/ feedback from Craig Miskell
> > ok dtucker@
> > 
> 
> It's nice to have this information visible, but it brings some problems.
> You can't now distinguish between multiple sshd processes (e.g. if you
> run several on different ports it's hard to figure out which one to
> signal if needed).

How could you discern between different sshd processes before? Just the
command-line args?

What information would you like there? We could put the first N listen
addrs in the proctitle if that would help.

-d



Re: GRE datagram socket support

2020-01-21 Thread Damien Miller
On Wed, 22 Jan 2020, David Gwynne wrote:

> Has anyone got an opinion on this? I am still interested in doing more
> packet capture things on OpenBSD using GRE as a transport, and the idea
> of maintaining this out of tree just makes me feel tired.

This is cool. I don't spot any major problems with this, but I'm rusty on
kernel networking.

> > Index: sys/kern/kern_pledge.c
> > ===
> > RCS file: /cvs/src/sys/kern/kern_pledge.c,v
> > retrieving revision 1.255
> > diff -u -p -r1.255 kern_pledge.c
> > --- sys/kern/kern_pledge.c  25 Aug 2019 18:46:40 -  1.255
> > +++ sys/kern/kern_pledge.c  29 Oct 2019 07:57:58 -
> > @@ -666,7 +666,7 @@ pledge_namei(struct proc *p, struct name
> > }
> > }
> >  
> > -   /* DNS needs /etc/{resolv.conf,hosts,services}. */
> > +   /* DNS needs /etc/{resolv.conf,hosts,services,protocols}. */
> > if ((ni->ni_pledge == PLEDGE_RPATH) &&
> > (p->p_p->ps_pledge & PLEDGE_DNS)) {
> > if (strcmp(path, "/etc/resolv.conf") == 0) {
> > @@ -678,6 +678,10 @@ pledge_namei(struct proc *p, struct name
> > return (0);
> > }
> > if (strcmp(path, "/etc/services") == 0) {
> > +   ni->ni_cnd.cn_flags |= BYPASSUNVEIL;
> > +   return (0);
> > +   }
> > +   if (strcmp(path, "/etc/protocols") == 0) {
> > ni->ni_cnd.cn_flags |= BYPASSUNVEIL;
> > return (0);

This looks like it is fixing a real, separate bug in pledge vs
getaddrinfo, no? (specifically: that lookups for named ports will fail
currently).



OpenSSH U2F/FIDO support in base

2019-11-14 Thread Damien Miller
Hi,

I just committed all the dependencies for OpenSSH security key (U2F)
support to base and tweaked OpenSSH to use them directly. This means
there will be no additional configuration hoops to jump through to use
U2F/FIDO2 security keys.

Hardware backed keys can be generated using "ssh-keygen -t ecdsa-sk"
(or "ed25519-sk" if your token supports it). Many tokens require to be
touched/tapped to confirm this step.

You'll get a public/private keypair back as usual, except in this case,
the private key file does not contain a highly-sensitive private key but
instead holds a "key handle" that is used by the security key to derive
the real private key at signing time.

So, stealing a copy of the private key file without also stealing your
security key (or access to it) should not give the attacker anything.

Once you have generated a key, you can use it normally - i.e. add it to
an agent, copy it to your destination's authorized_keys files (assuming
they are running -current too), etc. At authentication time, you will be
prompted to tap your security key to confirm the signature operation -
this makes theft-of-access attacks against security keys more difficult
too.

Please test this thoroughly - it's a big change that we want to have
stable before the next release.

-d



Re: tcpdump(8) mention USB interfaces in -i

2019-11-07 Thread Damien Miller
goddamn it, I could have used this last week :/

(ok djm)

On Wed, 6 Nov 2019, Stuart Henderson wrote:

> Found this diff when updating an old tree, ok?
> 
> 
> Index: usr.sbin/tcpdump/tcpdump.8
> ===
> RCS file: /cvs/src/usr.sbin/tcpdump/tcpdump.8,v
> retrieving revision 1.108
> diff -u -p -r1.108 tcpdump.8
> --- usr.sbin/tcpdump/tcpdump.831 Oct 2019 18:10:22 -  1.108
> +++ usr.sbin/tcpdump/tcpdump.86 Nov 2019 12:12:54 -
> @@ -146,6 +146,9 @@ searches the system interface list for t
>  interface
>  .Pq excluding loopback .
>  Ties are broken by choosing the earliest match.
> +.Ar interface
> +may be either a network interface or a USB interface, for example
> +.Ar usb0 .
>  .It Fl L
>  List the supported data link types for the interface and exit.
>  .It Fl l
> 
> 



Re: HID devices without numbered reports

2019-10-29 Thread Damien Miller
On Tue, 29 Oct 2019, Patrick Wildt wrote:

> Ok, so it turns out that this is related with opening/closing
> the uhid(4) device.  Because every open(2) and close(2) also
> opens and closes the pipe.  I think for ehci we somehow can
> save the toggle and restore it on re-open, and for xhci we
> somehow don't?

uhci isn't saving/restoring it either. Someone else reported no problems
using ohci.

> So I'm wondering if a) we should just keep the output pipe
> open or b) there's some way to save/restore the... endpoint
> context?  I'm looking into b) obviously.

Yeah, I spent some time reading xhci.c but couldn't figure out how
interrupt transfers actually get sent - I could see them get enqueud
but not where the got dequeued...

-d



Re: HID devices without numbered reports

2019-10-27 Thread Damien Miller
On Mon, 28 Oct 2019, Damien Miller wrote:

> BTW, the token still becomes unresponsive after the first transaction,
> but looking at a sniff (using an OpenViszla), it seems we're getting the
> DATA0/DATA1 flipping incorrect on the wire.
> 
> On OpenBSD, this is the last rx of the transaction with the card:
> 
> []  12.992349 d=  0.001951 [154.0 +  3.500] [  3] IN   : 8.4
> []  12.992352 d=  0.03 [154.0 +  6.667] [ 67] DATA0: 00 10 00 01 
> 0e 1b 4a 78 ec 87 06 bd 47 d4 a0 49 d9 c7 2d 89 d9 7e 2c c5 62 87 53 92 9b 90 
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 
> 00 00 00 00 00 00 00 00 3a e8
> 
> and this is the first tx of the first packet of the subsequent transaction
> that hangs:
> 
> []  22.201067 d=  0.001998 [146.0 +  4.333] [  3] OUT  : 8.4
> []  22.201070 d=  0.03 [146.0 +  7.583] [ 67] DATA0: ff ff ff ff 
> 86 00 08 c0 65 eb 53 9a 48 04 7d 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 
> 00 00 00 00 00 00 00 00 d6 1d

Since I don't really understand how USB sequence numbers work, it just
occurred to me to check the sequence bit on the last sent packet.
It too is a DATA0, so the sequence is definitiely getting lost across
close+open.

Here's an annotated trace - starting with the last command sent to
a Yk5 token during key enrollment:

[]   9.212345 d=  0.001992 [  2   +  4.333] [  3] OUT  : 10.4 
[]   9.212349 d=  0.03 [  2   +  7.583] [ 67] DATA1: 00 13 00 01 83 
00 47 00 01 00 00 00 00 40 a9 dc 95 51 0e ec 44 6d 7a b1 14 88 d1 84 93 b4 aa 
d2 e5 10 11 db 9c fa b4 b3 0b 89 2c ea 3f b9 e3 06 10 e8 a1 62 11 59 60 fe 1e 
c2 23 e6 52 9c 9f 4b 8a c8 
[]   9.212395 d=  0.46 [  2   + 53.750] [  1] ACK 
[]   9.212397 d=  0.02 [  2   + 55.667] [  3] IN   : 10.4 
[]   9.212400 d=  0.03 [  2   + 58.833] [  1] NAK 
[]   9.214346 d=  0.001946 [  4   +  4.500] [  3] OUT  : 10.4 
[]   9.214349 d=  0.03 [  4   +  7.750] [ 67] DATA0: 00 13 00 01 00 
6e 80 20 0d cb 5e 5c 32 1c 8a f1 e2 b1 bf 00 00 00 00 00 00 00 00 00 00 00 00 
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 
00 00 00 00 00 00 00 11 7c 
[]   9.214395 d=  0.46 [  4   + 53.750] [  1] ACK 
[]   9.214397 d=  0.02 [  4   + 55.667] [  3] IN   : 10.4 
[]   9.214400 d=  0.03 [  4   + 58.833] [  1] NAK 
[]   9.216345 d=  0.001945 [  6   +  4.000] [  3] IN   : 10.4 
[]   9.216349 d=  0.03 [  6   +  7.167] [  1] NAK 

Note that the last OUT was a DATA0.

The token replies with a bunch of data:

[]   9.350355 d=  0.001997 [140   +  3.250] [  3] IN   : 10.4 
[]   9.350358 d=  0.03 [140   +  6.417] [ 67] DATA1: 00 13 00 01 83 
03 8d 05 04 f6 80 b1 df 4d 8e fe 30 cd a0 2c 42 e1 a1 46 52 9f 8a 06 80 21 78 
7a 73 71 e3 f3 0a e6 f6 e0 74 54 ef df 0c 74 ed be 3f 35 1d dd 35 cf 33 14 fc 
33 6e b6 45 11 bd f5 79 99 
[]   9.350404 d=  0.46 [140   + 52.417] [  1] ACK 
[]   9.352356 d=  0.001951 [142   +  3.750] [  3] IN   : 10.4 
[]   9.352359 d=  0.03 [142   +  6.917] [ 67] DATA0: 00 13 00 01 00 
61 98 66 2c 14 5f 65 e5 4c 40 df 01 56 e8 d8 2b e8 f7 a8 ff 00 96 a6 f3 95 00 
d9 93 87 cb c8 b1 02 23 ec 8f 38 37 b8 cf da 73 62 18 90 ff 8b 01 cf a1 78 61 
3e cc 48 ac 7b 45 4f b0 6b 
[]   9.352405 d=  0.46 [142   + 53.167] [  1] ACK 
[]   9.354356 d=  0.001950 [144   +  3.500] [  3] IN   : 10.4 
[]   9.354359 d=  0.03 [144   +  6.667] [ 67] DATA1: 00 13 00 01 01 
4a 0d 3a 6d d7 ca fc 00 e1 ad 7e 78 b1 9d 88 30 82 02 bd 30 82 01 a5 a0 03 02 
01 02 02 04 18 ac 46 c0 30 0d 06 09 2a 86 48 86 f7 0d 01 01 0b 05 00 30 2e 31 
2c 30 2a 06 03 55 04 83 af 
[]   9.354405 d=  0.46 [144   + 52.833] [  1] ACK 
[]   9.356356 d=  0.001951 [146   +  3.583] [  3] IN   : 10.4 
[]   9.356359 d=  0.03 [146   +  6.750] [ 67] DATA0: 00 13 00 01 02 
03 13 23 59 75 62 69 63 6f 20 55 32 46 20 52 6f 6f 74 20 43 41 20 53 65 72 69 
61 6c 20 34 35 37 32 30 30 36 33 31 30 20 17 0d 31 34 30 38 30 31 30 30 30 30 
30 30 5a 18 0f 32 30 98 68 
[]   9.356405 d=  0.46 [146   + 52.667] [  1] ACK 
[]   9.358356 d=  0.001951 [148   +  3.583] [  3] IN   : 10.4 
[]   9.358359 d=  0.03 [148   +  6.750] [ 67] DATA1: 00 13 00 01 03 
35 30 30 39 30 34 30 30 30 30 30 30 5a 30 6e 31 0b 30 09 06 03 55 04 06 13 02 
53 45 31 12 30 10 06 03 55 04 0a 0c 09 59 75 62 69 63 6f 20 41 42 31 22 30 20 
06 03 55 04 0b 0c 19 69 76 
[]   9.358405 d=  0.46 [148   + 52.667] [  1] ACK 
[]   9.360357 d=  0.001952 [150   +  4.833] [  3] IN   : 10.4 
[]   9.360361 d=  0.03 [150   +  8.000] [ 67] DATA0: 00 13 00 01 04 
41 75 74 68 65 6e 74 69 63 61 74 6f 72 20 41 74 74 65 73 74 61 74 69 6f 6e 

Re: HID devices without numbered reports

2019-10-27 Thread Damien Miller
On Fri, 25 Oct 2019, Patrick Wildt wrote:

> > So from what I understood the Yubikey expects the transfer to happen
> > on the Interrupt OUT pipe instead of doing a control transfer.  Read-
> > ing some code and documentation, it looks like that we should by de-
> > fault send our reports on the interrupt pipe, and only if it does not
> > exist fall back to control transfers.  Linux seems to do exactly that.
> > 
> > I tried to come up with the following diff, which appeard to make a
> > test program work for me.  Though I'm not sure I got all the specifics
> > right.
> > 
> > Can you give this a go with your test progam?
> > 
> > Patrick
> 
> Oops, obvious error.  Though I still cannot write/read multiple times
> in a row, so there is probably still something wrong (unless my test
> is as well).

It didn't work for me - I think because uhidev_set_report() (and _async)
is still prepending a report ID. If I modify uhid.c to send writes
without a report ID, then it does work:

Index: uhid.c
===
RCS file: /cvs/src/sys/dev/usb/uhid.c,v
retrieving revision 1.71
diff -u -p -r1.71 uhid.c
--- uhid.c  1 May 2018 18:14:46 -   1.71
+++ uhid.c  27 Oct 2019 22:03:16 -
@@ -322,7 +322,7 @@ uhid_do_write(struct uhid_softc *sc, str
error = uiomove(sc->sc_obuf, size, uio);
if (!error) {
if (uhidev_set_report(sc->sc_hdev.sc_parent,
-   UHID_OUTPUT_REPORT, sc->sc_hdev.sc_report_id, sc->sc_obuf,
+   0, sc->sc_hdev.sc_report_id, sc->sc_obuf,
size) != size)
error = EIO;
}

I don't think this is optimal though - perhaps we should do something
like Linux: if the endpoint has an output report then use it, and fall
back to not prepending a report ID otherwise. This should be compatible
with current devices too (well, at least it shouldn't break any that
aren't already broken.)

(again, caveat - I don't know much about USB programming).

-d



HID devices without numbered reports

2019-10-24 Thread Damien Miller
Hi,

Some HID devices do not list any report IDs for communication, e.g. my
Yubikey5:

> uhidev0: iclass 3/0
> uhid0 at uhidev0: input=64, output=64, feature=0
> ugen0 at uhub0 port 2 configuration 1 "Yubico YubiKey FIDO+CCID" rev 
> 2.00/5.12 addr 2

on Linux, these can be talked to by sending to report ID 0. This AFAIK
causes the write(2)'d data to be sent to the interrupt pipe.

Our uhid(4) devices already consider write(2) operations to be
equivalent to USB_SET_REPORT to the UHID_OUTPUT_REPORT, so we can't
use that. So this patch allows USB_SET_REPORT with ucr_report=0
to do something similar to what Linux does.

Caveat: this is literally my first attempt at writing USB code, and
I ended up futzing with the kernel.

Index: sys/dev/usb/uhidev.c
===
RCS file: /cvs/src/sys/dev/usb/uhidev.c,v
retrieving revision 1.76
diff -u -p -r1.76 uhidev.c
--- sys/dev/usb/uhidev.c25 Aug 2018 18:32:05 -  1.76
+++ sys/dev/usb/uhidev.c25 Oct 2019 03:17:19 -
@@ -903,6 +903,15 @@ uhidev_ioctl(struct uhidev *sc, u_long c
case USB_SET_REPORT:
re = (struct usb_ctl_report *)addr;
switch (re->ucr_report) {
+   case 0:
+   /*
+* unnumbered report; send to interrupt pipe using
+* size from output report descriptor.
+*/
+   if (uhidev_write(sc->sc_parent,
+   re->ucr_data, sc->sc_osize) != 0)
+   return EIO;
+   break;
case UHID_INPUT_REPORT:
size = sc->sc_isize;
break;

Is this a sensible approach? Is there something obvious that I missed?

-d



Re: Potential null pointer dereference in sshkey shielding

2019-06-26 Thread Damien Miller
On Wed, 26 Jun 2019, Reynir Björnsson wrote:

> Hello,
> 
> I have noticed a potential NULL pointer dereference in the recent code
> for ssh key shielding. Essentially, during error handling
> explicit_bzero(enc, enclen) is called. This should be fine when enc is
> NULL as long as enclen is zero. However, there is one spot in the code
> where a malloc() call for enc could fail where enclen could be non-zero.

Thanks for looking at the code.

I think my mistake was not reaching for freezero() - this also fixes the
leak of enc on other error paths. It is guaranteed to be a no-op if its
first argument is NULL.

Index: sshkey.c
===
RCS file: /cvs/src/usr.bin/ssh/sshkey.c,v
retrieving revision 1.77
diff -u -p -r1.77 sshkey.c
--- sshkey.c23 Jun 2019 12:21:46 -  1.77
+++ sshkey.c27 Jun 2019 03:29:39 -
@@ -1943,9 +1943,9 @@ sshkey_shield_private(struct sshkey *k)
  out:
/* XXX behaviour on error - invalidate original private key? */
cipher_free(cctx);
-   explicit_bzero(enc, enclen);
explicit_bzero(keyiv, sizeof(keyiv));
explicit_bzero(, sizeof(tmp));
+   freezero(enc, enclen);
freezero(prekey, SSHKEY_SHIELD_PREKEY_LEN);
sshkey_free(kswap);
sshbuf_free(prvbuf);


Re: ssh-askpass(1): fix indicator size with multiple screens

2019-06-16 Thread Damien Miller



On Sun, 16 Jun 2019, Matthieu Herrb wrote:

> On Sun, Jun 09, 2019 at 04:47:53PM +0200, Matthieu Herrb wrote:
> > Hi,
> > 
> > ssh-askpass(1) is trying to be clever and computes the size of its
> > indicator relatively to the screen resolution.
> > 
> > Unfortunatly, when multiple screens are present, this gets ugly. The
> > support for Xinerama correctly computes the dimensions of the window
> > to be created, relatively to the screen on which it will appear, but
> > the computation of the indicator size is based on the size of the
> > whole display, resulting in too small indicators (and too many of them
> > if the screens hare layed out horizontally).
> > 
> > The patch below fixes that by computing the resolution of the whole
> > display before taking xinerama into account.
> > 
> > A better fix would be to make this application really aware of XRandR
> > and use the actual screen resolution from XRandR; this is an
> > interesting project, if anyone wants to give it a try.
> > 
> > ok?
> 
> ping

ok djm. Is there an upstream for ssh-askpass? The old one I have
http://www.jmknoble.net/software/x11-ssh-askpass/ doesn't resolve any more.

-d



Re: register DoT in /etc/services?

2019-01-27 Thread Damien Miller
On Sun, 27 Jan 2019, Theo de Raadt wrote:

> I need to add I worry for the future, the 512-1023 reserved space is
> being gobbled at a rapid pace by new services, which not only decreases
> the port# entropy but reduces the total number of reserved ports which
> can be allocated.  Fewer software services allocate reserved ports
> today, but it isn't a dead concept either, and people are likely to run
> more instances of software since machines got bigger.  I wonder if any
> old service entries can be can be purged.

telnet :P



Re: qsort comparision function bug

2019-01-21 Thread Damien Miller
On Mon, 21 Jan 2019, Dariusz Sendkowski wrote:

> Wouldn't it lead to undefined behavior?
> According to the standard: "... The value of the result of an integer
> arithmetic or conversion function cannot be represented (7.8.2.1, 7.8.2.2,
> 7.8.2.3, 7.8.2.4, 7.22.6.1, 7.22.6.2, 7.22.1) ..."
> This dump case is not compiled with -fwrapv flag, so I guess it might lead
> to undefined behavior.

OpenBSD sets -fwrapv by default. From clang-local(1):

 -   The -fwrapv option to treat signed integer overflows as defined is
 enabled by default to prevent dangerous optimizations which could
 remove security critical overflow checks.




Re: www/64.html - OpenSSH version 7.8 or 7.9?

2018-10-19 Thread Damien Miller
On Thu, 18 Oct 2018, jungle boogie wrote:

> I see the release notes are alive:
> https://www.openssh.com/txt/release-7.9
> 
> Might want to change the link on https://www.openssh.com leading to the
> release, still showing 7.8.

not everything updates at once, some things need to be committed before
others.



Re: close filedescriptors of children

2018-03-07 Thread Damien Miller
On Wed, 7 Mar 2018, Gerhard Roth wrote:

> Below is an updated patch that includes proc.c of switchd and vmd.
> It also passes the 'debug' flag to proc_init() so that it won't touch
> std* in that case.

FWIW sshd unconditionally clobbers stdin and stdout and will also
clobber stderr if the debug flag isn't set.

-d



Re: ssh: don't close fds multiple times and don't close(-1)

2018-02-04 Thread Damien Miller
ok djm

On Mon, 5 Feb 2018, Theo Buehler wrote:

> In channel_close_fd(), the file descriptors for the socket, stdin,
> stdout and stderr aren't necessarily distinct, so closing them results
> in EBADF. In addition, the diff adds a couple of positivity checks to
> avoid calling close(-1).
> 
> Index: usr.bin/ssh/channels.c
> ===
> RCS file: /var/cvs/src/usr.bin/ssh/channels.c,v
> retrieving revision 1.378
> diff -u -p -r1.378 channels.c
> --- usr.bin/ssh/channels.c23 Jan 2018 05:27:21 -  1.378
> +++ usr.bin/ssh/channels.c24 Jan 2018 00:41:18 -
> @@ -426,10 +426,15 @@ channel_close_fd(struct ssh *ssh, int *f
>  static void
>  channel_close_fds(struct ssh *ssh, Channel *c)
>  {
> + int sock = c->sock, rfd = c->rfd, wfd = c->wfd, efd = c->efd;
> +
>   channel_close_fd(ssh, >sock);
> - channel_close_fd(ssh, >rfd);
> - channel_close_fd(ssh, >wfd);
> - channel_close_fd(ssh, >efd);
> + if (rfd != sock)
> + channel_close_fd(ssh, >rfd);
> + if (wfd != sock && wfd != rfd)
> + channel_close_fd(ssh, >wfd);
> + if (efd != sock && efd != rfd && efd != wfd)
> + channel_close_fd(ssh, >efd);
>  }
>  
>  static void
> Index: usr.bin/ssh/monitor.c
> ===
> RCS file: /var/cvs/src/usr.bin/ssh/monitor.c,v
> retrieving revision 1.178
> diff -u -p -r1.178 monitor.c
> --- usr.bin/ssh/monitor.c 23 Jan 2018 05:27:21 -  1.178
> +++ usr.bin/ssh/monitor.c 24 Jan 2018 00:41:18 -
> @@ -230,8 +230,10 @@ monitor_child_preauth(Authctxt *_authctx
>  
>   debug3("preauth child monitor started");
>  
> - close(pmonitor->m_recvfd);
> - close(pmonitor->m_log_sendfd);
> + if (pmonitor->m_recvfd >= 0)
> + close(pmonitor->m_recvfd);
> + if (pmonitor->m_log_sendfd >= 0)
> + close(pmonitor->m_log_sendfd);
>   pmonitor->m_log_sendfd = pmonitor->m_recvfd = -1;
>  
>   authctxt = _authctxt;
> @@ -298,8 +300,10 @@ monitor_child_preauth(Authctxt *_authctx
>   while (pmonitor->m_log_recvfd != -1 && monitor_read_log(pmonitor) == 0)
>   ;
>  
> - close(pmonitor->m_sendfd);
> - close(pmonitor->m_log_recvfd);
> + if (pmonitor->m_recvfd >= 0)
> + close(pmonitor->m_recvfd);
> + if (pmonitor->m_log_sendfd >= 0)
> + close(pmonitor->m_log_sendfd);
>   pmonitor->m_sendfd = pmonitor->m_log_recvfd = -1;
>  }
>  
> Index: usr.bin/ssh/ssh-pkcs11-client.c
> ===
> RCS file: /var/cvs/src/usr.bin/ssh/ssh-pkcs11-client.c,v
> retrieving revision 1.7
> diff -u -p -r1.7 ssh-pkcs11-client.c
> --- usr.bin/ssh/ssh-pkcs11-client.c   30 May 2017 08:52:19 -  1.7
> +++ usr.bin/ssh/ssh-pkcs11-client.c   23 Jan 2018 00:09:22 -
> @@ -93,7 +93,8 @@ pkcs11_init(int interactive)
>  void
>  pkcs11_terminate(void)
>  {
> - close(fd);
> + if (fd >= 0)
> + close(fd);
>  }
>  
>  static int
> 
> 



Re: base system multi-booting in MBR

2018-02-01 Thread Damien Miller
On Wed, 31 Jan 2018, Alexei Malinin wrote:

> Hello.
> 
> If the base system supported multi-booting in MBR would the community be
> interested in it?

Doesn't it already? "machine boot sd0X"



Re: use inline functions instead of __statement

2018-01-03 Thread Damien Miller
On Thu, 4 Jan 2018, David Gwynne wrote:

> my theory is that __statement (a gcc extension) was used to allow
> macros to evaluate their argument(s) once by assigning it to a local
> variable, and then returning a value. this is difficult with normal
> macros.

Not understanding - doesn't this:

> -#define  __swap32md(x) __statement({ 
> \
> - __uint32_t __swap32md_x = (x);  \

evaluate its argument only once even without __statement?

-d



Re: sshd(8) logging of client disconnect from ClientAliveInterval

2017-10-17 Thread Damien Miller
ok by me

On Wed, 18 Oct 2017, Darren Tucker wrote:

> On Tue, Oct 17, 2017 at 09:10:38PM +0300, Lars Noodén wrote:
> > Here is a replacement patch.
> 
> I meant reusing the existing function rather than cloning it.  It's
> currently static so it needs to be exported but IMO that's better than
> duplicating the code.
> 
> Index: packet.c
> ===
> RCS file: /cvs/src/usr.bin/ssh/packet.c,v
> retrieving revision 1.265
> diff -u -p -r1.265 packet.c
> --- packet.c  13 Oct 2017 21:13:54 -  1.265
> +++ packet.c  17 Oct 2017 22:49:08 -
> @@ -1773,8 +1773,8 @@ ssh_packet_send_debug(struct ssh *ssh, c
>   fatal("%s: %s", __func__, ssh_err(r));
>  }
>  
> -static void
> -fmt_connection_id(struct ssh *ssh, char *s, size_t l)
> +void
> +sshpkt_fmt_connection_id(struct ssh *ssh, char *s, size_t l)
>  {
>   snprintf(s, l, "%.200s%s%s port %d",
>   ssh->log_preamble ? ssh->log_preamble : "",
> @@ -1790,7 +1790,7 @@ sshpkt_fatal(struct ssh *ssh, const char
>  {
>   char remote_id[512];
>  
> - fmt_connection_id(ssh, remote_id, sizeof(remote_id));
> + sshpkt_fmt_connection_id(ssh, remote_id, sizeof(remote_id));
>  
>   switch (r) {
>   case SSH_ERR_CONN_CLOSED:
> @@ -1852,7 +1852,7 @@ ssh_packet_disconnect(struct ssh *ssh, c
>* Format the message.  Note that the caller must make sure the
>* message is of limited size.
>*/
> - fmt_connection_id(ssh, remote_id, sizeof(remote_id));
> + sshpkt_fmt_connection_id(ssh, remote_id, sizeof(remote_id));
>   va_start(args, fmt);
>   vsnprintf(buf, sizeof(buf), fmt, args);
>   va_end(args);
> Index: packet.h
> ===
> RCS file: /cvs/src/usr.bin/ssh/packet.h,v
> retrieving revision 1.82
> diff -u -p -r1.82 packet.h
> --- packet.h  12 Sep 2017 06:32:07 -  1.82
> +++ packet.h  17 Oct 2017 22:49:08 -
> @@ -186,6 +186,7 @@ int   sshpkt_get_cstring(struct ssh *ssh, 
>  int  sshpkt_get_ec(struct ssh *ssh, EC_POINT *v, const EC_GROUP *g);
>  int  sshpkt_get_bignum2(struct ssh *ssh, BIGNUM *v);
>  int  sshpkt_get_end(struct ssh *ssh);
> +void sshpkt_fmt_connection_id(struct ssh *ssh, char *s, size_t l);
>  const u_char *sshpkt_ptr(struct ssh *, size_t *lenp);
>  
>  /* OLD API */
> Index: serverloop.c
> ===
> RCS file: /cvs/src/usr.bin/ssh/serverloop.c,v
> retrieving revision 1.198
> diff -u -p -r1.198 serverloop.c
> --- serverloop.c  12 Sep 2017 06:35:32 -  1.198
> +++ serverloop.c  17 Oct 2017 22:49:08 -
> @@ -162,10 +162,12 @@ static void
>  client_alive_check(struct ssh *ssh)
>  {
>   int channel_id;
> + char remote_id[512];
>  
>   /* timeout, check to see how many we have had */
>   if (packet_inc_alive_timeouts() > options.client_alive_count_max) {
> - logit("Timeout, client not responding.");
> + sshpkt_fmt_connection_id(ssh, remote_id, sizeof(remote_id));
> + logit("Timeout, client not responding from %s", remote_id);
>   cleanup_exit(255);
>   }
>  
> 
> -- 
> Darren Tucker (dtucker at zip.com.au)
> 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.
> 
> 


freezero(NULL, 0)

2017-08-23 Thread Damien Miller
Hi,

memset(NULL, 0, 0) is (strictly speaking) undefined behaviour, but
there's no reason that freezero(3) needs to follow suit.

This mentions that freezero(NULL, 0) is valid in the manpage, so that
anyone who copies this API should get it right too.

ok?

Index: malloc.3
===
RCS file: /cvs/src/lib/libc/stdlib/malloc.3,v
retrieving revision 1.115
diff -u -p -r1.115 malloc.3
--- malloc.315 May 2017 18:05:34 -  1.115
+++ malloc.324 Aug 2017 01:31:52 -
@@ -210,6 +210,12 @@ argument must be equal or smaller than t
 that returned
 .Fa ptr .
 .Fn freezero
+may be called with a
+.Dv NULL
+pointer argument if the
+.Fa size
+argument is zero.
+.Fn freezero
 guarantees the memory range starting at
 .Fa ptr
 with length



Re: systemd compat for doas

2017-07-03 Thread Damien Miller
On Mon, 3 Jul 2017, Franco Fichtner wrote:

> 
> > On 2. Jul 2017, at 8:59 PM, Ted Unangst  wrote:
> > 
> > If the username starts with a digit, but isn't a number, treat it like root.
> 
> I question the simplicity of this patch due to the fact that it leaves
> no head room for further security-related regressions.  Maybe more
> progressive over-engineering of the code is a better course of action.

yeah, where's the dbus integration?



Re: [PATCH 02/04] Adjust AES testcase to the new implementation

2017-04-24 Thread Damien Miller
ok

On Mon, 24 Apr 2017, Mike Belopuhov wrote:

> Adjusts the regress test.
> 
> ---
>  regress/sys/crypto/aes/Makefile  |  2 +-
>  regress/sys/crypto/aes/aestest.c | 10 +-
>  2 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git regress/sys/crypto/aes/Makefile regress/sys/crypto/aes/Makefile
> index 75e88b1a7a6..9120371a07d 100644
> --- regress/sys/crypto/aes/Makefile
> +++ regress/sys/crypto/aes/Makefile
> @@ -17,11 +17,11 @@ CDIAGFLAGS+=  -Wsign-compare
>  CDIAGFLAGS+= -Wshadow
>  
>  REGRESS_ROOT_TARGETS=run-regress-${PROG}
>  
>  .PATH:   ${DIR}/crypto
> -SRCS+=   rijndael.c
> +SRCS+=   aes.c
>  
>  run-regress-${PROG}: ${PROG}
>   ./${PROG} ${.CURDIR}/vectors/*.txt
>  
>  .include 
> diff --git regress/sys/crypto/aes/aestest.c regress/sys/crypto/aes/aestest.c
> index f51be2a2665..489ac404e45 100644
> --- regress/sys/crypto/aes/aestest.c
> +++ regress/sys/crypto/aes/aestest.c
> @@ -29,11 +29,11 @@
>   * Test kernel AES implementation with test vectors provided by
>   * Dr Brian Gladman:  http://fp.gladman.plus.com/AES/
>   */
>  
>  #include 
> -#include 
> +#include 
>  #include 
>  #include 
>  #include 
>  #include 
>  #include 
> @@ -41,21 +41,21 @@
>  
>  static int
>  docrypt(const unsigned char *key, size_t klen, const unsigned char *in,
>  unsigned char *out, size_t len, int do_encrypt)
>  {
> - rijndael_ctx ctx;
> + AES_CTX ctx;
>   int error = 0;
>  
>   memset(, 0, sizeof(ctx));
> - error = rijndael_set_key(, key, klen * 8);
> + error = AES_Setkey(, key, klen);
>   if (error)
>   return -1;
>   if (do_encrypt)
> - rijndael_encrypt(, in, out);
> + AES_Encrypt(, in, out);
>   else
> - rijndael_decrypt(, in, out);
> + AES_Decrypt(, in, out);
>   return 0;
>  }
>  
>  static int
>  match(unsigned char *a, unsigned char *b, size_t len)
> -- 
> 2.12.2
> 
> 



Re: tcpdump: decode BGP Administrative Shutdown Communication

2017-04-19 Thread Damien Miller
On Wed, 19 Apr 2017, Job Snijders wrote:

> The realisation that a shutdown communication may contain \0 (since NUL is a
> valid UTF-8 char)

\0 isn't a valid UTF-8 character. UTF-8 sets the MSB on code points > 127:
https://en.wikipedia.org/wiki/UTF-8#Description




Re: rebound quantum entanglement

2016-09-15 Thread Damien Miller
On Thu, 15 Sep 2016, Chris Cappuccio wrote:

> That rebound acts like a nameserver is what prompted the idea to
> hijack the resolver. But it's really a tool that takes over certain
> duties from the libc resolver, so the libc resolver should be properly
> configurable to hand over duties, or not. 'search' is the logical
> place for this.

+1 to this

Hidden magic configuration always causes grief.



Re: Default softraid crypto PBKDF2 rounds

2016-09-07 Thread Damien Miller
On Wed, 7 Sep 2016, Andreas Bartelt wrote:

> yes, due to the larger internal state of the blowfish algorithm which is
> harder to efficiently realize in dedicated hardware. However, since bcrypt's
> internal state effectively is of fixed size, scrypt would be an even better
> option since it allows for a parameterization of this internal state. Is there
> any interest in switching to scrypt in the context of password authentication
> on OpenBSD?

no, its advantages aren't sufficient for the disruption IMO.

We might consider whatever wins the shootout going on between balloon
hashing and Argon2, but bcrypt has survived so incredibly well that
we can afford to wait.



Re: Default softraid crypto PBKDF2 rounds

2016-09-07 Thread Damien Miller
On Tue, 6 Sep 2016, David Coppa wrote:

> Il 6 settembre 2016 14:56:32 CEST, Filippo Valsorda  ha 
> scritto:
> >Hello,
> >
> >I recently had the occasion to dive into the softraid crypto code [1]
> >and was quite pleased with the cleanliness of it all. However, I found
> >surprising the default value of 8k PBKDF2 rounds.
> >
> >I know it is easy to override and I should have RTFM, but I (naively,
> >I'll admit) assumed OpenBSD would pick very robust defaults, erring on
> >the conservative side. Is it maybe time to bump it up, or pick it based
> >on a quick machine benchmark?
> >
> >If there's consensus I might also provide a patch for the live
> >benchmark
> >option.
> 
> yes, autodetection of a sensible value would be cool...

using bcrypt_kdf would be better :)



Announce: OpenSSH 7.3 released

2016-08-01 Thread Damien Miller
orrect description of UseDNS: it affects ssh
   hostname processing for authorized_keys, not known_hosts; bz#2554

 * ssh(1): Fix authentication using lone certificate keys in an agent
   without corresponding private keys on the filesystem. bz#2550

 * sshd(8): Send ClientAliveInterval pings when a time-based
   RekeyLimit is set; previously keepalive packets were not being
   sent. bz#2252

Portability
---

 * ssh(1), sshd(8): Fix compilation by automatically disabling ciphers
   not supported by OpenSSL. bz#2466

 * misc: Fix compilation failures on some versions of AIX's compiler
   related to the definition of the VA_COPY macro. bz#2589

 * sshd(8): Whitelist more architectures to enable the seccomp-bpf
   sandbox. bz#2590

 * ssh-agent(1), sftp-server(8): Disable process tracing on Solaris
   using setpflags(__PROC_PROTECT, ...). bz#2584

 * sshd(8): On Solaris, don't call Solaris setproject() with
   UsePAM=yes it's PAM's responsibility. bz#2425

Checksums:
==

 - SHA1 (openssh-7.3.tar.gz) = b1641e5265d9ec68a9a19decc3a7edd1203cbd33
 - SHA256 (openssh-7.3.tar.gz) = vS0X35qrX9OOPBkyDMYhOje/DBwHBVEV7nv5rkzw4vM=

 - SHA1 (openssh-7.3p1.tar.gz) = bfade84283fcba885e2084343ab19a08c7d123a5
 - SHA256 (openssh-7.3p1.tar.gz) = P/uYmm3KppWUw7VQ1IVaWi4XGMzd5/XjY4e0JCIPvsw=

Please note that the SHA256 signatures are base64 encoded and not
hexadecimal (which is the default for most checksum tools). The PGP
key used to sign the releases is available as RELEASE_KEY.asc from
the mirror sites.

Reporting Bugs:
===

- Please read http://www.openssh.com/report.html
  Security bugs should be reported directly to open...@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.



Re: [armv7] introducing tipru(4)

2016-07-06 Thread Damien Miller
On Wed, 6 Jul 2016, Ian Sutton wrote:

> *   tipru comes disabled by default. Attempts to enable tipru, and
> following modification of the instruction/data/shared memory
> spaces, are only allowed when the system's securelevel(7) is equal
> or lesser than zero. When the system's securelevel(7) is greater
> than zero the PRU cores may only be started or halted via the
> PRUCTL ioctl(2) call, and the driver itself may be disabled via the
> PRUKILL ioctl(2) call which effectively halts and prevents the PRU
> from performing any actions until the system is rebooted.

That sounds like a reasonable compromise - it would let the admin load
code to the PRUs in rc.securelevel for later use, or set
kern.securelevel=0 in sysctl.conf if they wanted to do development
on a multi-user system.



Re: [armv7] introducing tipru(4)

2016-07-05 Thread Damien Miller
On Tue, 5 Jul 2016, Jonathan Gray wrote:

> On Tue, Jul 05, 2016 at 01:39:18AM -0400, Ian Sutton wrote:
> > On Mon, Jul 4, 2016 at 10:30 PM, Jonathan Gray  wrote:
> > > Lack of fdt use aside, we don't want to enable something that
> > > allows userspace access to system memory like this.
> > 
> > I can understand this sentiment. Maybe next time..
> > 
> > Are you saying you are catagorically opposed to a PRU driver or just
> > opposed to this driver in its current state?
> 
> I don't have time to look into how tied to the rest of the
> system the pru is at the moment.
> 
> Perhaps it could only permit access at a particular securelevel
> like gpio or be disabled by default.

+1 for doing something like gpio, e.g. an ioctl or sysctl to enable it
that can only be set at kern.securelevel=0



Re: [ntpd] Simultaneously listen on IPv4 and IPv6

2016-05-17 Thread Damien Miller
On Tue, 17 May 2016, Henning Brauer wrote:

> > What about systems with net.inet6.ip6.v6only=0? 
> 
> Those haven't been taken into consideration by yours truly and might be
> the compelling argument to have this code :)

That sysctl isn't hooked up to anything, it should be removed.
(compare IPV6CTL_VARS against IPV6CTL_NAMES in in6.h)

-d



Re: spamd - blacklists

2016-03-15 Thread Damien Miller
On Tue, 15 Mar 2016, li...@wrant.com wrote:

> What's going on with the BGP as a transport then, is it available to
> the general public?  Must be much better than the fubar DNS.  Nackts
> thing and we'd be attempting carping on tunnelled over DNS syndrome.

Years ago I added the pftable keyword to bgpd.conf for this very
reason. Assuming it hasn't bitrotted, it's trivial to use bgpd
to fill a PF table that can be used to block or tarpit spammers.

-d



OpenSSH Security Advisory: xauth command injection

2016-03-10 Thread Damien Miller

OpenSSH Security Advisory: x11fwd.adv

This document may be found at: http://www.openssh.com/txt/x11fwd.adv

1. Affected configurations

All versions of OpenSSH prior to 7.2p2 with X11Forwarding
enabled.

2. Vulnerability

Missing sanitisation of untrusted input allows an
authenticated user who is able to request X11 forwarding
to inject commands to xauth(1).

Injection of xauth commands grants the ability to read
arbitrary files under the authenticated user's privilege,
Other xauth commands allow limited information leakage,
file overwrite, port probing and generally expose xauth(1),
which was not written with a hostile user in mind, as an
attack surface.

xauth(1) is run under the user's privilege, so this
vulnerability offers no additional access to unrestricted
accounts, but could circumvent key or account restrictions
such as sshd_config ForceCommand, authorized_keys
command="..." or restricted shells.

3. Mitigation

Set X11Forwarding=no in sshd_config. This is the default.

For authorized_keys that specify a "command" restriction,
also set the "restrict" (available in OpenSSH >=7.2) or
"no-x11-forwarding" restrictions.

4. Details

As part of establishing an X11 forwarding session, sshd(8)
accepts an X11 authentication credential from the client.
This credential is supplied to the xauth(1) utility to
establish it for X11 applications that the user subsequently
runs.

The contents of the credential's components (authentication
scheme and credential data) were not sanitised to exclude
meta-characters such as newlines. An attacker could
therefore supply a credential that injected commands to
xauth(1). The attacker could then use a number of xauth
commands to read or overwrite arbitrary files subject to
file permissions, connect to local ports or perform attacks
on xauth(1) itself.

OpenSSH 7.2p2 implements a whitelist of characters that
are permitted to appear in X11 authentication credentials.

5. Credit

This issue was identified by github.com/tintinweb and
communicated to the OpenSSH developers on March 3rd, 2016.

6. Fix

Portable OpenSSH 7.2p2 contains a fix for this vulnerability.

Patches for supported OpenBSD releases (5.7, 5.8 and 5.9) have
been committed to the -STABLE branches and are available on the
errata pages:

http://www.openbsd.org/errata57.html
http://www.openbsd.org/errata58.html
http://www.openbsd.org/errata59.html



Re: Xorg stipple

2016-03-09 Thread Damien Miller
On Wed, 9 Mar 2016, joshua stein wrote:

> Is anyone seriously finding video/Xorg bugs through the default X
> stipple pattern anymore?  Xorg changed the default to draw a black
> background a while ago (with stipple enabled using the -retro flag),
> but we have this local change that reverted it while adding a silly
> -retard flag in order to show the black background.
> 
> I think we can finally stop partying like it's 1989 (vax is dead,
> after all) and have X show a solid black background by default.

X11 isn't my area, but 1000 x YES PLEASE



Re: utf8 hack for ls

2015-10-26 Thread Damien Miller
rather than scattering hacks in each program that needs to
output utf8 to the console, how about making something
for libutil that they all can use?

On Sun, 25 Oct 2015, Ted Unangst wrote:

> it only gets deeper and thicker...
> 
> this decodes chars and prints ? for bytes it doesn't like, as well as
> codepoints (128-159) it doesn't like.
> 
> (this is extracted from some old utf8 code i had laying around. it's a bit
> simpler than the stringprep stuff but it seems to handle the case of some
> incorrect sequences correctly. it does allow overlong encodings, but "not my
> problem"?)



Re: ChachaPoly-03: Chacha20-Poly1305 AEAD construction as per RFC7634

2015-10-26 Thread Damien Miller
On Mon, 26 Oct 2015, Mike Belopuhov wrote:

> OK?

Will this get the nonce right on BE systems?

> + /* initial counter is 1 */
> + ctx->nonce[0] = 1;
> + memcpy(ctx->nonce + CHACHA20_CTR, key + CHACHA20_KEYSIZE,
> + CHACHA20_SALT);



Re: [PATCH] SSH tunnels without root permissions

2015-10-06 Thread Damien Miller
On Tue, 6 Oct 2015, Ossi Herrala wrote:

> ping?
> 
> On Fri, Sep 18, 2015 at 06:46:20PM +0300, Ossi Herrala wrote:
> > Hi everyone,
> > 
> > The following patch makes it possible to build SSH layer 2 (and layer
> > 3) tunnels without using root permissions when connecting.
> > 
> > This is achieved by root setting up everything beforehand so sshd
> > doesn't have to do it. However, the old functionality of sshd setting
> > things up with root permissions is preserved.

Thanks, I just committed this with some extra debug() statements added.

-d



Re: UTF-8 string filtering

2015-09-20 Thread Damien Miller
On Sat, 12 Sep 2015, Stefan Sperling wrote:

> 
> On Fri, Sep 04, 2015 at 03:17:31PM +1000, Damien Miller wrote:
> > Hi,
> > 
> > For a long time OpenBSD has been careful about filtering potentially-
> > hostile strings that were destined for logs or TTYs using strvis(3) and
> > friends. Unfortunately, these don't do a great job for UTF-8 strings
> > since they mangle anything that isn't basic ASCII (not even ISO-8859-1).
> 
> Any news on this? I'd like to use it.

Here's the current version of the patch that incorporates most of the
feedback I received on the old one. I was hoping that someone could
pick it up at the utf8 hackathon and convert it to be a libutil
function.


diff --git a/lib/Makefile b/lib/Makefile
index ed505b4..05cf8a0 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -34,7 +34,8 @@ SRCS= ${LIB_SRCS} \
smult_curve25519_ref.c \
kexc25519.c kexc25519c.c kexc25519s.c \
roaming_dummy.c \
-   chacha.c poly1305.c cipher-chachapoly.c ssh-ed25519.c hmac.c umac.c
+   chacha.c poly1305.c cipher-chachapoly.c ssh-ed25519.c hmac.c umac.c \
+   utf8_stringprep.c
 
 .if (${SSH1:L} == "yes")
 SRCS+= cipher-3des1.c cipher-bf1.c
diff --git a/misc.h b/misc.h
index 53d469b..e476f1d 100644
--- a/misc.h
+++ b/misc.h
@@ -133,4 +133,7 @@ char*read_passphrase(const char *, int);
 int ask_permission(const char *, ...) __attribute__((format(printf, 1, 
2)));
 int read_keyfile_line(FILE *, const char *, char *, size_t, u_long *);
 
+/* utf8_stringprep.c */
+int utf8_stringprep(const char *, char *, size_t);
+
 #endif /* _MISC_H */
diff --git a/ssh.c b/ssh.c
index 2275b6e..b235a68 100644
--- a/ssh.c
+++ b/ssh.c
@@ -62,6 +62,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #ifdef WITH_OPENSSL
 #include 
@@ -500,6 +501,8 @@ main(int ac, char **av)
struct ssh_digest_ctx *md;
u_char conn_hash[SSH_DIGEST_MAX_LENGTH];
 
+   setlocale(LC_CTYPE, "");
+
/* Ensure that fds 0, 1 and 2 are open or directed to /dev/null */
sanitise_stdfd();
 
diff --git a/sshconnect2.c b/sshconnect2.c
index 2b525ac..162befe 100644
--- a/sshconnect2.c
+++ b/sshconnect2.c
@@ -39,6 +39,8 @@
 #include 
 #include 
 #include 
+#include 
+#include 
 
 #include "xmalloc.h"
 #include "ssh.h"
@@ -455,21 +457,53 @@ input_userauth_error(int type, u_int32_t seq, void *ctxt)
return 0;
 }
 
+/*
+ * Check whether we can display UTF-8 safely.
+ * NB. requires setlocale() to have been called.
+ */
+static int
+utf8_ok(void)
+{
+   static int ret = -1;
+   char *cp;
+
+   if (ret == -1) {
+   cp = nl_langinfo(CODESET);
+   ret = strcmp(cp, "UTF-8") == 0;
+   }
+   return ret;
+}
+
 /* ARGSUSED */
 int
 input_userauth_banner(int type, u_int32_t seq, void *ctxt)
 {
char *msg, *raw, *lang;
-   u_int len;
+   u_int done, len;
 
debug3("input_userauth_banner");
+
raw = packet_get_string();
lang = packet_get_string(NULL);
if (len > 0 && options.log_level >= SYSLOG_LEVEL_INFO) {
if (len > 65536)
len = 65536;
msg = xmalloc(len * 4 + 1); /* max expansion from strnvis() */
-   strnvis(msg, raw, len * 4 + 1, VIS_SAFE|VIS_OCTAL|VIS_NOSLASH);
+   done = 0;
+   if (utf8_ok()) {
+   if (utf8_stringprep(raw, msg, len * 4 + 1) == 0)
+   done = 1;
+   else
+   debug2("%s: UTF8 stringprep failed", __func__);
+   }
+   /*
+* Fallback to strnvis if UTF8 display not supported or
+* conversion failed.
+*/
+   if (!done) {
+   strnvis(msg, raw, len * 4 + 1,
+   VIS_SAFE|VIS_OCTAL|VIS_NOSLASH);
+   }
fprintf(stderr, "%s", msg);
free(msg);
}
diff --git a/stringprep-tables.c b/stringprep-tables.c
new file mode 100644
index 000..49f4d9d
--- /dev/null
+++ b/stringprep-tables.c
@@ -0,0 +1,661 @@
+/* Public domain.  */
+
+/* $OpenBSD$ */
+
+/*
+ * Tables for RFC3454 stringprep algorithm, updated with a table of allocated
+ * characters generated from Unicode.6.2's UnicodeData.txt
+ *
+ * Intended to be included directly from utf8_stringprep.c
+ */
+
+/* Unassigned characters in Unicode 6.2 */
+static const struct u32_range unassigned[] = {
+   { 0x0378, 0x0379 },
+   { 0x037F, 0x0383 },
+   { 0x038B, 0x038B },
+   { 0x038D, 0x038D },
+   { 0x03A2, 0x03A2 },
+   { 0x0528, 0x0530 },
+   { 0x0557, 0x0558 },
+   { 0x0560, 0x0560 },
+   { 0x0588, 0x0588 },
+   { 0x058B, 0x058E },
+   { 0x0590, 0x0590 },
+   { 0x05C8, 0x05CF },
+   { 0x05EB, 0

UTF-8 string filtering

2015-09-03 Thread Damien Miller
{ 0x, 0x001F },
+   { 0x007F, 0x007F },
+   /* C.2.2 Non-ASCII control characters */
+   { 0x0080, 0x009F },
+   { 0x06DD, 0x06DD },
+   { 0x070F, 0x070F },
+   { 0x180E, 0x180E },
+   { 0x200C, 0x200C },
+   { 0x200D, 0x200D },
+   { 0x2028, 0x2028 },
+   { 0x2029, 0x2029 },
+   { 0x2060, 0x2060 },
+   { 0x2061, 0x2061 },
+   { 0x2062, 0x2062 },
+   { 0x2063, 0x2063 },
+   { 0x206A, 0x206F },
+   { 0xFEFF, 0xFEFF },
+   { 0xFFF9, 0xFFFC },
+   { 0x1D173, 0x1D17A },
+   /* C.3 Private use */
+   { 0xE000, 0xF8FF },
+   { 0xF, 0xD },
+   { 0x10, 0x10FFFD },
+   /* C.4 Non-character code points */
+   { 0xFDD0, 0xFDEF },
+   { 0xFFFE, 0x },
+   { 0x1FFFE, 0x1 },
+   { 0x2FFFE, 0x2 },
+   { 0x3FFFE, 0x3 },
+   { 0x4FFFE, 0x4 },
+   { 0x5FFFE, 0x5 },
+   { 0x6FFFE, 0x6 },
+   { 0x7FFFE, 0x7 },
+   { 0x8FFFE, 0x8 },
+   { 0x9FFFE, 0x9 },
+   { 0xAFFFE, 0xA },
+   { 0xBFFFE, 0xB },
+   { 0xCFFFE, 0xC },
+   { 0xDFFFE, 0xD },
+   { 0xEFFFE, 0xE },
+   { 0xE, 0xF },
+   { 0x10FFFE, 0x10 },
+   /* C.5 Surrogate codes */
+   { 0xD800, 0xDFFF },
+   /* C.6 Inappropriate for plain text */
+   { 0xFFF9, 0xFFF9 },
+   { 0xFFFA, 0xFFFA },
+   { 0xFFFB, 0xFFFB },
+   { 0xFFFC, 0xFFFC },
+   { 0xFFFD, 0xFFFD },
+   /* C.7 Inappropriate for canonical representation */
+   { 0x2FF0, 0x2FFB },
+   /* C.8 Change display properties or are deprecated */
+   { 0x0340, 0x0340 },
+   { 0x0341, 0x0341 },
+   { 0x200E, 0x200E },
+   { 0x200F, 0x200F },
+   { 0x202A, 0x202A },
+   { 0x202B, 0x202B },
+   { 0x202C, 0x202C },
+   { 0x202D, 0x202D },
+   { 0x202E, 0x202E },
+   { 0x206A, 0x206A },
+   { 0x206B, 0x206B },
+   { 0x206C, 0x206C },
+   { 0x206D, 0x206D },
+   { 0x206E, 0x206E },
+   { 0x206F, 0x206F },
+   /* C.9 Tagging characters */
+   { 0xE0001, 0xE0001 },
+   { 0xE0020, 0xE007F },
+};
diff --git a/utf8_stringprep.c b/utf8_stringprep.c
new file mode 100644
index 000..dcbd304
--- /dev/null
+++ b/utf8_stringprep.c
@@ -0,0 +1,457 @@
+/*
+ * Copyright (c) 2013 Damien Miller <d...@mindrot.org>
+ *
+ * Permission to use, copy, modify, and distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+
+/*
+ * This is a simple RFC3454 stringprep profile to sanitise UTF-8 strings
+ * from untrusted sources.
+ *
+ * It is intended to be used prior to display of untrusted strings only.
+ * It should not be used for logging because of bi-di ambiguity. It
+ * should also not be used in any case where lack of normalisation may
+ * cause problems.
+ *
+ * This profile uses the prohibition and mapping tables from RFC3454
+ * (listed below) but the unassigned character table has been updated to
+ * Unicode 6.2. It uses a local whitelist of whitespace characters (\n,
+ * \a and \t). Unicode normalisation and bi-di testing are not used.
+ *
+ * XXX: implement bi-di handling (needed for logs)
+ * XXX: implement KC normalisation (needed for passing to libs/syscalls)
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "misc.h"
+
+struct u32_range {
+   u_int32_t lo, hi;  /* Inclusive */
+};
+
+#include "stringprep-tables.c"
+
+/* Returns 1 if code 'c' appears in the table or 0 otherwise */
+static int
+code_in_table(u_int32_t c, const struct u32_range *table, size_t tlen)
+{
+   const struct u32_range *e, *end = (void *)(tlen + (char *)table);
+
+   for (e = table; e < end; e++) {
+   if (c >= e->lo && c <= e->hi)
+   return 1;
+   }
+   return 0;
+}
+
+/*
+ * Decode the next valid UCS character from a UTF-8 string, skipping past bad
+ * codes. Returns the decoded character or 0 for end-of-string and updates
+ * nextc to point to the start of the next character (if any).
+ * had_error is set if an invalid code was encountered.
+ */
+static u_int32_t
+decode_utf8(const char *in, const char **nextc, int *had_error)
+{
+   int state = 0;
+   size_t i;
+   u_int32_t c, e;
+
+   e = c = 0;

Re: NTRU Open Source Project / Post-quantum era

2015-05-25 Thread Damien Miller


On Sat, 23 May 2015, ertetlen barmok wrote:

 Hello, 
 
 https://github.com/NTRUOpenSourceProject
 
 When will LibreSSL have ciphers for the Post-quantum era? 
 
 http://tech.slashdot.org/story/15/05/15/007248/are-we-entering-a-golden-age-of-quantum-computing-research

From wikipedia: NTRU is a patented and ...

oh, I stopped reading there.




Re: OpenBSD/NTRU policy mismatch [Was: NTRU Open Source Project / Post-quantum era]

2015-05-25 Thread Damien Miller
No clarification needed: NTRU is patented, with no free for all patent
grant. It is a complete non-starter for OpenBSD or OpenSSH.

On Tue, 26 May 2015, Douglas Ray wrote:

 Thanks William and Ertetlen for clarifying:
 
 
 On 25/05/15 10:09 PM, William Whyte wrote:
  Hi Ertetlen,
  
  The base license for NTRU is GPL v2 or higher. However, there's a
  license to distribute NTRU under GPL alongside open source projects that
  exist under other licenses: see details at
  
  https://github.com/NTRUOpenSourceProject/ntru-crypto/blob/master/FOSS%20Exception.md
 
 1. I can't speak for the real developers of OpenBSD.
 (and I'm not advocating anything here, just trying to
 keep the issues clear).
 
 2. The FOSS exception clause above won't help with existing
 OpenBSD policy, insofar as I understand it here:
   http://www.openbsd.org/policy.html
 [note section towards end on GPL under Specific Cases]
 
 So NTRU doesn't seem likely.  It would require the project
 leaders / developers to find some irresistable attraction in
 NTRU, so great that they wanted to modify the licence policy.
 
 3. I am so out-of-touch I didn't realise new OpenBSD code is
 under a re-wording of an ISC licence - not the two-clause BSD -
 so OpenBSD may no longer comply with NTRU's FOSS exception
 clause.
 http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/share/misc/license.template?rev=HEAD
 
 cheers,
 Douglas
 
  Which NTRU ciphersuite were you implementing? We're in the process of
  specifying a new hybrid ciphersuite that allows an NTRU key exchange
  to be run in parallel to a selected classical ciphersuite, allowing
  users to use a currently trusted algorithm with NTRU as an additional
  layer of security; I attach the relevant (not yet distributed) Internet
  Draft, we'd value your feedback.
  
  Cheers,
  
  William
  
  
  
  
  
  On Sun, May 24, 2015 at 8:12 AM, ertetlen barmok
  ertetlenbar...@safe-mail.net mailto:ertetlenbar...@safe-mail.net wrote:
  
  Hello,
  
  Is the NTRU source available via BSD licence?
  
  Thank you.
  
   Original Message 
  From: Douglas Ray doug...@cpan.org mailto:doug...@cpan.org
  To: ertetlen barmok ertetlenbar...@safe-mail.net
  Subject: Re: NTRU Open Source Project / Post-quantum era
  Date: Sun, 24 May 2015 20:32:29 +1000
  
to clarify
   
On 24/05/15 1:57 AM, Mike Burns wrote:
 On 2015-05-23 05.24.30 -0400, ertetlen barmok wrote:
 https://github.com/NTRUOpenSourceProject


  
  https://github.com/NTRUOpenSourceProject/ntru-crypto/blob/master/LICENSE.md

 NTRU cryptographic IP and reference software may be used and
  modified
 to the needs of the user as long as the user adheres to version
  two (2)
 or higher of the GPL License

 When will LibreSSL have ciphers for the Post-quantum era?

 When you submit the patch -- with the correct license.


   
OpenBSD excludes GPL - it uses a BSD licence.
   
Wikipedia claims NTRU is available under GPL or BSD licence.
This seems to be contradicted by the NTRU source quoted above.
   
Is there another tree using BSD licencing?
  
  
 
 



Re: OpenBSD on Kosagi Novena open-source ARM board/desktop/laptop

2015-05-11 Thread Damien Miller
On Mon, 11 May 2015, Jonathan Gray wrote:

  If you can get an installation completed (the imxenet is pretty flaky,
  possibly because of an all-0 MAC address), then you'll also need to
  copy bsd*.IMX.umg to the /boot partition of the sdcard under Linux
  (OpenBSD can't access the sdcard yet) and arrange uboot to fatload
  it from there. Something like:
 
 There are two sd slots which one doesn't work?
 
 The (internal?) micro-sd slot doesn't have card detect
 the (external?) normal sized sd slot does

I haven't tried the external one, but the internal one doesn't work.
I'm not sure which is which in the dmesg:

imxesdhc0 at imx0
sdmmc0 at imxesdhc0
imxesdhc1 at imx0
sdmmc1 at imxesdhc1
...
sdmmc1: can't enable card

-d



OpenBSD on Kosagi Novena open-source ARM board/desktop/laptop

2015-05-10 Thread Damien Miller
Hi,

Thanks to jsg@, the latest snapshot releases of
OpenBSD will boot on the Novenai open-source laptop
(http://www.kosagi.com/w/index.php?title=Novena_Main_Page). It's still
very rough: no SMP, flaky USB and support for the eeprom (so no MAC
addr on the IMX ethernet), but it gets to multi-user and could probably
compile its own kernel given the chance.

http://pastebin.com/raw.php?i=dr2qZSi2 is a boot log

To try it out, the easiest way is to setup a tftp server and configure
DHCP to tell Novena about it. Then boot/reset with the user button held
down, ^C when prompted (be quick, you only have a couple of seconds) and
issue the following u-boot commands:

setenv machid 10ad
setenv loadaddr 0x1080
dhcp
bootm

Note that u-boot doesn't seem to fully reset the ethernet
controller/MAC. If you experience partial transfers that abort due to
timeouts during the 'dhcp' command then a workaround is to completely
remove power from Novena and try again after a minute or three.

If you can get an installation completed (the imxenet is pretty flaky,
possibly because of an all-0 MAC address), then you'll also need to
copy bsd*.IMX.umg to the /boot partition of the sdcard under Linux
(OpenBSD can't access the sdcard yet) and arrange uboot to fatload
it from there. Something like:

setenv machid 10ad
setenv loadaddr 0x1080
fatload mmc 0 0x1080 bsd.IMG.umg
bootm

I baked these sequences into u-boot by following the instructions at
http://www.kosagi.com/w/index.php?title=U-boot_PVT_Notes and
modifying include/configs/novena.h with the additional commands,
but it's also probably possible to put them in the /boot/uEnv.txt file
that uboot reads at startup.

-d



Re: GSoC project: KMS driver for Cirrus Logic graphics

2015-05-10 Thread Damien Miller
On Mon, 11 May 2015, L?o Grange wrote:

 The goal of the project is to port the current Cirrus userland driver
 from X.Org to an OpenBSD KMS driver, and to document the process in
 order to make easier the addition of new KMS drivers for various
 graphics adapter.
 As QEMU emulates a Cirrus CLGD 5446, this card is virtually one of the
 easiest to find for testing purpose, is relatively well documented,
 and is simple compared to latest intel or radeon cards. For these
 reasons it's a pretty good candidate to be used as a sample
 implementation of DRM/KMS.

Hi and welcome!

Possibly dumb question: why the Cirrus CLGD 5446? It doesn't seem like
a very common chipset.

Is the goal of the project to have a model KMS driver and
documentation that can serve as a guide? Or a KMS driver that works well
in a common virtualisation environment? or something else?

-d



Re: seccomp system call

2015-05-05 Thread Damien Miller
On Mon, 4 May 2015, Todd C. Miller wrote:

 On Sun, 03 May 2015 20:44:25 -, Loganaden Velvindron wrote:
 
  OpenBSD already has systrace.
 
 Last I checked, systrace doesn't work well with multi-threaded
 programs and was trivial to bypass.  The basic design where you
 have a userland monitor process is flawed.  Something where a policy
 is pushed into (and enforced by) the kernel would be better.

seccomp-bpf doesn't offer inspection of indirect syscall arguments
either, so in that regard it's no better than systrace. AFAIK it
would be less intrusive, have fewer knobs and not need a persistent
root-privileged monitor process though.

-d




Re: seccomp system call

2015-05-05 Thread Damien Miller
On Mon, 4 May 2015, Theo de Raadt wrote:

 Personally, I think seccomp-bpf could be a superior alternative to
 systrace and I'd love to see an implementation. Other developers (inc.
 Theo) are skeptical though, but this is probably a case where the
 argument won't be settled without a concrete implementation to look at.
 
 I am very skeptical about a bpf-style model, because:
 
 People are currently writing policies specific to what glibc does;
 or what they believe it is doing.

I don't think we could expect to use syscall policies written for
Linux as anything more than rough guidance (or perhaps cautionary
example)

 Those policies will be wide open, or too strict.  If we adopt this
 into our world, the next step after that is going to be wide use of
 #ifdef within bpf rulesets.

I don't think that's necessarily true, OpenSSH only uses ifdef in
the systrace rulesets for new/deprecated syscalls and the bpf
rulesets would be identical. Most other privsep daemons would be
in a similar boat I expect.

-d



Re: seccomp system call

2015-05-03 Thread Damien Miller
On Sun, 3 May 2015, Nicolas Bedos wrote:

 I am wondering if the seccomp system call [1] would be welcomed
 in the OpenBSD tree. I remember it was among the subjects of last
 year's Google Summer of Code. If there is still interest in having
 it implemented, I am willing to work on it: I have a diff that
 creates the system call and allows seccomp to be called with the
 SECCOMP_SET_MODE_STRICT operation. It's a first step, the next (big)
 one would be BPF(4) syscall filtering.

Personally, I think seccomp-bpf could be a superior alternative to
systrace and I'd love to see an implementation. Other developers (inc.
Theo) are skeptical though, but this is probably a case where the
argument won't be settled without a concrete implementation to look at.

I'd welcome any work you do on it.

-d



OpenSSH: ssh protocol 1 now disabled at compile time

2015-03-23 Thread Damien Miller
Hi,

I just committed a change to src/usr.bin/ssh/Makefile.inc to compile-
time disable SSH protocol 1. This protocol is old, unsafe and really,
really shouldn't be used at all any more.

If you have need of it, then you can re-enable it for yourself using
the knob in Makefile.inc.

If you run into bugs related to this change, please tell
open...@openssh.com and we'll fix them quickly. We're deliberately
doing this change early in the release cycle to flush out bugs and
find out how many people are still using this terrible old protocol.

-d



Re: ksh version lies

2015-02-15 Thread Damien Miller
On Sun, 15 Feb 2015, Ted Unangst wrote:

 ksh (and sh) have a version string embedded in them:
 @(#)PD KSH v5.2.14 99/07/13.2
 
 This is clearly a lie. We've added, removed, and fixed bugs and features since
 then. I first noticed the lie in the man page, then saw that it's also
 exported via the environment and other places.
 
 Instead of trying to fix something that can't be fixed,

That's a bit of a non sequitur; there are lots of ways to fix it.
E.g. OpenBSD 5.7 ksh; based on PD KSH v5.2.14

-d




Re: permuate lines in random

2014-12-22 Thread Damien Miller
On Mon, 22 Dec 2014, Ted Unangst wrote:

 I would like to generate a permutation of some lines. We have random,
 which is vageuly similar. This adds a -p option to instead permute
 instead of randomly select.

 + for (j = numlines; j  1; j--) {
 + size_t s = arc4random_uniform(j);
 + char *tmp = array[s];
 + array[s] = array[j - 1];
 + array[j - 1] = tmp;
 + }

You could use the forward Knuth shuffle like that used in ip_id.c to
perform the permutation while the lines are being read and avoid the
extra pass.

Some people have said that this is less random than the traditional
backwards shuffle, but none have ever provided an argument why and I
couldn't see how they differ myself.

-d



Re: improving OpenBSD's gmac.c...

2014-11-12 Thread Damien Miller
On Wed, 12 Nov 2014, Mike Belopuhov wrote:

  isn't this likely to make it more likely to be subject to timing
  attacks?
 
 
 then how is this different to our table based aes implementation?
 and it's the same C code as in openssl which also uses table based
 gcm implementation.

Yeah, that's crappy too - IMO we should definitely look at replacing it,
but I haven't found an implementation that is a) native C, b) doesn't
use FP/SSE tricksies and c) is acceptably fast.

OpenSSL only falls back to the table-based code if none of the assembler
versions are selected for the platform. AFAIK many of these are constant
time.

 what countermeasures can be applied to the table lookup code
 to fight these attacks?

It's possible to do dummy accesses, but these do slow things down and
many have been broken anyway.

-d



Re: improving OpenBSD's gmac.c...

2014-10-09 Thread Damien Miller
On Thu, 9 Oct 2014, Christian Weisgerber wrote:

 John-Mark Gurney:
 
  I also have an implementation of ghash that does a 4 bit lookup table
  version with the table split between cache lines in p4 at:
  https://p4db.freebsd.org/fileViewer.cgi?FSPC=//depot/projects/opencrypto/sys/opencrypto/gfmult.cREV=4
  
  This also has a version with does 4 blocks at a time getting a
  further speed up...
 
 FWIW, I did a quick  dirty merge of this into the OpenBSD tree and
 the speed of my test (net6501-50, tcpbench -u over esp aes-128-gmac)
 almost doubled.

isn't this likely to make it more likely to be subject to timing
attacks?

-d



Re: reduce the number of missed PCB cache with tcpbench -su

2014-08-29 Thread Damien Miller
On Fri, 29 Aug 2014, Daniel Jakots wrote:

 Hi,
 
 When running tcpbench -su, a lot of them are counted as missed PCB
 cache.
...
 + n = recvfrom(fd, ptb-dummybuf, ptb-dummybuf_len, 0,
 + (struct sockaddr *)ss, slen);
 + if (n  0  connect(fd, (const struct sockaddr *)ss, slen))
 + warn(fail to connect);

What's the benefit of this? I've never seen an application do this;
wouldn't it be better to improve the PCB cache so it caught this case
(which seems the usual way UDP applications behave) instead?

-d



Re: slightly stricter check for genentropy_urandom

2014-06-25 Thread Damien Miller
On Wed, 25 Jun 2014, Martijn van Duren wrote:

 Hello tech@,
 
 Here is a minor diff to do a little more strict checking on the device id for
 urandom. It would be a shame if someone replaced a genuine urandom with a
 /dev/null or some other predictable device.

that's what the ioctl is for



Re: compare memcmp with 0

2014-06-19 Thread Damien Miller
On Thu, 19 Jun 2014, Ted Unangst wrote:

 Always explicitly compare memcmp with 0. I find this adds clarity.

If you don't care which way a different comparison points, then why
not use bcmp?



Re: malloc freelists

2014-05-01 Thread Damien Miller
On Thu, 1 May 2014, Ted Unangst wrote:

 What's better than a freelist? Four freelists!

Apart from moar = better, what's the motivation? Do you have a particular
attack in mind? The only thing I can think of where this change might help
is an attack that speculatively spams small offsets from the overflow and
hopes it doesn't run off the end of the page, and this seems fairly
contrived...

-d



Re: polling SSL kerberos and srp support

2014-04-29 Thread Damien Miller
On Mon, 28 Apr 2014, Ted Unangst wrote:

 Hi there. I'm trying to find somebody who is actually using either
 Kerberos or SRP support in libssl. I'm inclined to remove support for
 them. While the bulk of the code sits off to the side, the integration
 requires adding several additional cases to some of the most critical
 paths.

+1 to dropping both in the bin



Re: Switch OpenBSD manuals to DocBook

2014-04-01 Thread Damien Miller
On Tue, 1 Apr 2014, Christian Weisgerber wrote:

 On 2014-04-01, Theo de Raadt dera...@cvs.openbsd.org wrote:
 
  Another approach is to extend the usage() in every program so that it
  provides more information.
 
 Just embed the whole man page, as in curl -M.

Putting stuff in usage() is pretty retro. Modern programs embed a small
webserver so you can read the documentation with a web browser of your
choice.



Re: Boot network for remote unlock of fde

2014-03-05 Thread Damien Miller
On Wed, 5 Mar 2014, Stuart Henderson wrote:

 What are you trying to protect against?

 If somebody has physical access, they can presumably replace the
 kernel/initramfs with a trojanned version ...

It protects against stolen machines, but not active attacks.

Our cryptoraid doesn't protect against active attacks either - the
attacker can replace the bootloader with something that phishes your
password. The closest we could get to fixing that would be to use the
TPM on some x86 systems, but there are ways around that too...

-d



Re: Weird loop in ftp client

2013-11-22 Thread Damien Miller
On Fri, 22 Nov 2013, Stuart Henderson wrote:

   do {
   wr = write(fileno(fout), buf + d, rd);
 - if (wr == -1  errno == EPIPE)
 - break;
 - d += wr;
 - rd -= wr;
 + if (wr == -1) {
 + if (errno == EPIPE)
 + break;
 + } else {
 + d += wr;
 + rd -= wr;
 + }
   } while (d  c);

That still loops endlessly for errors other than EPIPE.



Re: more /dev/ugen*

2013-09-13 Thread Damien Miller
On Fri, 13 Sep 2013, Martin Pieuchot wrote:

   16097 pcscdNAMI  /dev/ugen2.00
 
 Out of curiosity, can I see the dmesg for this machine?  I'd like to
 know which devices attach at ugen(4).s

It's a Lenovo x61t. Two devices attach to ugen before I plug anything in,
the built-in fingerprint reader (blech) and bluetooth:

OpenBSD 5.4-current (GENERIC.MP) #27: Mon Aug 26 21:24:08 EST 2013
d...@mothra.mindrot.org:/usr/src/sys/arch/amd64/compile/GENERIC.MP
real mem = 8496283648 (8102MB)
avail mem = 8262021120 (7879MB)
mainbus0 at root
bios0 at mainbus0: SMBIOS rev. 2.4 @ 0xe0010 (63 entries)
bios0: vendor LENOVO version 7SET36WW (1.22 ) date 11/27/2008
bios0: LENOVO 776773M
acpi0 at bios0: rev 2
acpi0: sleep states S0 S3 S4 S5
acpi0: tables DSDT FACP SSDT ECDT TCPA APIC MCFG HPET SLIC BOOT ASF! SSDT SSDT 
SSDT SSDT
acpi0: wakeup devices LID_(S3) SLPB(S3) DURT(S3) IGBE(S4) EXP0(S4) EXP1(S4) 
EXP2(S4) EXP3(S4) EXP4(S4) PCI1(S4) USB0(S3) USB1(S3) USB2(S3) USB3(S3) 
USB4(S3) EHC0(S3) [...]
acpitimer0 at acpi0: 3579545 Hz, 24 bits
acpiec0 at acpi0
acpimadt0 at acpi0 addr 0xfee0: PC-AT compat
cpu0 at mainbus0: apid 0 (boot processor)
cpu0: Intel(R) Core(TM)2 Duo CPU L7500 @ 1.60GHz, 1795.79 MHz
cpu0: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,DTES64,MWAIT,DS-CPL,VMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,NXE,LONG,LAHF,PERF
cpu0: 4MB 64b/line 16-way L2 cache
cpu0: smt 0, core 0, package 0
cpu0: apic clock running at 199MHz
cpu1 at mainbus0: apid 1 (application processor)
cpu1: Intel(R) Core(TM)2 Duo CPU L7500 @ 1.60GHz, 1596.01 MHz
cpu1: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,DTES64,MWAIT,DS-CPL,VMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,NXE,LONG,LAHF,PERF
cpu1: 4MB 64b/line 16-way L2 cache
cpu1: smt 0, core 1, package 0
ioapic0 at mainbus0: apid 1 pa 0xfec0, version 20, 24 pins
ioapic0: misconfigured as apic 2, remapped to apid 1
acpimcfg0 at acpi0 addr 0xf000, bus 0-63
acpihpet0 at acpi0: 14318179 Hz
acpiprt0 at acpi0: bus 0 (PCI0)
acpiprt1 at acpi0: bus -1 (AGP_)
acpiprt2 at acpi0: bus 2 (EXP0)
acpiprt3 at acpi0: bus 3 (EXP1)
acpiprt4 at acpi0: bus -1 (EXP2)
acpiprt5 at acpi0: bus -1 (EXP3)
acpiprt6 at acpi0: bus -1 (EXP4)
acpiprt7 at acpi0: bus 5 (PCI1)
acpicpu0 at acpi0: C3, C2, C1, PSS
acpicpu1 at acpi0: C3, C2, C1, PSS
acpipwrres0 at acpi0: PUBS
acpitz0 at acpi0: critical temperature is 127 degC
acpitz1 at acpi0: critical temperature is 100 degC
acpibtn0 at acpi0: LID_
acpibtn1 at acpi0: SLPB
acpibat0 at acpi0: BAT0 model 93P5030 serial75 type LION oem SANYO
acpibat1 at acpi0: BAT1 not present
acpibat2 at acpi0: BAT2 not present
acpiac0 at acpi0: AC unit online
acpithinkpad0 at acpi0
acpidock0 at acpi0: GDCK not docked (0)
cpu0: Enhanced SpeedStep 1795 MHz: speeds: 1601, 1600, 1200, 800 MHz
pci0 at mainbus0 bus 0
pchb0 at pci0 dev 0 function 0 Intel GM965 Host rev 0x0c
vga1 at pci0 dev 2 function 0 Intel GM965 Video rev 0x0c
intagp0 at vga1
agp0 at intagp0: aperture at 0xe000, size 0x1000
inteldrm0 at vga1
drm0 at inteldrm0
intel_overlay_map_regs partial stub
inteldrm0: 1024x768
wsdisplay0 at vga1 mux 1: console (std, vt100 emulation)
wsdisplay0: screen 1-5 added (std, vt100 emulation)
Intel GM965 Video rev 0x0c at pci0 dev 2 function 1 not configured
em0 at pci0 dev 25 function 0 Intel ICH8 IGP M AMT rev 0x03: msi, address 
00:16:d3:3e:ba:40
uhci0 at pci0 dev 26 function 0 Intel 82801H USB rev 0x03: apic 1 int 20
uhci1 at pci0 dev 26 function 1 Intel 82801H USB rev 0x03: apic 1 int 21
ehci0 at pci0 dev 26 function 7 Intel 82801H USB rev 0x03: apic 1 int 22
usb0 at ehci0: USB revision 2.0
uhub0 at usb0 Intel EHCI root hub rev 2.00/1.00 addr 1
azalia0 at pci0 dev 27 function 0 Intel 82801H HD Audio rev 0x03: msi
azalia0: codecs: Analog Devices AD1984, Conexant/0x2bfa, using Analog Devices 
AD1984
audio0 at azalia0
ppb0 at pci0 dev 28 function 0 Intel 82801H PCIE rev 0x03: msi
pci1 at ppb0 bus 2
ppb1 at pci0 dev 28 function 1 Intel 82801H PCIE rev 0x03: msi
pci2 at ppb1 bus 3
athn0 at pci2 dev 0 function 0 Atheros AR5418 rev 0x01: apic 1 int 17
athn0: MAC AR5418 rev 2, RF AR5133 (2T3R), ROM rev 8, address 00:21:63:2f:e9:e7
uhci2 at pci0 dev 29 function 0 Intel 82801H USB rev 0x03: apic 1 int 16
uhci3 at pci0 dev 29 function 1 Intel 82801H USB rev 0x03: apic 1 int 17
ehci1 at pci0 dev 29 function 7 Intel 82801H USB rev 0x03: apic 1 int 19
usb1 at ehci1: USB revision 2.0
uhub1 at usb1 Intel EHCI root hub rev 2.00/1.00 addr 1
ppb2 at pci0 dev 30 function 0 Intel 82801BAM Hub-to-PCI rev 0xf3
pci3 at ppb2 bus 5
cbb0 at pci3 dev 0 function 0 Ricoh 5C476 CardBus rev 0xba: apic 1 int 16
sdhc0 at pci3 dev 0 function 2 Ricoh 5C822 SD/MMC rev 0x21: apic 1 int 18
sdmmc0 at sdhc0
cardslot0 at cbb0 slot 0 flags 0
cardbus0 at cardslot0: bus 6 device 0 cacheline 0x8, lattimer 0xb0
pcmcia0 at cardslot0
pcib0 at pci0 dev 31 function 0 Intel 82801HEM LPC 

Re: base apache and HonorCipherOrder

2013-07-07 Thread Damien Miller
On Sun, 7 Jul 2013, Aaron Stellman wrote:

 On Tue, Apr 23, 2013 at 09:08:19AM +0200, Otto Moerbeek wrote:
  If there is any interest, I might add the manual stuff, get ok's and
  commit it. 
 
 I find it useful to have SSLHonorCipherOrder in OpenBSD's apache.

More than that, AFAIK it is necessary to mitigate some of the TLS crypto
attacks. IMO it is well worth having.

It would also be good if someone could make a patch to enable ECDHE cipher
suites in Apache-1.x. This nginx patch is a good reference to what needs to
be done:

http://hg.nginx.org/nginx/rev/0832a6997227

-d



Re: bzip2

2013-06-06 Thread Damien Miller
On Thu, 6 Jun 2013, David Coppa wrote:

  But even more so than with nl(1), why would we want to use something
  that's different from what everybody else uses?  If we want bzip2 in
  base (and I think there are good reasons for having it) we should
  simply use the standard bzip2 code.
 
 Seconded.

Thirded



Re: add nl(1)

2013-05-09 Thread Damien Miller
On Wed, 8 May 2013, Ted Unangst wrote:

 On Tue, Apr 30, 2013 at 18:57, Arto Jonsson wrote:
  Taken from netbsd with minor modifications. Comments?
 
 I don't think you've received much feedback. I don't know how other
 developers feel, but the question I have is can't this be done with a
 rather simple awk script? or perl? One of the reasons we have perl in
 base is precisely so it can be used for things like this.

This implementation has the benefits of being small, having existing
maintainers (NetBSD) and already having been written and debugged. It
seems like make-work to do it over in Perl.

-d



Re: DPI for pf(4)

2013-05-02 Thread Damien Miller
On Wed, 1 May 2013, Franco Fichtner wrote:

 Not sure if that's a fitting comparison; and I know too little OSPF
 to answer.  Let me try another route.  The logic consists of an array
 of application detection functions, which can be invoked via their
 respective IP types.

I don't like this approach at all - it leads to a proliferation (as
demonstrated by your already long list) of kernel-side parsers that
will be a maintenance, and possibly security, nightmare.

The last thing we want it a rotting pile of protocol parsing code like
wireshark.

 On May 1, 2013, at 1:14 AM, Ted Unangst t...@tedunangst.com wrote:

  My thoughts on the matter have always been that it would be cool to
  integrate bpf into pf (though other developers surely have other
  opinions). Then you get filtering for as many protocols as you care to
  write bpf matchers for.
 
 You mean externalising the DPI?  People[1] have tried to work on such
 ideas, but the general drift is that there are not enough interested
 individuals in the field to drive second tier development for
 application detections.

So if there is not enough interest to develop app protocol detectors/
disectors in bpf then why should C be any different?

 I find C to be quite flexible and empowering
 if one doesn't overcomplicate[2].

 [2] https://github.com/fichtner/OpenDPI/blob/master/src/lib/protocols/ssl.c

That's complicated and scary code for a kernel, e.g. multiple opportunities
for unsigned overflow that don't seem to be checked for.

I agree with tedu - it safer and more flexible to write parsers in bpf.
If that isn't desirable then maybe we could consider some other automata
classifier, but I think it is a bad, bad idea to do it in C.

-d



Re: DPI for pf(4)

2013-05-02 Thread Damien Miller
On Thu, 2 May 2013, Franco Fichtner wrote:

 as stated before, breaking down complexity to the bare minimum is my
 requirement for this to be happening at all.  You all get to be the
 judges.  I'm just trying to work on something worth doing.

Well, bare minimum complexity per-protocol * large_number_of_protocols =
a lot of complexity. The incentive is always going to be to add more
protocols and never retire them.

Also, doesn't IPPROTO_DIVERT or SO_BINDANY+SO_SPLICE allow you to do
near zero-overhead DPI completely in userspace?

-d



Re: DPI for pf(4)

2013-05-02 Thread Damien Miller
On Thu, 2 May 2013, Franco Fichtner wrote:

  Well, bare minimum complexity per-protocol * large_number_of_protocols =
  a lot of complexity. The incentive is always going to be to add more
  protocols and never retire them.
 
 I guess that's true for most software projects.

We try not to implement an effectively unbounded number of protocol
parsers in the kernel.

  Also, doesn't IPPROTO_DIVERT or SO_BINDANY+SO_SPLICE allow you to do
  near zero-overhead DPI completely in userspace?
 
 Wouldn't that mean pf.conf(5) syntax extensions cannot be implemented?

It doesn't mean that - you'd just need some way for userspace to signal
information to pf. E.g add a SO_PF_TAG to set the pf tag. Then you could
use some program that used SO_BINDANY to inspect the beginning of the
session, set a pf tag using setsockopt, SO_SPLICE to avoid further need
to copy the session in userspace and control the traffic in pf using the
tagged keyword.

 It's not full-blown DPI analysis for extracting all kinds of events
 from a flow -- it's merely a tagging tool, and if that sits in user
 space, it's really not helpful except for logging / accounting. One
 could do that with a simple pcap(3) binding as well.

Why not do the tagging in userspace using the existing facilities?

-d



Re: DPI for pf(4)

2013-05-02 Thread Damien Miller
On Thu, 2 May 2013, Franco Fichtner wrote:

 Moving implementations to user space does not necessarily make them
 better or less of a problem.

The big difference is that its possible to sandbox a userspace
implementation so that small integer overflow bugs or length checking
failures don't become arbitrary kmem reads or, worse, RCE.

-d



Re: DPI for pf(4)

2013-05-02 Thread Damien Miller
On Thu, 2 May 2013, Franco Fichtner wrote:

 OK, the implementation only pulls a couple of bytes from the packet's
 payload. It will never pull bytes that are not verified. It will never
 allocate anything. It will never test against something that's neither
 hard-coded nor available in the range of the approved payload. It will
 never return more than unsigned int with a number describing the
 actual application. It will never manipulate any input value, lest of
 all the packet itself. It will never run into endless loops. And I'll
 gladly zap everything that could still considered be a potential risk.

You've just described bpf, right down to no endless loops and the amount
of data it returns.

For a little more code that it takes to write one packet parser
(basically: loading bpf rules from pf and making the bpf_filter()'s
return value available to it) you get everything you described above and
more.

-d



Re: DPI for pf(4)

2013-05-02 Thread Damien Miller
On Thu, 2 May 2013, Damien Miller wrote:

 You've just described bpf, right down to no endless loops and the amount
 of data it returns.
 
 For a little more code that it takes to write one packet parser
 (basically: loading bpf rules from pf and making the bpf_filter()'s
 return value available to it) you get everything you described above and
 more.

Actually, you could even make the bpf inspection stateful and bi-directional
if you preserved its scratch memory between packets.

-d



Re: IKEd support for ECDSA key authentication?

2013-03-12 Thread Damien Miller
On Sun, 10 Mar 2013, Jason Hall wrote:

 Are there plans to support ECDSA keys?  All other recommended
 protocols (AES GCM, ECDH) are currently supported.
 
 When attempting to start IKEd (iked -dvv) with ECDSA keys, the error message 
 is:
 ca_key_serialize: unsupported key type 408
 fatal: ca: failed to serialize private key
 
 For more information on Suite B Authentication Methods, check out RFC
 6380 (https://tools.ietf.org/html/rfc6380) section 4.3, and Suite B in
 general RFC 6379 (http://tools.ietf.org/html/rfc6379).

OpenSSH also has some decent examples for handling ECDSA including
serialisation and deserialisation and public value sanity checking.

I'm happy to answer questions if someone is implementing it.

-d



Re: [PATCH] Support for virtio random device

2013-01-27 Thread Damien Miller
On Fri, 25 Jan 2013, Stefan Fritsch wrote:

 Hi,
 
 qemu 1.3 has added a virtio entropy device. Here is a driver for it.
 Comments?
 OKs?
 
 As the entropy reserve of the host may not be unlimited, the OpenBSD guest
 should only ask for entropy when it actually needs it. Would it make sense to
 add an API that allows a driver to determine how full the entropy queue is?
 This could also be used by some hardware drivers to avoid polling for entropy
 if not necessary. E.g. amdpm does the polling in every timer tick, which is
 wasteful.

I don't think such an API is necessary.

For this particular case, I agree with Kettenis that if qemu allows exhaustion
of the hypervisor's PRNG using this call then the bug lies with qemu.

That being said, if you want to avoid this bug then reading a largeish
seed once at system boot and stirring it into the PRNG would be sufficient
to ensure the PRNG pool is unpredictable to an adversary who tries to guess
its state.

-d



Re: rc.d/sshd reload test

2012-11-28 Thread Damien Miller
I like this - it's what -t is intended for.

On Wed, 28 Nov 2012, Alexander Hall wrote:

 Make sure new config is valid before SIGHUP'ing sshd, which would
 otherwise just kill it. Invalid configuration now gives:
 
 # pgrep sshd
 18998
 # /etc/rc.d/sshd reload 
 sshd(failed)
 # pgrep sshd 
 18998
 
 Pros: Less risk of accidentally killing sshd and locking yourself out.
 Cons: You may think that you have made changes that have not taken
   effect, and will still screw you after a reboot.
 
 I think the pros win anyway.
 
 OK?
 
 /Alexander
 
 Index: rc.d/sshd
 ===
 RCS file: /data/openbsd/cvs/src/etc/rc.d/sshd,v
 retrieving revision 1.1
 diff -u -p -r1.1 sshd
 --- rc.d/sshd 6 Jul 2011 18:55:36 -   1.1
 +++ rc.d/sshd 28 Nov 2012 00:27:01 -
 @@ -6,4 +6,8 @@ daemon=/usr/sbin/sshd
   . /etc/rc.d/rc.subr
  +rc_reload() {
 + ${daemon} ${daemon_flags} -t  pkill -HUP -f ^${pexp}
 +}
 +
  rc_cmd $1



Re: sftp diff to allow uploading from command line

2011-09-23 Thread Damien Miller
On Wed, 21 Sep 2011, Loganaden Velvindron wrote:

 s/similar/A little bit like
 
 The diff has issues with stuff like sftp 127.0.0.1. I've
 fixed it.

I think this might get confused by something like:

sftp blah user@host: foo user2@host:

IMO it would be better to walk all the arguments and then either 
error (if multiple remote entries were specified) or continue. What
do you think?

-d



Re: sftp diff to allow uploading from command line

2011-09-21 Thread Damien Miller
On Wed, 21 Sep 2011, Loganaden Velvindron wrote:

 s/similar/A little bit like
 
 The diff has issues with stuff like sftp 127.0.0.1. I've
 fixed it.

The way I'd like to see the sftp commandline go is to become mostly
compatible with scp(1). So:

sftp local [local...] remote:/path # do an upload
sftp remote:/path [remote:path...] local   # do a download

-d


 Index: src/usr.bin/ssh/sftp.c
 ===
 RCS file: /cvs/src/usr.bin/ssh/sftp.c,v
 retrieving revision 1.132
 diff -u -p -r1.132 sftp.c
 --- src/usr.bin/ssh/sftp.c4 Dec 2010 00:18:01 -   1.132
 +++ src/usr.bin/ssh/sftp.c21 Sep 2011 07:30:08 -
 @@ -25,6 +25,7 @@
  
  #include ctype.h
  #include errno.h
 +#include fcntl.h
  #include glob.h
  #include histedit.h
  #include paths.h
 @@ -175,7 +176,7 @@ static const struct CMD cmds[] = {
   { NULL, -1, -1  }
  };
  
 -int interactive_loop(struct sftp_conn *, char *file1, char *file2);
 +int interactive_loop(struct sftp_conn *, char *file1, char *file2, int);
  
  /* ARGSUSED */
  static void
 @@ -1834,7 +1835,7 @@ complete(EditLine *el, int ch)
  }
  
  int
 -interactive_loop(struct sftp_conn *conn, char *file1, char *file2)
 +interactive_loop(struct sftp_conn *conn, char *file1, char *file2, int tflag)
  {
   char *remote_path;
   char *dir = NULL;
 @@ -1889,10 +1890,19 @@ interactive_loop(struct sftp_conn *conn,
   }
   } else {
   if (file2 == NULL)
 - snprintf(cmd, sizeof cmd, get %s, dir);
 + if (tflag == 0)
 + snprintf(cmd, sizeof cmd, get %s,
 +  dir);
 + else
 + snprintf(cmd, sizeof cmd, put %s, 
 +  dir);
   else
 - snprintf(cmd, sizeof cmd, get %s %s, dir,
 - file2);
 + if (tflag == 0)
 + snprintf(cmd, sizeof cmd, get %s %s,
 +  dir, file2);
 + else
 + snprintf(cmd, sizeof cmd, put %s %s,
 +  dir, file2);
  
   err = parse_dispatch_command(conn, cmd,
   remote_path, 1);
 @@ -2034,12 +2044,13 @@ usage(void)
  int
  main(int argc, char **argv)
  {
 - int in, out, ch, err;
 + int in, out, ch, tflag = 0, fd, err;
   char *host = NULL, *userhost, *cp, *file2 = NULL;
   int debug_level = 0, sshver = 2;
   char *file1 = NULL, *sftp_server = NULL;
   char *ssh_program = _PATH_SSH_PROGRAM, *sftp_direct = NULL;
   const char *errstr;
 + char path[MAXPATHLEN];
   LogLevel ll = SYSLOG_LEVEL_INFO;
   arglist args;
   extern int optind;
 @@ -2163,9 +2174,24 @@ main(int argc, char **argv)
   if (optind == argc || argc  (optind + 2))
   usage();
  
 - userhost = xstrdup(argv[optind]);
 - file2 = argv[optind+1];
 -
 + if ((strrchr(argv[optind], '@') != NULL) ||
 + (argc == optind + 1) ||
 + (strrchr(argv[optind], ':') != NULL)) {
 + userhost = xstrdup(argv[optind]);
 + file2 = argv[optind + 1];
 + }
 + else {
 + tflag = 1;
 + userhost = xstrdup(argv[optind + 1]);
 + file2 = argv[optind];
 + if ((fd = open(file2, O_RDONLY | O_NONBLOCK, 0))  0) {
 + fprintf(stderr, \n%s\n,strerror(errno));
 + _exit(1);
 + }
 + if ((getcwd(path, sizeof(path)) != NULL) 
 + (file2[0] != '/'))
 + file2 = path_append(path, file2);
 + }
   if ((host = strrchr(userhost, '@')) == NULL)
   host = userhost;
   else {
 @@ -2219,9 +2245,10 @@ main(int argc, char **argv)
   else
   fprintf(stderr, Attached to %s.\n, sftp_direct);
   }
 -
 - err = interactive_loop(conn, file1, file2);
 -
 + if (tflag == 0)
 + err = interactive_loop(conn, file1, file2, 0);
 + else
 + err = interactive_loop(conn, file2, file1, 1);
   close(in);
   close(out);
   if (batchmode)



use OpenSSL EVP for SSH umac and CTR cipher modes

2011-09-08 Thread Damien Miller
Hi,

This diff needs testing, particularly on systems that support hardware
acceleration of AES via the OpenSSL EVP layer (e.g. Intel Core i7).
It uses OpenSSL's EVP AES API rather than the lower-level one and should
give an opportunity for the acceleration to work.

A useful benchmark would be

dd if=/dev/arandom bs=10 count=1000 | time ssh localhost cat  /dev/null

before and after (you will need passwordless authentication setup so
as not to stall at the password prompt)

Index: cipher-ctr.c
===
RCS file: /cvs/src/usr.bin/ssh/cipher-ctr.c,v
retrieving revision 1.11
diff -u -p -r1.11 cipher-ctr.c
--- cipher-ctr.c1 Oct 2010 23:05:32 -   1.11
+++ cipher-ctr.c9 Sep 2011 00:49:18 -
@@ -30,7 +30,7 @@ void ssh_aes_ctr_iv(EVP_CIPHER_CTX *, in
 
 struct ssh_aes_ctr_ctx
 {
-   AES_KEY aes_ctx;
+   EVP_CIPHER_CTX  aes_evp_ctx;
u_char  aes_counter[AES_BLOCK_SIZE];
 };
 
@@ -64,7 +64,9 @@ ssh_aes_ctr(EVP_CIPHER_CTX *ctx, u_char 
 
while ((len--)  0) {
if (n == 0) {
-   AES_encrypt(c-aes_counter, buf, c-aes_ctx);
+   if (EVP_Cipher(c-aes_evp_ctx, buf, c-aes_counter,
+   AES_BLOCK_SIZE) == 0)
+   fatal(%s: EVP_Cipher failed, __func__);
ssh_ctr_inc(c-aes_counter, AES_BLOCK_SIZE);
}
*(dest++) = *(src++) ^ buf[n];
@@ -83,9 +85,26 @@ ssh_aes_ctr_init(EVP_CIPHER_CTX *ctx, co
c = xmalloc(sizeof(*c));
EVP_CIPHER_CTX_set_app_data(ctx, c);
}
-   if (key != NULL)
-   AES_set_encrypt_key(key, EVP_CIPHER_CTX_key_length(ctx) * 8,
-   c-aes_ctx);
+   if (key != NULL) {
+   const EVP_CIPHER *cipher = NULL;
+
+   switch (EVP_CIPHER_CTX_key_length(ctx) * 8) {
+   case 128:
+   cipher = EVP_aes_128_ecb();
+   break;
+   case 192:
+   cipher = EVP_aes_192_ecb();
+   break;
+   case 256:
+   cipher = EVP_aes_256_ecb();
+   break;
+   default:
+   fatal(%s: invalid key length %d, __func__,
+   EVP_CIPHER_CTX_key_length(ctx) * 8);
+   }
+   if (EVP_CipherInit(c-aes_evp_ctx, cipher, key, NULL, 1) == 0)
+   fatal(%s: EVP_CipherInit failed, __func__);
+   }
if (iv != NULL)
memcpy(c-aes_counter, iv, AES_BLOCK_SIZE);
return (1);
@@ -97,6 +116,8 @@ ssh_aes_ctr_cleanup(EVP_CIPHER_CTX *ctx)
struct ssh_aes_ctr_ctx *c;
 
if ((c = EVP_CIPHER_CTX_get_app_data(ctx)) != NULL) {
+   if (EVP_CIPHER_CTX_cleanup(c-aes_evp_ctx) == 0)
+   error(%s: EVP_CIPHER_CTX_cleanup failed, __func__);
memset(c, 0, sizeof(*c));
xfree(c);
EVP_CIPHER_CTX_set_app_data(ctx, NULL);
Index: umac.c
===
RCS file: /cvs/src/usr.bin/ssh/umac.c,v
retrieving revision 1.3
diff -u -p -r1.3 umac.c
--- umac.c  12 May 2008 20:52:20 -  1.3
+++ umac.c  9 Sep 2011 00:49:18 -
@@ -65,9 +65,11 @@
 
 #include sys/types.h
 #include sys/endian.h
+#include stdarg.h
 
 #include xmalloc.h
 #include umac.h
+#include log.h
 #include string.h
 #include stdlib.h
 #include stddef.h
@@ -167,12 +169,30 @@ static void STORE_UINT32_REVERSED(void *
 #define AES_BLOCK_LEN  16
 
 /* OpenSSL's AES */
-#include openssl/aes.h
-typedef AES_KEY aes_int_key[1];
-#define aes_encryption(in,out,int_key)  \
-  AES_encrypt((u_char *)(in),(u_char *)(out),(AES_KEY *)int_key)
-#define aes_key_setup(key,int_key)  \
-  AES_set_encrypt_key((u_char *)(key),UMAC_KEY_LEN*8,int_key)
+#include openssl/evp.h
+typedef EVP_CIPHER_CTX aes_int_key[1];
+
+static void
+aes_encryption(u_char *in, u_char *out, aes_int_key int_key)
+{
+   if (EVP_Cipher((EVP_CIPHER_CTX *)int_key, out, in, AES_BLOCK_LEN) == 0)
+   fatal(%s: EVP_Cipher failed, __func__);
+}
+
+static void
+aes_key_setup(u_char *key, aes_int_key int_key)
+{
+   if (EVP_CipherInit((EVP_CIPHER_CTX *)int_key, EVP_aes_128_ecb(),
+   key, NULL, 1) == 0)
+   fatal(%s: EVP_CipherInit failed, __func__);
+}
+
+static void
+aes_key_cleanup(aes_int_key int_key)
+{
+   if (EVP_CIPHER_CTX_cleanup((EVP_CIPHER_CTX *)int_key) == 0)
+   error(%s: EVP_CIPHER_CTX_cleanup failed, __func__);
+}
 
 /* The user-supplied UMAC key is stretched using AES in a counter
  * mode to supply all random bits needed by UMAC. The kdf function takes
@@ -228,6 +248,11 @@ static void pdf_init(pdf_ctx *pc, aes_in
 aes_encryption(pc-nonce, pc-cache, pc-prf_key);
 }
 

ksh wish

2011-09-01 Thread Damien Miller
Hi,

While people are excited about hacking on ksh(1) - let me add my wish:
unrestricted multibyte character binding so I can have ctrl-left_arrow
(^[[1;5D on my terminal) bound to backward-word and so forth.

Last time I checked the code for bind could only handle a couple of
characters after ^[

-d



Re: TOS option to tcpbench ala pf.conf

2011-08-19 Thread Damien Miller
On Thu, 18 Aug 2011, Christiano F. Haesbaert wrote:

 Hi,
 
 I'm tinkering with ToS-CoS (802.1p) translation in vlan(4) so I
 needed something to test, tcpbench seems to deserve a tos option.  
 
 It uses the same map_option() from pfctl with some minor tweeks.
 So it accepts decimal, hexadecimal, critical, lowdelay, af11...
 
 Option chosen was -t, couldn't find anything related in other programs.

Thanks, I like this. Could you add IPV6_TCLASS for IF_INET6 too?

-d



Re: Shouldn't call munmap(2) if mmap(2) failed in catopen(3)

2011-07-12 Thread Damien Miller
ok djm@

On Tue, 12 Jul 2011, Matthew Dempsky wrote:

 ok?
 
 Index: catopen.c
 ===
 RCS file: /home/mdempsky/anoncvs/cvs/src/lib/libc/nls/catopen.c,v
 retrieving revision 1.13
 diff -U5 -p -r1.13 catopen.c
 --- catopen.c 26 Jun 2008 05:42:05 -  1.13
 +++ catopen.c 12 Jul 2011 21:05:47 -
 @@ -131,11 +131,10 @@ load_msgcat(const char *path)
  
   data = mmap(0, (size_t) st.st_size, PROT_READ, MAP_SHARED, fd, 
 (off_t)0);
   close (fd);
  
   if (data == MAP_FAILED) {
 - munmap(data, (size_t) st.st_size);
   return (nl_catd) -1;
   }
  
   if (ntohl(((struct _nls_cat_hdr *) data)-__magic) != _NLS_MAGIC) {
   munmap(data, (size_t) st.st_size);



Re: sysctl.conf example for tcp.always_keepalive

2011-07-11 Thread Damien Miller
On Mon, 11 Jul 2011, Ted Unangst wrote:

 On Mon, Jul 11, 2011, Stuart Henderson wrote:
  Trying to work out a good way to describe always_keepalive in
  a short enough space for a sysctl.conf comment, this is the best
  I've come up with. Can anyone do better? OK?
 
 I think it'd be a big help to explain that these are half-seconds.  I
 think a comment of Keepalive after 600 half-seconds = 5 minutes would be
 most informative.

... but verbose. How about (1/2 second increments) or something that
might fit on the line?

-d



Re: malloc: rework MALLOC_MAXSHIFT

2011-05-17 Thread Damien Miller
On Sun, 15 May 2011, Otto Moerbeek wrote:

 Hi,
 
 define MALLOC_MAXSHIFT and related stuff more consistently. Also, zap
 region_bits, it is not used.

looks ok. some questions:

 - struct chunk_head chunk_dir[MALLOC_MAXSHIFT];
 + struct chunk_head chunk_dir[MALLOC_MAXSHIFT + 1];

Why does this grow? Isn't the MALLOC_MAXSHIFT changes above a noop as far
as the actual values are concerned?

 - d-regions_bits = 9;
 - d-regions_free = d-regions_total = 1  d-regions_bits;
 + d-regions_free = d-regions_total = 512;

Maybe make this a #define too?

 - for (i = 0; i  MALLOC_MAXSHIFT; i++)
 + for (i = 0; i = MALLOC_MAXSHIFT; i++)

Because of the array size change above?



Re: ssh, consistent use of fcntl(2) with F_SETFD

2011-05-15 Thread Damien Miller
applied - thanks

On Sat, 14 May 2011, Aaron Stellman wrote:

 Please review the diff.
 Thanks
 
 Index: usr.bin/ssh/authfd.c
 ===
 RCS file: /cvs/src/usr.bin/ssh/authfd.c,v
 retrieving revision 1.84
 diff -p -u -r1.84 authfd.c
 --- usr.bin/ssh/authfd.c  31 Aug 2010 11:54:45 -  1.84
 +++ usr.bin/ssh/authfd.c  14 May 2011 17:57:48 -
 @@ -108,7 +108,7 @@ ssh_get_authentication_socket(void)
   return -1;
  
   /* close on exec */
 - if (fcntl(sock, F_SETFD, 1) == -1) {
 + if (fcntl(sock, F_SETFD, FD_CLOEXEC) == -1) {
   close(sock);
   return -1;
   }
 Index: usr.bin/ssh/monitor.c
 ===
 RCS file: /cvs/src/usr.bin/ssh/monitor.c,v
 retrieving revision 1.110
 diff -p -u -r1.110 monitor.c
 --- usr.bin/ssh/monitor.c 9 Sep 2010 10:45:45 -   1.110
 +++ usr.bin/ssh/monitor.c 14 May 2011 17:57:48 -
 @@ -1513,7 +1513,7 @@ mm_init_compression(struct mm_master *mm
  /* XXX */
  
  #define FD_CLOSEONEXEC(x) do { \
 - if (fcntl(x, F_SETFD, 1) == -1) \
 + if (fcntl(x, F_SETFD, FD_CLOEXEC) == -1) \
   fatal(fcntl(%d, F_SETFD), x); \
  } while (0)
  
 Index: usr.bin/ssh/serverloop.c
 ===
 RCS file: /cvs/src/usr.bin/ssh/serverloop.c,v
 retrieving revision 1.159
 diff -p -u -r1.159 serverloop.c
 --- usr.bin/ssh/serverloop.c  28 May 2009 16:50:16 -  1.159
 +++ usr.bin/ssh/serverloop.c  14 May 2011 17:57:48 -
 @@ -127,8 +127,8 @@ notify_setup(void)
  {
   if (pipe(notify_pipe)  0) {
   error(pipe(notify_pipe) failed %s, strerror(errno));
 - } else if ((fcntl(notify_pipe[0], F_SETFD, 1) == -1) ||
 - (fcntl(notify_pipe[1], F_SETFD, 1) == -1)) {
 + } else if ((fcntl(notify_pipe[0], F_SETFD, FD_CLOEXEC) == -1) ||
 + (fcntl(notify_pipe[1], F_SETFD, FD_CLOEXEC) == -1)) {
   error(fcntl(notify_pipe, F_SETFD) failed %s, strerror(errno));
   close(notify_pipe[0]);
   close(notify_pipe[1]);



Re: Bus Pirate: bus hacking tool for hardware developers

2011-05-14 Thread Damien Miller
On Thu, 12 May 2011, Jona Joachim wrote:

 Hi,
 I just wanted to share this board that I discovered today:
 http://dangerousprototypes.com/bus-pirate-manual/
 
 It's an uftdi(4) board that gives you access to the following bus
 protocols:
 1-Wire, I2C, SPI, JTAG, RS-232, MIDI, ...
 http://dangerousprototypes.com/docs/Features_overview

I have had one of these for a while and used it to test an SPI D/A.

It works very nicely under OpenBSD.

-d



  1   2   >