Re: [patch] netcat: support --crlf

2023-08-23 Thread Damien Miller
On Wed, 23 Aug 2023, Pietro Cerutti wrote:

> Hi,
> 
> here at FreeBSD, we vendor in your netcat with a few local modifications.
> 
> I'm working on adding support to --crlf. I have a diff against the FreeBSD
> version here: https://reviews.freebsd.org/D41489
> 
> I'd like this to be upstreamed. If there's consensus, I'll prepare a patch
> against OpenBSD's version and send it over.

What is the motivation for this option beyond "Linux has it"? Correct
me if I'm wrong but it seems trivial to do this conversion without
writing more code by sticking tr in a pipe with nc.

OpenBSD's nc doesn't use getopt_long at present and I'm not sure there
would be appetite to do it for a single new flag. I note that nc on the
Debian machine I have at hand does -C but doesn't recognise --crlf.
IMO the long option therefore just adds incompatibility.

[djm@dvm ~]$ nc --crlf
nc: invalid option -- '-'
usage: nc [-46CDdFhklNnrStUuvZz] [-I length] [-i interval] [-M ttl]
 [-m minttl] [-O length] [-P proxy_username] [-p source_port]
 [-q seconds] [-s sourceaddr] [-T keyword] [-V rtable] [-W recvlimit]
 [-w timeout] [-X proxy_protocol] [-x proxy_address[:port]]
 [destination] [port]
[djm@dvm ~]$ uname -a
Linux dvm 6.1.0-10-amd64 #1 SMP PREEMPT_DYNAMIC Debian 6.1.38-1 (2023-07-14) 
x86_64 GNU/Linux


-d



Re: sshd: reduce preauth log verbosity

2023-08-20 Thread Damien Miller



On Fri, 18 Aug 2023, Tobias Heider wrote:

> Hi,
> 
> I was looking at my authlog today and as expected on a server exposed on the
> public internet it is filled with random scanners and brute force attacks.
> One thing I noticed is that there is a lot of information we log multiple
> times for a each failed connection.
> 
> Some examples below:
> 
> sshd[6216]: error: kex_exchange_identification: banner line contains invalid 
> characters
> sshd[6216]: banner exchange: Connection from xx.97.73.149 port 64744: invalid 
> format 
> sshd[68416]: error: kex_exchange_identification: banner line contains invalid 
> characters
> sshd[68416]: banner exchange: Connection from xx.97.73.149 port 63955: 
> invalid format 
> 
> There are a few more parsing errors like this that result in a print of the 
> exact
> issue error followed by 'goto invalid' which causes the more general "invalid 
> format"
> message. I think "invalid format" is enough information in most cases.
> 
> sshd[50752]: error: kex_exchange_identification: Connection closed by remote 
> host 
> sshd[50752]: Connection closed by xx.94.81.243 port 61000
> 
> Same as above, the kex_exchange_identification doesn't really add anything.
> 
> sshd[51579]: Invalid user tom from xx.134.191.142 port 35480
> sshd[51579]: Received disconnect from xx.134.191.142 port 35480:11: Bye Bye 
> [preauth]
> sshd[51579]: Disconnected from invalid user tom xx.134.191.142 port 35480 
> [preauth]
> sshd[94857]: Invalid user long from xx.97.173.1 port 51140
> sshd[94857]: Received disconnect from xx.97.173.1 port 51140:11: Bye Bye 
> [preauth]
> sshd[94857]: Disconnected from invalid user long xx.97.173.1 port 51140 
> [preauth]
> 
> Here the "Disconnected" line contains all the info from "Invalid user" line.
> Those invalid user messages make up the largest part of my log file,
> so deduplicating them makes a huge difference.
> 
> Below is a diff to make some of those log to debug if the same information
> is also logged elsewhere.
> Is there some general interest in diffs to clean this up a bit?

ok to the kex.c ones, but no to the auth.c one. There are other exit
paths that sshd can take that might lose the "Invalid user" message and
then that information would be lost.

> Index: auth.c
> ===
> RCS file: /cvs/src/usr.bin/ssh/auth.c,v
> retrieving revision 1.160
> diff -u -p -r1.160 auth.c
> --- auth.c5 Mar 2023 05:34:09 -   1.160
> +++ auth.c18 Aug 2023 14:22:55 -
> @@ -431,7 +431,7 @@ getpwnamallow(struct ssh *ssh, const cha
>  
>   pw = getpwnam(user);
>   if (pw == NULL) {
> - logit("Invalid user %.100s from %.100s port %d",
> + debug("Invalid user %.100s from %.100s port %d",
>   user, ssh_remote_ipaddr(ssh), ssh_remote_port(ssh));
>   return (NULL);
>   }
> Index: kex.c
> ===
> RCS file: /cvs/src/usr.bin/ssh/kex.c,v
> retrieving revision 1.179
> diff -u -p -r1.179 kex.c
> --- kex.c 18 Aug 2023 01:37:41 -  1.179
> +++ kex.c 18 Aug 2023 14:22:55 -
> @@ -1336,7 +1336,7 @@ kex_exchange_identification(struct ssh *
>   len = atomicio(read, ssh_packet_get_connection_in(ssh),
>   , 1);
>   if (len != 1 && errno == EPIPE) {
> - error_f("Connection closed by remote host");
> + debug_f("Connection closed by remote host");
>   r = SSH_ERR_CONN_CLOSED;
>   goto out;
>   } else if (len != 1) {
> @@ -1352,7 +1352,7 @@ kex_exchange_identification(struct ssh *
>   if (c == '\n')
>   break;
>   if (c == '\0' || expect_nl) {
> - error_f("banner line contains invalid "
> + debug_f("banner line contains invalid "
>   "characters");
>   goto invalid;
>   }
> @@ -1362,7 +1362,7 @@ kex_exchange_identification(struct ssh *
>   goto out;
>   }
>   if (sshbuf_len(peer_version) > SSH_MAX_BANNER_LEN) {
> - error_f("banner line too long");
> + debug_f("banner line too long");
>   goto invalid;
>   }
>   }
> @@ -1378,7 +1378,7 @@ kex_exchange_identification(struct ssh *
>   }
>   /* Do not accept lines before the SSH ident from a client */
>   if (ssh->kex->server) {
> - error_f("client sent invalid protocol identifier "
> + debug_f("client sent invalid protocol identifier "
>   "\"%.256s\"", 

Re: ssh nits

2023-03-08 Thread Damien Miller



On Thu, 9 Mar 2023, Darren Tucker wrote:

> On Thu, 9 Mar 2023 at 02:09, joshua stein  wrote:
> > cppcheck found these, are they worth fixing?
> >
> > In the non-fail case, done is set to NULL and then free()d.
> > free(NULL) is legal but maybe worth removing?
> 
> ssh uses this pattern a lot, and I agree with millert that it's not
> worth changing.
> 
> char *thing = NULL;
> [lots of stuff that might set thing]
> if (maybe()) {
> free(thing);
> thing = something;
> }
> [more stuff]
> free(thing)
> 
> We actually went through and changed all the "if (thing) free(thing)"
> instances to drop the "if" some time ago.
> 
> > grp == NULL fatal()s, so remove the ternary operations that will
> > never be the conditionals they aspire to be.
> 
> That's true in OpenBSD, but not in -portable where the check is a
> debug only.  That said, there's already a diff in this code block
> that'll stop future syncs from applying cleanly so I don't mind either
> way.
> 
> > +   if ((r = encode_dest_constraint_hop(b, >from)) != 0 ||
> > +   (r = encode_dest_constraint_hop(b, >to)) != 0 ||
> 
> I'll wait for djm to comment on this one.
> 
> > +   if ((negate = (attrib[0] == '!')))
> 
> This seems to be one too many parens?   ie
> if (negate = (attrib[0] == '!'))
> 
> > -   if ((r = parse_dest_constraint_hop(frombuf, >from) != 0) ||
> > -   (r = parse_dest_constraint_hop(tobuf, >to) != 0))
> > +   if ((r = parse_dest_constraint_hop(frombuf, >from)) != 0 ||
> > +   (r = parse_dest_constraint_hop(tobuf, >to)) != 0)
> 
> also djm.

ok djm for these



Re: ssh-pkcs11.c: fix some error messages

2023-03-07 Thread Damien Miller



On Tue, 7 Mar 2023, Theo Buehler wrote:

> Some error messages don't match the function calls. There's still this
> one that looks weird. Not sure what it should say to be helpful:
> "unexpected ec signature length" perhaps?
> 
>   if (siglen < 64 || siglen > 132 || siglen % 2) {
>   ossl_error("d2i_ECDSA_SIG failed");

Yeah, that should be:

error_f("bad signature length %lu", (u_long)siglen);

or similar.

ok for that and the others

> Index: ssh-pkcs11.c
> ===
> RCS file: /cvs/src/usr.bin/ssh/ssh-pkcs11.c,v
> retrieving revision 1.55
> diff -u -p -r1.55 ssh-pkcs11.c
> --- ssh-pkcs11.c  18 Nov 2021 21:11:01 -  1.55
> +++ ssh-pkcs11.c  4 Mar 2023 08:21:59 -
> @@ -513,7 +513,7 @@ ecdsa_do_sign(const unsigned char *dgst,
>   BIGNUM  *r = NULL, *s = NULL;
>  
>   if ((k11 = EC_KEY_get_ex_data(ec, ec_key_idx)) == NULL) {
> - ossl_error("EC_KEY_get_key_method_data failed for ec");
> + ossl_error("EC_KEY_get_ex_data failed for ec");
>   return (NULL);
>   }
>  
> @@ -545,7 +545,7 @@ ecdsa_do_sign(const unsigned char *dgst,
>   }
>   if ((r = BN_bin2bn(sig, bnlen, NULL)) == NULL ||
>   (s = BN_bin2bn(sig+bnlen, bnlen, NULL)) == NULL) {
> - ossl_error("d2i_ECDSA_SIG failed");
> + ossl_error("BN_bin2bn failed");
>   ECDSA_SIG_free(ret);
>   ret = NULL;
>   goto done;
> 
> 



KRL 4/4: regression test for signing/verification

2023-01-16 Thread Damien Miller
Hi,

The final OpenSSH key revocation list (KRL) diff for now :)

This extends the existing krl.sh regression test to exercise signing and
verification. (This depends on the last two diffs)

ok?

Index: krl.sh
===
RCS file: /cvs/src/regress/usr.bin/ssh/krl.sh,v
retrieving revision 1.12
diff -u -p -r1.12 krl.sh
--- krl.sh  16 Jan 2023 04:11:29 -  1.12
+++ krl.sh  16 Jan 2023 08:00:35 -
@@ -1,4 +1,4 @@
-#  $OpenBSD: krl.sh,v 1.12 2023/01/16 04:11:29 djm Exp $
+#  $OpenBSD: krl.sh,v 1.11 2019/12/16 02:39:05 djm Exp $
 #  Placed in the Public Domain.
 
 tid="key revocation lists"
@@ -22,7 +22,16 @@ done
 # Old keys will interfere with ssh-keygen.
 rm -f $OBJ/revoked-* $OBJ/krl-*
 
-# Generate a CA key
+# Generate some KRL signing keys
+$SSHKEYGEN -t ed25519 -f $OBJ/krl-sign  -C "" -N "" > /dev/null ||
+   fatal "$SSHKEYGEN signing key failed"
+$SSHKEYGEN -t ed25519 -f $OBJ/krl-sign-wrong  -C "" -N "" > /dev/null ||
+   fatal "$SSHKEYGEN signing key-wrong failed"
+$SSHKEYGEN -t ed25519 -f $OBJ/krl-sign2 -C "" -N "" > /dev/null ||
+   fatal "$SSHKEYGEN signing key2 failed"
+$SSHKEYGEN -t ed25519 -f $OBJ/krl-sign3 -C "" -N "" > /dev/null ||
+   fatal "$SSHKEYGEN signing key3 failed"
+# Generate some CA keys
 $SSHKEYGEN -t $ktype1 -f $OBJ/revoked-ca  -C "" -N "" > /dev/null ||
fatal "$SSHKEYGEN CA failed"
 $SSHKEYGEN -t $ktype2 -f $OBJ/revoked-ca2  -C "" -N "" > /dev/null ||
@@ -108,7 +117,14 @@ for rkey in $RKEYS; do
 done
 
 genkrls() {
-   OPTS=$1
+   #OPTS="-vvv $@"
+   OPTS="$@"
+
+$SSHKEYGEN $OPTS -kf $OBJ/krl-revoked-signing $OBJ/krl-sign2.pub \
+   >/dev/null || fatal "$SSHKEYGEN KRL failed"
+$SSHKEYGEN $OPTS -kf $OBJ/krl-revoked-signing2 \
+$OBJ/krl-sign2.pub $OBJ/krl-sign3.pub \
+   >/dev/null || fatal "$SSHKEYGEN KRL failed"
 $SSHKEYGEN $OPTS -kf $OBJ/krl-empty - /dev/null || fatal "$SSHKEYGEN KRL failed"
 $SSHKEYGEN $OPTS -kf $OBJ/krl-keys $RKEYS \
@@ -136,9 +152,9 @@ $SSHKEYGEN $OPTS -kf $OBJ/krl-serial -s 
 $SSHKEYGEN $OPTS -kf $OBJ/krl-keyid -s $OBJ/revoked-ca.pub \
$OBJ/revoked-keyid >/dev/null || fatal "$SSHKEYGEN KRL failed"
 # These should succeed; they specify an wildcard CA key.
-$SSHKEYGEN $OPTS -kf $OBJ/krl-serial-wild -s NONE $OBJ/revoked-serials \
+$SSHKEYGEN $OPTS -kf $OBJ/krl-srl-wild -s NONE $OBJ/revoked-serials \
>/dev/null || fatal "$SSHKEYGEN KRL failed"
-$SSHKEYGEN $OPTS -kf $OBJ/krl-keyid-wild -s NONE $OBJ/revoked-keyid \
+$SSHKEYGEN $OPTS -kf $OBJ/krl-id-wild -s NONE $OBJ/revoked-keyid \
>/dev/null || fatal "$SSHKEYGEN KRL failed"
 # Revoke the same serials with the second CA key to ensure a multi-CA
 # KRL is generated.
@@ -149,16 +165,18 @@ $SSHKEYGEN $OPTS -kf $OBJ/krl-serial -u 
 ## XXX dump with trace and grep for set cert serials
 ## XXX test ranges near (u64)-1, etc.
 
-verbose "$tid: generating KRLs"
-genkrls
-
 check_krl() {
KEY=$1
KRL=$2
EXPECT_REVOKED=$3
TAG=$4
-   $SSHKEYGEN -Qf $KRL $KEY >/dev/null
+   ARG=$5
+   $SSHKEYGEN $ARG -Qf $KRL $KEY >/dev/null 2>&1
result=$?
+   case "x$EXPECT_REVOKED" in
+   xx|xy) ;;
+   default) fatal "bad expectation $EXPECT_REVOKED"
+   esac
if test "x$EXPECT_REVOKED" = "xy" -a $result -eq 0 ; then
fatal "key $KEY not revoked by KRL $KRL: $TAG"
elif test "x$EXPECT_REVOKED" = "xn" -a $result -ne 0 ; then
@@ -177,41 +195,107 @@ test_rev() {
CA_RESULT=$9
SERIAL_WRESULT=${10}
KEYID_WRESULT=${11}
+   ARG=${12}
verbose "$tid: checking revocations for $TAG"
for f in $FILES ; do
-   check_krl $f $OBJ/krl-empty no  "$TAG"
-   check_krl $f $OBJ/krl-keys  $KEYS_RESULT"$TAG"
-   check_krl $f $OBJ/krl-all   $ALL_RESULT "$TAG"
-   check_krl $f $OBJ/krl-sha1  $HASH_RESULT"$TAG"
-   check_krl $f $OBJ/krl-sha256$HASH_RESULT"$TAG"
-   check_krl $f $OBJ/krl-hash  $HASH_RESULT"$TAG"
-   check_krl $f $OBJ/krl-serial$SERIAL_RESULT  "$TAG"
-   check_krl $f $OBJ/krl-keyid $KEYID_RESULT   "$TAG"
-   check_krl $f $OBJ/krl-cert  $CERTS_RESULT   "$TAG"
-   check_krl $f $OBJ/krl-ca$CA_RESULT  "$TAG"
-   check_krl $f $OBJ/krl-serial-wild   $SERIAL_WRESULT "$TAG"
-   check_krl $f $OBJ/krl-keyid-wild$KEYID_WRESULT  "$TAG"
+   check_krl $f $OBJ/krl-empty no  "$TAG" "$ARG"
+   check_krl $f $OBJ/krl-keys  $KEYS_RESULT"$TAG" "$ARG"
+   check_krl $f $OBJ/krl-all   $ALL_RESULT "$TAG" "$ARG"
+   check_krl $f $OBJ/krl-sha1  $HASH_RESULT"$TAG" "$ARG"
+   

KRL 3/4: plumb in signing and verification to ssh-keygen

2023-01-16 Thread Damien Miller
Hi,

This is another OpenSSH key revocation list (KRL) change: to support KRL
signing and verification in ssh-keygen(1).

The KRL format has supported signing of KRLs and verification of KRL
signatures for a long time, but there is currently no way to generate a
signed KRL or check the signature on one. The final patch hooks this up
for ssh-keygen(1) at least.

ok?

---
 ssh-keygen.c | 62 ++--
 1 file changed, 51 insertions(+), 11 deletions(-)

diff --git a/ssh-keygen.c b/ssh-keygen.c
index 869b675..29b8aec 100644
--- a/ssh-keygen.c
+++ b/ssh-keygen.c
@@ -2179,7 +2179,8 @@ do_show_cert(struct passwd *pw)
 }
 
 static void
-load_krl(const char *path, struct ssh_krl **krlp)
+load_krl(const char *path, struct ssh_krl **krlp,
+struct sshkey **trusted_keys, size_t ntrusted_keys)
 {
struct sshbuf *krlbuf;
int r;
@@ -2187,7 +2188,8 @@ load_krl(const char *path, struct ssh_krl **krlp)
if ((r = sshbuf_load_file(path, )) != 0)
fatal_r(r, "Unable to load KRL %s", path);
/* XXX check sigs */
-   if ((r = ssh_krl_from_blob(krlbuf, krlp, NULL, 0)) != 0 ||
+   if ((r = ssh_krl_from_blob(krlbuf, krlp,
+   trusted_keys, ntrusted_keys)) != 0 ||
*krlp == NULL)
fatal_r(r, "Invalid KRL file %s", path);
sshbuf_free(krlbuf);
@@ -2378,20 +2380,54 @@ update_krl_from_file(struct passwd *pw, const char 
*file, int wild_ca,
free(path);
 }
 
+static void
+krl_process_opts(struct passwd *pw, char * const *opts, size_t nopts,
+struct sshkey ***sig_keys, size_t *nsig_keys)
+{
+   struct sshkey *key;
+   char *path;
+   size_t i;
+
+   for (i = 0; i < nopts; i++) {
+   if (strncasecmp(opts[i], "signing-key=", 12) == 0) {
+   path = tilde_expand_filename(opts[i] + 12, pw->pw_uid);
+   key = load_identity(path, NULL);
+   free(path);
+   *sig_keys = xrecallocarray(*sig_keys, *nsig_keys,
+   *nsig_keys + 1, sizeof(*sig_keys));
+   (*sig_keys)[(*nsig_keys)++] = key;
+   } else
+   fatal("Invalid option \"%s\"", opts[i]);
+   }
+}
+
+static void
+free_sig_keys(struct sshkey **sig_keys, size_t nsig_keys)
+{
+   size_t i;
+
+   for (i = 0; i < nsig_keys; i++)
+   sshkey_free(sig_keys[i]);
+   free(sig_keys);
+}
+
 static void
 do_gen_krl(struct passwd *pw, int updating, const char *ca_key_path,
 unsigned long long krl_version, const char *krl_comment,
+char * const *opts, size_t nopts,
 int argc, char **argv)
 {
struct ssh_krl *krl;
struct stat sb;
-   struct sshkey *ca = NULL;
+   struct sshkey *ca = NULL, **sig_keys = NULL;
int i, r, wild_ca = 0;
+   size_t nsig_keys = 0;
char *tmp;
struct sshbuf *kbuf;
 
if (*identity_file == '\0')
fatal("KRL generation requires an output file");
+   krl_process_opts(pw, opts, nopts, _keys, _keys);
if (stat(identity_file, ) == -1) {
if (errno != ENOENT)
fatal("Cannot access KRL \"%s\": %s",
@@ -2411,7 +2447,7 @@ do_gen_krl(struct passwd *pw, int updating, const char 
*ca_key_path,
}
 
if (updating)
-   load_krl(identity_file, );
+   load_krl(identity_file, , NULL, 0);
else if ((krl = ssh_krl_init()) == NULL)
fatal("couldn't create KRL");
 
@@ -2425,26 +2461,30 @@ do_gen_krl(struct passwd *pw, int updating, const char 
*ca_key_path,
 
if ((kbuf = sshbuf_new()) == NULL)
fatal("sshbuf_new failed");
-   if (ssh_krl_to_blob(krl, kbuf, NULL, 0) != 0)
+   if (ssh_krl_to_blob(krl, kbuf, sig_keys, nsig_keys) != 0)
fatal("Couldn't generate KRL");
if ((r = sshbuf_write_file(identity_file, kbuf)) != 0)
fatal("write %s: %s", identity_file, strerror(errno));
sshbuf_free(kbuf);
ssh_krl_free(krl);
sshkey_free(ca);
+   free_sig_keys(sig_keys, nsig_keys);
 }
 
 static void
-do_check_krl(struct passwd *pw, int print_krl, int argc, char **argv)
+do_check_krl(struct passwd *pw, int print_krl,
+char * const *opts, size_t nopts, int argc, char **argv)
 {
int i, r, ret = 0;
char *comment;
struct ssh_krl *krl;
-   struct sshkey *k;
+   struct sshkey *k = NULL, **sig_keys = NULL;
+   size_t nsig_keys = 0;
 
if (*identity_file == '\0')
fatal("KRL checking requires an input file");
-   load_krl(identity_file, );
+   krl_process_opts(pw, opts, nopts, _keys, _keys);
+   load_krl(identity_file, , sig_keys, nsig_keys);
if (print_krl)
krl_dump(krl, stdout);
for (i = 0; i < argc; i++) {
@@ -2460,6 +2500,7 @@ do_check_krl(struct passwd *pw, int print_krl, int argc, 

KRL 2/4: Refactor parsing and signature verification

2023-01-16 Thread Damien Miller
Hi,

This is the second of the OpenSSH key revocation list (KRL) diffs.
This one refactors KRL parsing, and particularly signature verification.

It splits the KRL parsing logic into three phases: signature
verification, key trust verification and everything else. The idea is
to make this easier to read and verify, but also to allow reuse of the
first two steps in other contexts (e.g. a KRL parser that just tests a
single key, instead of trying to load the whole KRL into RAM).

Also at https://github.com/djmdjm/openssh-wip/pull/19

ok?

(this is a bit of a mess to read as a diff; easier to look at before
and after files or use the github view)

---
 krl.c | 270 +++---
 krl.h |   2 +-
 2 files changed, 165 insertions(+), 107 deletions(-)

diff --git a/krl.c b/krl.c
index a8a6018..e5ef0bf 100644
--- a/krl.c
+++ b/krl.c
@@ -1054,87 +1054,81 @@ extension_section(struct sshbuf *sect, struct ssh_krl 
*krl)
return r;
 }
 
-/* Attempt to parse a KRL, checking its signature (if any) with sign_ca_keys. 
*/
-int
-ssh_krl_from_blob(struct sshbuf *buf, struct ssh_krl **krlp,
-const struct sshkey **sign_ca_keys, size_t nsign_ca_keys)
+/*
+ * Checks whether this is a KRL (correct magic) with supported version and
+ * verified signatures if present. NB. does not check whether keys are trusted
+ * to sign the KRL, caller must do by checking contents of returned key list.
+ * Does not modify buffer.
+ */
+static int
+check_krl_signature_header(struct sshbuf *buf,
+struct sshkey ***signing_keysp, size_t *nsigning_keysp)
 {
-   struct sshbuf *copy = NULL, *sect = NULL;
-   struct ssh_krl *krl = NULL;
-   char timestamp[64];
-   int r = SSH_ERR_INTERNAL_ERROR, sig_seen;
-   struct sshkey *key = NULL, **ca_used = NULL, **tmp_ca_used;
-   u_char type;
-   const u_char *blob;
-   size_t i, j, sig_off, sects_off, blen, nca_used;
+   int r = SSH_ERR_INTERNAL_ERROR;
+   struct sshbuf *copy = NULL;
u_int format_version;
+   u_char type;
+   const u_char *blob;
+   struct sshkey *key = NULL, **tmp, **sig_keys = NULL;
+   size_t i, sig_off, blen, nsig_keys = 0;
 
-   nca_used = 0;
-   *krlp = NULL;
-   if (sshbuf_len(buf) < sizeof(KRL_MAGIC) - 1 ||
-   memcmp(sshbuf_ptr(buf), KRL_MAGIC, sizeof(KRL_MAGIC) - 1) != 0) {
-   debug3_f("not a KRL");
-   return SSH_ERR_KRL_BAD_MAGIC;
+   if (signing_keysp == NULL || nsigning_keysp == NULL)
+   return SSH_ERR_INTERNAL_ERROR;
+   *signing_keysp = NULL;
+   *nsigning_keysp = 0;
+
+   /* KRL must begin with magic string */
+   if ((r = sshbuf_cmp(buf, 0, KRL_MAGIC, sizeof(KRL_MAGIC) - 1)) != 0) {
+   debug2_f("bad KRL magic header");
+   goto out;
}
-
-   /* Take a copy of the KRL buffer so we can verify its signature later */
+   /* Don't modify original buffer */
if ((copy = sshbuf_fromb(buf)) == NULL) {
-   r = SSH_ERR_ALLOC_FAIL;
+   error_f("sshbuf_fromb failed");
goto out;
}
-   if ((r = sshbuf_consume(copy, sizeof(KRL_MAGIC) - 1)) != 0)
-   goto out;
-
-   if ((krl = ssh_krl_init()) == NULL) {
-   error_f("alloc failed");
-   goto out;
-   }
-
-   if ((r = sshbuf_get_u32(copy, _version)) != 0)
+   if ((r = sshbuf_consume(copy, sizeof(KRL_MAGIC) - 1)) != 0 ||
+   (r = sshbuf_get_u32(copy, _version)) != 0)
goto out;
if (format_version != KRL_FORMAT_VERSION) {
+   error_f("unsupported KRL format version %u", format_version);
r = SSH_ERR_INVALID_FORMAT;
goto out;
}
-   if ((r = sshbuf_get_u64(copy, >krl_version)) != 0 ||
-   (r = sshbuf_get_u64(copy, >generated_date)) != 0 ||
-   (r = sshbuf_get_u64(copy, >flags)) != 0 ||
+   /* Skip header contents */
+   if ((r = sshbuf_get_u64(copy, NULL)) != 0 ||
+   (r = sshbuf_get_u64(copy, NULL)) != 0 ||
+   (r = sshbuf_get_u64(copy, NULL)) != 0 ||
(r = sshbuf_skip_string(copy)) != 0 ||
-   (r = sshbuf_get_cstring(copy, >comment, NULL)) != 0)
+   (r = sshbuf_get_cstring(copy, NULL, NULL)) != 0) {
+   error_fr(r, "parse KRL header");
goto out;
-
-   format_timestamp(krl->generated_date, timestamp, sizeof(timestamp));
-   debug("KRL version %llu generated at %s%s%s",
-   (long long unsigned)krl->krl_version, timestamp,
-   *krl->comment ? ": " : "", krl->comment);
-
+   }
/*
-* 1st pass: verify signatures, if any. This is done to avoid
-* detailed parsing of data whose provenance is unverified.
+* Perform minimal parsing (section type/len only) to reach signature
+* sections, if present.
 */
-   sig_seen = 0;
if (sshbuf_len(buf) < 

KRL 1/4: extension mechanism

2023-01-15 Thread Damien Miller
Hi,

This is the first of a few changes to krl.c and related code.

This defines and implements an extension mechanism for KRLs.

This takes the form of new (sub-)section types that contain named
extensions. These may be flagged as "critical" which causes the KRL
parser to treat them as mandatory-to implement. If they aren't flagged
as critical then they are ignored.

I honestly feel kind of stupid for not doing this when I wrote the
format...

Unfortunately KRLs with extensions are not backwards-compatible, as
the parser treats unknown section types as a fatal error. They are
forwards-compatible though.

I didn't update the KRL format version because I couldn't see any
practical difference between "KRL parsing fails because of an unknown
section type" and "KRL parsing fails because the format version is
wrong", except that the latter also makes life harder for the no-
extension case.

This doesn't add support for any extensions, just the mechanism itself.

ok?

PS. also at https://github.com/djmdjm/openssh-wip/pull/19

 PROTOCOL.krl | 49 +-
 krl.c| 84 
 krl.h|  2 ++
 3 files changed, 134 insertions(+), 1 deletion(-)

diff --git a/PROTOCOL.krl b/PROTOCOL.krl
index 115f80e..0bd0a82 100644
--- a/PROTOCOL.krl
+++ b/PROTOCOL.krl
@@ -37,6 +37,7 @@ The available section types are:
 #define KRL_SECTION_FINGERPRINT_SHA1   3
 #define KRL_SECTION_SIGNATURE  4
 #define KRL_SECTION_FINGERPRINT_SHA256 5
+#define KRL_SECTION_EXTENSION  255
 
 2. Certificate section
 
@@ -64,6 +65,7 @@ The certificate section types are:
 #define KRL_SECTION_CERT_SERIAL_RANGE  0x21
 #define KRL_SECTION_CERT_SERIAL_BITMAP 0x22
 #define KRL_SECTION_CERT_KEY_ID0x23
+#define KRL_SECTION_CERT_EXTENSION 0x39
 
 2.1 Certificate serial list section
 
@@ -114,6 +116,29 @@ associated with a particular identity, e.g. a host or a 
user.
 This section must contain at least one "key_id". This section may appear
 multiple times.
 
+2.5. Certificate Extension subsections
+
+This subsection type provides a generic extension mechanism to the
+certificates KRL section that may be used to provide optional or critical
+data.
+
+Extensions are stored in subsections of type
+KRL_SECTION_CERT_EXTENSION with the following contents:
+
+   string  extension_name
+   boolean is_critical
+   string  extension_contents.
+
+Where "extension_name" describes the type of extension. It is
+recommended that user extensions follow "cert-n...@domain.org" naming.
+
+The "is_critical" indicates whether this extension is mandatory or
+optional. If true, then any unsupported extension encountered should
+result in KRL parsing failure. If false, then it may be safely be
+ignored.
+
+The "extension_contents" contains the body of the extension.
+
 3. Explicit key sections
 
 These sections, identified as KRL_SECTION_EXPLICIT_KEY, revoke keys
@@ -144,7 +169,29 @@ as a big-endian integer.
 
 This section may appear multiple times.
 
-5. KRL signature sections
+5. Extension sections
+
+This section type provides a generic extension mechanism to the KRL
+format that may be used to provide optional or critical data.
+
+Extensions are recorded in sections of type KRL_SECTION_EXTENSION
+with the following contents:
+
+   string  extension_name
+   boolean is_critical
+   string  extension_contents.
+
+Where "extension_name" describes the type of extension. It is
+recommended that user extensions follow "n...@domain.org" naming.
+
+The "is_critical" indicates whether this extension is mandatory or
+optional. If true, then any unsupported extension encountered should
+result in KRL parsing failure. If false, then it may be safely be
+ignored.
+
+The "extension_contents" contains the body of the extension.
+
+6. KRL signature sections
 
 The KRL_SECTION_SIGNATURE section serves a different purpose to the
 preceding ones: to provide cryptographic authentication of a KRL that
diff --git a/krl.c b/krl.c
index f491c24..a8a6018 100644
--- a/krl.c
+++ b/krl.c
@@ -837,6 +837,45 @@ format_timestamp(u_int64_t timestamp, char *ts, size_t nts)
}
 }
 
+static int
+cert_extension_subsection(struct sshbuf *subsect, struct ssh_krl *krl)
+{
+   int r = SSH_ERR_INTERNAL_ERROR;
+   u_char critical = 1;
+   struct sshbuf *value = NULL;
+   char *name = NULL;
+
+   if ((r = sshbuf_get_cstring(subsect, , NULL)) != 0 ||
+   (r = sshbuf_get_u8(subsect, )) != 0 ||
+   (r = sshbuf_froms(subsect, )) != 0) {
+   debug_fr(r, "parse");
+   error("KRL has invalid certificate extension subsection");
+   r = SSH_ERR_INVALID_FORMAT;
+   goto out;
+   }
+   if (sshbuf_len(subsect) != 0) {
+   error("KRL has invalid certificate extension subsection: "
+   "trailing data");
+   r = SSH_ERR_INVALID_FORMAT;

Re: openssh: update ed25519 and squash into a single file

2023-01-13 Thread Damien Miller



On Fri, 13 Jan 2023, Damien Miller wrote:

> Hi,
> 
> Forewarning: this is a big, noisy diff. Also on Github at
> https://github.com/djmdjm/openssh-wip/pull/18
> 
> This updates the ED25519 code to the latest version of SUPERCOP (20221122),
> but the real motivation for this is to move the ED25519 code to the same
> approach we use for the Streamlined NTRUPrime code: using a shell-script
> to extract the bits we want from SUPERCOP and squish them all into a
> single file.
> 
> This removes a bunch of exported function names, a bit of unused
> code and means that all the ED25519 code is in a single file rather
> than eight.
> 
> To review this, it's probably best to run the shellscript locally
> (use sh ed25519.sh /path/to/directory/with/supercop) and inspect the
> output. Apart from the original ed25519.c (assembled from the keypair.c,
> sign.c and open.c files in SUPERCOP) there are no substantial changes.

Here's a better way to look at the substantive changes:

1. Assemble the existing ed25519 code in the same order as how this
   patch arranges things:

cat verify.c fe25519.h fe25519.c sc25519.h sc25519.c \
ge25519.h ge25519.c ed25519.c | \
sed -e '/#include "ge25519_base.data"/r ge25519_base.data' \
-e '/#include.*/d'  > ed25519.c.old

2. Apply the patch

3. Diff the original and new code (below)

This isn't completely without noise, but it lets you see the substantive
changes clearly.

-d




--- /tmp/ed25519.c  Sat Jan 14 16:25:09 2023
+++ ed25519.c   Sat Jan 14 16:25:41 2023
@@ -1,12 +1,30 @@
-/* $OpenBSD: verify.c,v 1.3 2013/12/09 11:03:45 markus Exp $ */
+/*  $OpenBSD: $ */
 
 /*
- * Public Domain, Author: Daniel J. Bernstein
- * Copied from nacl-20110221/crypto_verify/32/ref/verify.c
+ * Public Domain, Authors:
+ * - Daniel J. Bernstein
+ * - Niels Duif
+ * - Tanja Lange
+ * - lead: Peter Schwabe
+ * - Bo-Yin Yang
  */
 
+#include 
 
-int crypto_verify_32(const unsigned char *x,const unsigned char *y)
+#include "crypto_api.h"
+
+#define int8 crypto_int8
+#define uint8 crypto_uint8
+#define int16 crypto_int16
+#define uint16 crypto_uint16
+#define int32 crypto_int32
+#define uint32 crypto_uint32
+#define int64 crypto_int64
+#define uint64 crypto_uint64
+
+/* from supercop-20221122/crypto_verify/32/ref/verify.c */
+
+static int crypto_verify_32(const unsigned char *x,const unsigned char *y)
 {
   unsigned int differentbits = 0;
 #define F(i) differentbits |= x[i] ^ y[i];
@@ -44,14 +62,7 @@
   F(31)
   return (1 & ((differentbits - 1) >> 8)) - 1;
 }
-/* $OpenBSD: fe25519.h,v 1.3 2013/12/09 11:03:45 markus Exp $ */
-
-/*
- * Public Domain, Authors: Daniel J. Bernstein, Niels Duif, Tanja Lange,
- * Peter Schwabe, Bo-Yin Yang.
- * Copied from supercop-20130419/crypto_sign/ed25519/ref/fe25519.h
- */
-
+/* from supercop-20221122/crypto_sign/ed25519/ref/fe25519.h */
 #ifndef FE25519_H
 #define FE25519_H
 
@@ -80,52 +91,45 @@
 }
 fe25519;
 
-void fe25519_freeze(fe25519 *r);
+static void fe25519_freeze(fe25519 *r);
 
-void fe25519_unpack(fe25519 *r, const unsigned char x[32]);
+static void fe25519_unpack(fe25519 *r, const unsigned char x[32]);
 
-void fe25519_pack(unsigned char r[32], const fe25519 *x);
+static void fe25519_pack(unsigned char r[32], const fe25519 *x);
 
-int fe25519_iszero(const fe25519 *x);
+static int fe25519_iszero(const fe25519 *x);
 
-int fe25519_iseq_vartime(const fe25519 *x, const fe25519 *y);
+static int fe25519_iseq_vartime(const fe25519 *x, const fe25519 *y);
 
-void fe25519_cmov(fe25519 *r, const fe25519 *x, unsigned char b);
+static void fe25519_cmov(fe25519 *r, const fe25519 *x, unsigned char b);
 
-void fe25519_setone(fe25519 *r);
+static void fe25519_setone(fe25519 *r);
 
-void fe25519_setzero(fe25519 *r);
+static void fe25519_setzero(fe25519 *r);
 
-void fe25519_neg(fe25519 *r, const fe25519 *x);
+static void fe25519_neg(fe25519 *r, const fe25519 *x);
 
 unsigned char fe25519_getparity(const fe25519 *x);
 
-void fe25519_add(fe25519 *r, const fe25519 *x, const fe25519 *y);
+static void fe25519_add(fe25519 *r, const fe25519 *x, const fe25519 *y);
 
-void fe25519_sub(fe25519 *r, const fe25519 *x, const fe25519 *y);
+static void fe25519_sub(fe25519 *r, const fe25519 *x, const fe25519 *y);
 
-void fe25519_mul(fe25519 *r, const fe25519 *x, const fe25519 *y);
+static void fe25519_mul(fe25519 *r, const fe25519 *x, const fe25519 *y);
 
-void fe25519_square(fe25519 *r, const fe25519 *x);
+static void fe25519_square(fe25519 *r, const fe25519 *x);
 
-void fe25519_invert(fe25519 *r, const fe25519 *x);
+static void fe25519_invert(fe25519 *r, const fe25519 *x);
 
-void fe25519_pow2523(fe25519 *r, const fe25519 *x);
+static void fe25519_pow2523(fe25519 *r, const fe25519 *x);
 
 #endif
-/* $OpenBSD: fe25519.c,v 1.3 2013/12/09 11:03:45 markus Exp $ */
-
-/*
- * Public Domain, Authors: Daniel J. Bernstein, Niels Duif, Tanja Lange,
- * Peter Schwabe, Bo-Yin Yang.
- 

Re: libcbor v0.10.0

2022-12-29 Thread Damien Miller
On Fri, 30 Dec 2022, Theo Buehler wrote:

> On Fri, Dec 30, 2022 at 10:09:16AM +1100, Damien Miller wrote:
> > This updates libcbor to upstream version v.0.10.0. This version includes
> > clang15 header fixes and fixes a few memory leaks. Full release notes
> > are at https://github.com/PJK/libcbor/releases/tag/v0.10.0
> 
> I understand that it is a libcbor major bump. Why is the libfido2 bump
> needed?

Caution - some of the libcbor changes were listed as "breaking" in the
release notes. I took this to mean an ABI bump. libfido2 depends on
libcbor, so I figured that bumping it would avoid any possibility of
inconsistency between them at link time. Too much?

> The CBOR_CUSTOM_ALLOC deprecation and the fact that cbor_set_allocs() is
> now exposed is a bit disappointing.
> 
> https://github.com/PJK/libcbor/pull/237
> 
> Other than that the diff looks good to me, build tested on sparc64, so
> gcc-archs should be fine, too.

That's reasonable. I can chop those out:


Index: Makefile
===
RCS file: /cvs/src/lib/libcbor/Makefile,v
retrieving revision 1.3
diff -u -p -r1.3 Makefile
--- Makefile3 Aug 2020 02:34:31 -   1.3
+++ Makefile30 Dec 2022 01:37:31 -
@@ -3,6 +3,8 @@
 .PATH: ${.CURDIR}/src ${.CURDIR}/src/cbor ${.CURDIR}/src/cbor/internal
 
 CFLAGS+= -I${.CURDIR}/src -DHAVE_ENDIAN_H -std=c99
+# We don't support custom allocators.
+CFLAGS+= -D_cbor_malloc=malloc -D_cbor_realloc=realloc -D_cbor_free=free
 
 LIB=   cbor
 SRCS=  cbor.c

cbor/common.h now looks like this:

+#if 0 /* custom allocators are not supported on OpenBSD */
+typedef void *(*_cbor_malloc_t)(size_t);
+typedef void *(*_cbor_realloc_t)(void *, size_t);
+typedef void (*_cbor_free_t)(void *);
+
+CBOR_EXPORT extern _cbor_malloc_t _cbor_malloc;
+CBOR_EXPORT extern _cbor_realloc_t _cbor_realloc;
+CBOR_EXPORT extern _cbor_free_t _cbor_free;
+#endif

...

+#if 0 /* not on OpenBSD */
+CBOR_EXPORT void cbor_set_allocs(_cbor_malloc_t custom_malloc,
+ _cbor_realloc_t custom_realloc,
+ _cbor_free_t custom_free);
 #endif






Re: clang 15 and zlib

2022-12-28 Thread Damien Miller



On Wed, 28 Dec 2022, Todd C. Miller wrote:

> OK millert@ as well.  There is no point in trying to fix this locally
> when upstream zlib will be changing it in the near(?) future.

He committed to removing the K function declarations when C23 is
finalised:
https://github.com/madler/zlib/issues/633#issuecomment-1275165338

ETA by the end of next year:
https://github.com/madler/zlib/issues/633#issuecomment-1276016034

Otherwise, there's the https://github.com/zlib-ng/zlib-ng fork which has
a bit more appetite for change and has already removed them, but that's
obviously a bigger discussion.

-d



Re: OpenSSH and -current out-of-tree patched for ~C?

2022-11-30 Thread Damien Miller
On Wed, 30 Nov 2022, Theo de Raadt wrote:

> >> It allows a much tighter pledge in the client, so less attack surface
> >> against a bad server.
> >
> >So it's to  prevent a malicious SSH server from  exploiting a client who
> >choses  to use  ~C to  open up  the ssh>  prompt and  create or  destroy
> >tunnels?
> 
> No.
> 
> 
> It makes ssh safer for people who don't use the fancy features,
> because the ssh client cannot perform a vast number of system calls
> if it gets fooled.

Anyway it's all committed now, including additional pledge(2) restrictions
as well as documentation for the new option which ended up being called
EnableEscapeCommandline to avoid some ambiguity.

You can switch it on everywhere you need to command-line.

-d



Re: ssh-keygen(1): by default generate ed25519 key (instead of rsa)

2022-11-06 Thread Damien Miller
I think it's time; RFC 8709 has been a thing for a couple of years
now and a bit of gentle pressure is good.

ok djm, but cc openssh@ so others can chime in

-d

On Sun, 6 Nov 2022, Job Snijders wrote:

> Dear all,
> 
> Support for using Ed25519 for server and user authentication was
> introduced in 2014. I like the compactness of Ed25519 public keys.
> 
> Perhaps now is a good time to make Ed25519 the default key type when
> invoking ssh-keygen(1) without arguments?
> 
> Kind regards,
> 
> Job
> 
> Index: ssh-keygen.1
> ===
> RCS file: /cvs/src/usr.bin/ssh/ssh-keygen.1,v
> retrieving revision 1.226
> diff -u -p -r1.226 ssh-keygen.1
> --- ssh-keygen.1  10 Sep 2022 08:50:53 -  1.226
> +++ ssh-keygen.1  6 Nov 2022 13:31:19 -
> @@ -185,7 +185,7 @@ The type of key to be generated is speci
>  option.
>  If invoked without any arguments,
>  .Nm
> -will generate an RSA key.
> +will generate an ed25519 key.
>  .Pp
>  .Nm
>  is also used to generate groups for use in Diffie-Hellman group
> Index: ssh-keygen.c
> ===
> RCS file: /cvs/src/usr.bin/ssh/ssh-keygen.c,v
> retrieving revision 1.459
> diff -u -p -r1.459 ssh-keygen.c
> --- ssh-keygen.c  11 Aug 2022 01:56:51 -  1.459
> +++ ssh-keygen.c  6 Nov 2022 13:31:21 -
> @@ -61,12 +61,6 @@
>  #include "ssh-pkcs11.h"
>  #endif
>  
> -#ifdef WITH_OPENSSL
> -# define DEFAULT_KEY_TYPE_NAME "rsa"
> -#else
> -# define DEFAULT_KEY_TYPE_NAME "ed25519"
> -#endif
> -
>  /*
>   * Default number of bits in the RSA, DSA and ECDSA keys.  These value can be
>   * overridden on the command line.
> @@ -252,7 +246,7 @@ ask_filename(struct passwd *pw, const ch
>   char *name = NULL;
>  
>   if (key_type_name == NULL)
> - name = _PATH_SSH_CLIENT_ID_RSA;
> + name = _PATH_SSH_CLIENT_ID_ED25519;
>   else {
>   switch (sshkey_type_from_name(key_type_name)) {
>   case KEY_DSA_CERT:
> @@ -3748,7 +3742,7 @@ main(int argc, char **argv)
>   }
>  
>   if (key_type_name == NULL)
> - key_type_name = DEFAULT_KEY_TYPE_NAME;
> + key_type_name = "ed25519";
>  
>   type = sshkey_type_from_name(key_type_name);
>   type_bits_valid(type, key_type_name, );
> 
> 



ssh internal refactor

2022-10-27 Thread Damien Miller
Hi,

I just committed a fairly large refactoring of openssh's key handling
code. It *should* be completely invisible, but if you notice any
new problems then please let open...@openssh.com know.

Thanks,
Damien



Re: [PATCH] ssh: don't make the umask more permissive than the user has set

2022-10-06 Thread Damien Miller
On Tue, 4 Oct 2022, Alex Henrie wrote:

> Daniel Kahn Gillmor suggested this exact change on the openssh-unix-dev
> mailing list in 2008, but I couldn't find any reply. [1]
> 
> Ignoring the current umask makes it hard to fulfill the Defense
> Information Systems Agency's Security Technical Implementation Guide. In
> particular, it requires:
> 
> "The Red Hat Enterprise Linux operating system must be configured so
> that all files and directories contained in local interactive user home
> directories have a mode of 0750 or less permissive." [2]
> 
> "The Red Hat Enterprise Linux operating system must set the umask value
> to 077 for all local interactive user accounts." [3]

Thanks - applied with a slight tweak:

-   umask(022);
+   umask(022 | umask(077));

which better matches what we do in sshd.

-d



Re: libfido2 update

2022-08-28 Thread Damien Miller
On Sun, 28 Aug 2022, Stuart Henderson wrote:

> On 2022/08/24 17:09, Damien Miller wrote:
> > Hi,
> > 
> > https://www.mindrot.org/misc/libfido2-1.11.0.diff contains an update
> > for src/libfido2 from 1.8 to 1.11 (about 10 months of upstream
> > development).
> > 
> > I've tested it with OpenSSH, which is the only thing in src/ that
> > uses it as well as compiling www/chromium and performing a FIDO login
> > with it.
> > 
> > ok?
> > 
> > -d
> > 
> 
> Works for me, OK sthen@.

Thanks!

> Would you mind adding NEWS from upstream so we can see a quick summary
> of what's changed please?

done



Re: libfido2 update

2022-08-24 Thread Damien Miller
On Wed, 24 Aug 2022, Damien Miller wrote:

> Hi,
> 
> https://www.mindrot.org/misc/libfido2-1.11.0.diff contains an update
> for src/libfido2 from 1.8 to 1.11 (about 10 months of upstream
> development).
> 
> I've tested it with OpenSSH, which is the only thing in src/ that
> uses it as well as compiling www/chromium and performing a FIDO login
> with it.
> 
> ok?

If you tried to fetch the patch and got a HTTP 500, then please try
again. Unsure why httpd returns 500 for bad permissions instead of
the more usual 403...

-d



libfido2 update

2022-08-24 Thread Damien Miller
Hi,

https://www.mindrot.org/misc/libfido2-1.11.0.diff contains an update
for src/libfido2 from 1.8 to 1.11 (about 10 months of upstream
development).

I've tested it with OpenSSH, which is the only thing in src/ that
uses it as well as compiling www/chromium and performing a FIDO login
with it.

ok?

-d



Re: ksh: PROMPT_COMMAND

2022-08-03 Thread Damien Miller
On Thu, 4 Aug 2022, Christian Weisgerber wrote:

> Damien Miller:
> 
> > bash has a PROMPT_COMMAND that allows a command to be executed before
> > each PS1 prompt is displayed. I've found this useful on occasion, so
> > this is the same thing for ksh(1).
> 
> PS1 is evaluated each time the prompt is displayed.  You can put a
> command in $() in there.

ah, I missed that this was possible - thanks :)



ksh: PROMPT_COMMAND

2022-08-02 Thread Damien Miller
Hi,

bash has a PROMPT_COMMAND that allows a command to be executed before
each PS1 prompt is displayed. I've found this useful on occasion, so
this is the same thing for ksh(1).

In particular, this allows PROMPT_COMMAND to be set to a user-defined
shell function that can modify PS1, though it could also produce its
own output directly.

Is anyone other than me interested in this? If so, review by someone
familiar with ksh's guts would be welcome.

-d


Index: ksh.1
===
RCS file: /cvs/src/bin/ksh/ksh.1,v
retrieving revision 1.216
diff -u -p -r1.216 ksh.1
--- ksh.1   31 Mar 2022 17:27:14 -  1.216
+++ ksh.1   2 Aug 2022 09:07:43 -
@@ -1520,6 +1520,15 @@ See
 below.
 .It Ev PPID
 The process ID of the shell's parent (read-only).
+.It Ev PROMPT_COMMAND
+Used to specify a shell command that is executed before
+.Ev PS1
+is displayed.
+.Ev PROMPT_COMMAND
+may be a normal environment variable, in which case it is executed directly,
+or may be an array of commands that will be executed in order.
+Commands may be functions that modify or replace
+.Ev PS1 .
 .It Ev PS1
 The primary prompt for interactive shells.
 Parameter, command, and arithmetic
Index: lex.c
===
RCS file: /cvs/src/bin/ksh/lex.c,v
retrieving revision 1.78
diff -u -p -r1.78 lex.c
--- lex.c   15 Jan 2018 14:58:05 -  1.78
+++ lex.c   2 Aug 2022 09:07:43 -
@@ -1173,6 +1173,37 @@ special_prompt_expand(char *str)
return str;
 }
 
+static void
+prompt_command(char *str)
+{
+   struct op *t;
+   Source *s;
+
+   s = pushs(SSTRING, ATEMP);
+   s->start = s->str = str;
+   t = compile(s);
+   if (t == NULL || t->type == TEOF)
+   return;
+   exstat = execute(t, XERROK, NULL);
+}
+
+/* If $PROMPT_COMMAND is set, then try to execute it before expanding $PS1 */
+static void
+try_prompt_command(void)
+{
+   struct tbl *e;
+
+   e = global("PROMPT_COMMAND");
+   /* Handle regular variables and arrays */
+   for (; e; e = e->u.array) {
+   if (!(e->flag & ISSET))
+   continue;
+   prompt_command(str_val(e));
+   if (!(e->flag & ARRAY))
+   return;
+   }
+}
+
 void
 set_prompt(int to)
 {
@@ -1183,6 +1214,7 @@ set_prompt(int to)
 
switch (to) {
case PS1: /* command */
+   try_prompt_command();
ps1 = str_save(str_val(global("PS1")), ATEMP);
saved_atemp = ATEMP;/* ps1 is freed by substitute() */
newenv(E_ERRH);



Re: randomise arc4random() rekey interval

2022-07-30 Thread Damien Miller
On Fri, 29 Jul 2022, Theo de Raadt wrote:

> The question is what _rs_random_u32() will do when it calls
> _rs_stir_if_needed().
>
> There is one potential problem. lib/libcrypto/arc4random/*.h contains
> portable wrappers for _rs_forkdetect(), which actually do things.
> memset(rs, 0, sizeof(*rs)) will trash the rs state. Let's imagine a
> "fork" has happened same time that bytes run out.
>
> _rs_stir()
> ...
> rs->rs_count = REKEY_BASE;
> _rs_random_u32 -> _rs_stir_if_needed -> _rs_forkdetect
>   - all rs fields are zero'd with memset
>   - _rs_forkdetect returns
> 
> back in _rs_stir_if_needed,
>- if (!rs || rs->rs_count <= len)
>_rs_stir();
> 
>
> So it will recurse once (only once, because a 2nd fork cannot happen).
> But this is fragile.
>
> Alternatives are to get the value direct from getentropy -- with
> KEYSZ + IVSZ + 4 maybe? Or fetch a value for this random bias early
> and track it in rs, but don't damage it in the memset? Or split
> _rs_random_u32() so that a sub-function of it may collect these 4
> keystream bytes without the _rs_stir_if_needed/_rs_rekey checks?

I don't see how a fork could trash these - do you mean one that
happened in a thread or a signal handler? AFAIK arc4random() isn't
safe in these contexts right now, even without fork().

Anyway, this version invokes the chacha context directly so there's
not possibility of _rs_stir() reentrance. It is still not safe against
something clobbering rsx concurrently (but neither is the existing
code).

Index: crypt/arc4random.c
===
RCS file: /cvs/src/lib/libc/crypt/arc4random.c,v
retrieving revision 1.56
diff -u -p -r1.56 arc4random.c
--- crypt/arc4random.c  28 Feb 2022 21:56:29 -  1.56
+++ crypt/arc4random.c  30 Jul 2022 08:38:44 -
@@ -49,6 +49,8 @@
 #define BLOCKSZ64
 #define RSBUFSZ(16*BLOCKSZ)
 
+#define REKEY_BASE (1024*1024) /* NB. should be a power of 2 */
+
 /* Marked MAP_INHERIT_ZERO, so zero'd out in fork children. */
 static struct _rs {
size_t  rs_have;/* valid bytes at end of rs_buf */
@@ -86,6 +88,7 @@ static void
 _rs_stir(void)
 {
u_char rnd[KEYSZ + IVSZ];
+   uint32_t rekey_fuzz = 0;
 
if (getentropy(rnd, sizeof rnd) == -1)
_getentropy_fail();
@@ -100,7 +103,10 @@ _rs_stir(void)
rs->rs_have = 0;
memset(rsx->rs_buf, 0, sizeof(rsx->rs_buf));
 
-   rs->rs_count = 160;
+   /* rekey interval should not be predictable */
+   chacha_encrypt_bytes(>rs_chacha, (uint8_t *)_fuzz,
+(uint8_t *)_fuzz, sizeof(rekey_fuzz));
+   rs->rs_count += REKEY_BASE + (rekey_fuzz % REKEY_BASE);
 }
 
 static inline void



Re: randomise arc4random() rekey interval

2022-07-27 Thread Damien Miller
On Thu, 28 Jul 2022, Damien Miller wrote:

> On Wed, 27 Jul 2022, Theo de Raadt wrote:
> 
> > +   rs->rs_count += rekey_fuzz & (REKEY_BASE - 1);
> > 
> > I mean, why not use % here
> 
> Sure, that's reasonable.

revised:

Index: crypt/arc4random.c
===
RCS file: /cvs/src/lib/libc/crypt/arc4random.c,v
retrieving revision 1.56
diff -u -p -r1.56 arc4random.c
--- crypt/arc4random.c  28 Feb 2022 21:56:29 -  1.56
+++ crypt/arc4random.c  28 Jul 2022 00:59:34 -
@@ -49,6 +49,8 @@
 #define BLOCKSZ64
 #define RSBUFSZ(16*BLOCKSZ)
 
+#define REKEY_BASE (1024*1024) /* NB. should be a power of 2 */
+
 /* Marked MAP_INHERIT_ZERO, so zero'd out in fork children. */
 static struct _rs {
size_t  rs_have;/* valid bytes at end of rs_buf */
@@ -63,6 +65,8 @@ static struct _rsx {
 
 static inline int _rs_allocate(struct _rs **, struct _rsx **);
 static inline void _rs_forkdetect(void);
+static inline void _rs_random_u32(uint32_t *);
+
 #include "arc4random.h"
 
 static inline void _rs_rekey(u_char *dat, size_t datlen);
@@ -86,6 +90,7 @@ static void
 _rs_stir(void)
 {
u_char rnd[KEYSZ + IVSZ];
+   uint32_t rekey_fuzz;
 
if (getentropy(rnd, sizeof rnd) == -1)
_getentropy_fail();
@@ -100,7 +105,10 @@ _rs_stir(void)
rs->rs_have = 0;
memset(rsx->rs_buf, 0, sizeof(rsx->rs_buf));
 
-   rs->rs_count = 160;
+   rs->rs_count = REKEY_BASE;
+   /* rekey interval should not be predictable */
+   _rs_random_u32(_fuzz);
+   rs->rs_count += rekey_fuzz % REKEY_BASE;
 }
 
 static inline void



Re: randomise arc4random() rekey interval

2022-07-27 Thread Damien Miller
On Wed, 27 Jul 2022, Theo de Raadt wrote:

> +   rs->rs_count += rekey_fuzz & (REKEY_BASE - 1);
> 
> I mean, why not use % here

Sure, that's reasonable.

> And then, set the default to a pow2.
> 
> But if someone changes it to not pow2, it still works.
> 
> The & is premature hand-optimization, let a compiler do it if it can.



Re: randomise arc4random() rekey interval

2022-07-27 Thread Damien Miller
On Wed, 27 Jul 2022, Theo de Raadt wrote:

> I love it.
> 
> > +#define REKEY_BASE (1<<20) /* NB. *must* be a power of 2 */
> 
> Why insist on that?

Because I need to do this later:

+   rs->rs_count += rekey_fuzz & (REKEY_BASE - 1);

because we can't use arc4random_uniform() in this context.

> Also, I would prefer (1024*1024), it is quicker to read.

ack



randomise arc4random() rekey interval

2022-07-27 Thread Damien Miller
Hi,

arc4random() rekeys currently rekeys from the kernel every 1.6MB.
It costs us almost nothing to make this interval non-deterministic,
so let's do that.

With the below it will rekey randomly somewhere between 1MB and 2MB.

ok?

Index: crypt/arc4random.c
===
RCS file: /cvs/src/lib/libc/crypt/arc4random.c,v
retrieving revision 1.56
diff -u -p -r1.56 arc4random.c
--- crypt/arc4random.c  28 Feb 2022 21:56:29 -  1.56
+++ crypt/arc4random.c  28 Jul 2022 00:37:13 -
@@ -49,6 +49,8 @@
 #define BLOCKSZ64
 #define RSBUFSZ(16*BLOCKSZ)
 
+#define REKEY_BASE (1<<20) /* NB. *must* be a power of 2 */
+
 /* Marked MAP_INHERIT_ZERO, so zero'd out in fork children. */
 static struct _rs {
size_t  rs_have;/* valid bytes at end of rs_buf */
@@ -63,6 +65,8 @@ static struct _rsx {
 
 static inline int _rs_allocate(struct _rs **, struct _rsx **);
 static inline void _rs_forkdetect(void);
+static inline void _rs_random_u32(uint32_t *);
+
 #include "arc4random.h"
 
 static inline void _rs_rekey(u_char *dat, size_t datlen);
@@ -86,6 +90,7 @@ static void
 _rs_stir(void)
 {
u_char rnd[KEYSZ + IVSZ];
+   uint32_t rekey_fuzz;
 
if (getentropy(rnd, sizeof rnd) == -1)
_getentropy_fail();
@@ -100,7 +105,10 @@ _rs_stir(void)
rs->rs_have = 0;
memset(rsx->rs_buf, 0, sizeof(rsx->rs_buf));
 
-   rs->rs_count = 160;
+   rs->rs_count = REKEY_BASE;
+   /* rekey interval should not be predictable */
+   _rs_random_u32(_fuzz);
+   rs->rs_count += rekey_fuzz & (REKEY_BASE - 1);
 }
 
 static inline void



Re: vsw.4: mention veb next to bridge

2022-07-20 Thread Damien Miller
On Wed, 20 Jul 2022, Chris Cappuccio wrote:

> Klemens Nanni [k...@openbsd.org] wrote:
> > veb(4) works just fine in this setup, so don't give the impression only
> > bridge(4) would work.
> 
> In related items, is it time to tedu bridge(4) and vether(4) ? Is there
> anything veb(4) and vport(4) can't do?

There's the link2 ipsec stuff that bridge(4) supports but veb(4)
doesn't, IDK if anyone uses it. There's some commented out code in
if_veb.c for this, but I don't know whether it is commented out because
it doesn't work or because nobody has asked for it

At the very least, the ioctls documentation in bridge.4 would need
to be copied over to veb.4 - it looks like most of them are supported.

-d



Re: Picky, but much more efficient arc4random_uniform!

2022-05-18 Thread Damien Miller
On Wed, 18 May 2022, Otto Moerbeek wrote:

> instrumenting the code to count the number of arc4random calls I see thsi:
> 
> openbsd; elapsed = 2.835819; calls = 12340949
> bitmask; elapsed = 4.335576; calls = 17836216
> bitmask+reuse; elapsed = 3.710277; calls = 15245337
> 
> (this is a different number of trials on a slow machine).
> 
> So the promise of less calls is not fulfilled. Sounds like a bug.

Well, I don't know whether the simple bitmasking approach will really
result in fewer calls. I *think* our current approach has a higher
probability per loop of suceeding (at least for small upper_bounds) than
mask-and-test, but my brain is too addled with a cold to calculate it :/

What Theo said is 100% right - the cost is dominated by that of the
underlying RNG. If anyone wants a faster arc4random_uniform() then the
first place to look it at arc4random().

It's entirely possible that the speedups measured in that article
are because they are using a omgoptimised (non-crypto) RNG and the
improvements they saw were due solely to reduction the overhead inside
the uniformity code even if it came at the cost of more RNG queries.

Anyway, here's a tweaked up version of the test harness that fakes out
arc4random() with a deterministic RNG that counts how often it is called
in case anyone wants to play with it further.




#include 
#include 

#include 
#include 

static uint32_t nqueries;

/* deterministic, query-counting arc4random() replacement for benchmarking */
static uint32_t
fake_random(void)
{
static uint32_t ready, remain, msb;
uint32_t ret;

if (!ready) {
srand_deterministic(31337);
ready = 1;
}
if (remain == 0) {
msb = (uint32_t)rand();
remain = 31; /* XXX makes assumption re RAND_MAX */
}
ret = (uint32_t)rand() | (msb << 31);
msb >>= 1;
remain--;
nqueries++;
return ret; 
}

#define arc4random() fake_random()

#ifndef __has_builtin
#define __has_builtin(x) 0
#endif
#if __has_builtin(__builtin_clz)
#define arc4random_clz(x) __builtin_clz(x)
#else
#warning lacks __builtin_clz()
/* Count most-significant zero bits, like __builtin_clz() */
static int
arc4random_clz(unsigned int x)
{
int ret = 0;
unsigned int n;
const int lut[16] = { 4, 3, 2, 2, 1, 1, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0 };

while (x != 0) {
n = (x >> 28) & 0xf;
if (n != 0)
return ret + lut[n];
x <<= 4;
ret += 4;
}
return 32;
}
#endif

/* Current OpenBSD implementation */
static uint32_t
arc4random_uniform1(uint32_t upper_bound)
{
uint32_t r, min;

if (upper_bound < 2)
return 0;

/* 2**32 % x == (2**32 - x) % x */
min = -upper_bound % upper_bound;

/*
 * This could theoretically loop forever but each retry has
 * p > 0.5 (worst case, usually far better) of selecting a
 * number inside the range we need, so it should rarely need
 * to re-roll.
 */
for (;;) {
r = arc4random();
if (r >= min)
break;
}

return r % upper_bound;
}

/*
 * Like "Bitmask with Rejection" implementation from
 * https://www.pcg-random.org/posts/bounded-rands.html
 */
static uint32_t
arc4random_uniform2(uint32_t upper_bound)
{
uint32_t mask, r;

if (upper_bound < 2)
return 0;

mask = 0x >> arc4random_clz((upper_bound - 1) | 1);;
for (;;) {
r = arc4random() & mask;
if (r < upper_bound)
return r;
}
/* NOTREACHED */
}

/*
 * Like Apple's
 * 
https://opensource.apple.com/source/Libc/Libc-1158.50.2/gen/FreeBSD/arc4random.c
 */
static uint32_t
arc4random_uniform3(uint32_t upper_bound)
{
int zeroes, bits, remain;
uint32_t mask, r, rm;

if (upper_bound < 2)
return 0;

zeroes = arc4random_clz((upper_bound - 1) | 1);
bits = 32 - zeroes;
mask = (uint32_t)-1 >> zeroes;

for (;;) {
r = arc4random();
rm = r & mask;
if (rm < upper_bound)
return rm;
for (remain = zeroes; remain >= bits; remain -= bits) {
r >>= bits;
rm = r & mask;
if (rm < upper_bound)
return rm;
}
}
/* NOTREACHED */
}

#include 

static uint32_t
fixture(const char *s, uint32_t (* const rnd)(uint32_t))
{
const uint32_t trials = 20 * 1000 * 1000;
const uint32_t max = 0x8000;
const uint32_t mul = 7;
unsigned int v, i, r;
struct timeval start, finish, delta;

nqueries = 0;
gettimeofday(, 

Re: Picky, but much more efficient arc4random_uniform!

2022-05-17 Thread Damien Miller
On Tue, 17 May 2022, Raimo Niskanen wrote:

> Why reinvent the wheel?
> 
> Here is a pretty good walkthrough of established methods:
> 
> https://www.pcg-random.org/posts/bounded-rands.html
> 
> It sounds to me as if your suggested methor essentially is
> "Bitmask with Rejection -- Apple's Method" with the added twist
> to keep the unused bits of the base generator's word and append
> new, which might be an optimization for small ranges, but might
> be just overhead for large ones.
> 
> In that post one can see that there might be other suggested smaller
> optimizations that can be applied to the OpenBSD method, and that
> the bitmask method is effective in many cases but not a clear winner.

Oh nice, I wasn't aware that Apple had an improved algorithm. I always
thought the best avenue for a speedup was to consider only the bits that
could satisfy the constraint, but it never occurred to me how to actually
make use of this :)

The toy benchmark below compares the existing implementation with
reimplementations of both their version as well as something more close
to Apple's actual method (which reuses the high bits).

However, I don't see a speedup for either of the alternate approaches.

[djm@djm ~]$ cc -O2 -Werror -Wall -Wextra -o rb rb.c && doas nice -n -20 ./rb 
openbsd; elapsed = 8.327954
bitmask; elapsed = 13.306228
bitmask+reuse; elapsed = 11.567389

Maybe my benchmark is crap or maybe I need dlg@ to omgoptimize it for me...



#include 
#include 
#include 
#include 

#ifndef __has_builtin
#define __has_builtin(x) 0
#endif
#if __has_builtin(__builtin_clz)
#define arc4random_clz(x) __builtin_clz(x)
#else
#warning lacks __builtin_clz()
/* Count most-significant zero bits, like __builtin_clz() */
static int
arc4random_clz(unsigned int x)
{
int ret = 0;
unsigned int n;
const int lut[16] = { 4, 3, 2, 2, 1, 1, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0 };

while (x != 0) {
n = (x >> 28) & 0xf;
if (n != 0)
return ret + lut[n];
x <<= 4;
ret += 4;
}
return 32;
}
#endif

/* Current OpenBSD implementation */
static uint32_t
arc4random_uniform1(uint32_t upper_bound)
{
uint32_t r, min;

if (upper_bound < 2)
return 0;

/* 2**32 % x == (2**32 - x) % x */
min = -upper_bound % upper_bound;

/*
 * This could theoretically loop forever but each retry has
 * p > 0.5 (worst case, usually far better) of selecting a
 * number inside the range we need, so it should rarely need
 * to re-roll.
 */
for (;;) {
r = arc4random();
if (r >= min)
break;
}

return r % upper_bound;
}

/*
 * Like "Bitmask with Rejection" implementation from
 * https://www.pcg-random.org/posts/bounded-rands.html
 */
static uint32_t
arc4random_uniform2(uint32_t upper_bound)
{
uint32_t mask, r;

if (upper_bound < 2)
return 0;

mask = (uint32_t)-1 >> arc4random_clz((upper_bound - 1) | 1);;
for (;;) {
r = arc4random();
if ((r & mask) < upper_bound)
return r & mask;
}
/* NOTREACHED */
}

/*
 * Like Apple's
 * 
https://opensource.apple.com/source/Libc/Libc-1158.50.2/gen/FreeBSD/arc4random.c
 */
static uint32_t
arc4random_uniform3(uint32_t upper_bound)
{
int zeroes, bits, remain = 32;
uint32_t mask, r;

if (upper_bound < 2)
return 0;

zeroes = arc4random_clz((upper_bound - 1) | 1);
bits = 32 - zeroes;
mask = (uint32_t)-1 >> zeroes;

for (;;) {
r = arc4random();
if ((r & mask) < upper_bound)
return r & mask;
for (remain = zeroes; remain >= bits; remain -= bits) {
r >>= bits;
if ((r & mask) < upper_bound)
return r & mask;
}
}
/* NOTREACHED */
}


#include 

static uint32_t
fixture(const char *s, uint32_t (* const rnd)(uint32_t))
{
const uint32_t trials = 50 * 1000 * 1000;
const uint32_t max = 0x8000;
const uint32_t mul = 7;
unsigned int v, i, r;
struct timeval start, finish, delta;

gettimeofday(, NULL);
for (v = 3, r = 0; v < max; v *= mul) {
/* printf("%u\n", v); */
for (i = 0; i < trials; i++)
r |= rnd(v);
}
gettimeofday(, NULL);
timersub(, , );
printf("%s; elapsed = %lld.%06lld\n", s,
(long long)delta.tv_sec, (long long)delta.tv_usec);
return r;
}

int main(void) {
fixture("openbsd", arc4random_uniform1);
fixture("bitmask", arc4random_uniform2);
fixture("bitmask+reuse", arc4random_uniform3);
}



Re: Picky, but much more efficient arc4random_uniform!

2022-05-16 Thread Damien Miller
On Mon, 16 May 2022, Luke Small wrote:

> Yeah, I see your point.
>
> I suppose it depends on how conservative you want to be and whether
> you want to supply options to people like getchar_unlocked when it
> isn’t essential.
>
> It could be made manually fork-safe if I could make a simple feature
> where “arc4random_uniform_unlocked(0);” with a 0 upperbound could
> trigger a reset of the static variables rand_bits and rand_holder
> which would be simple enough and could be added to a man page. I
> certainly read the man pages. (I’m surprised “man getchar” doesn’t
> “see also” to getchar_unlocked() by the way.)
>
> If you look at profiling with programs that call it a lot,
> arc4random() inside arc4random_uniform() calls the expensive rekey
> function which makes it take more time. That’s why I can get around
> 2X-3X performance on a typical repetitive small upperbound loop and
> an extra 20% improvement on a single 65536 Knuth shuffle, loop even
> though my function repeats a binary search for ‘bit’ size every single
> time and has misses which demands calling up more data.
>
> Otherwise my function would be particularly useful when there’s a loop
> with small upperbound like: 26+26+10 which, if I recall correctly, is
> in identd, which would call it frequently.

Just to bring this back to where we came in: even putting thread-safety
aside (which you absolutely can't): it doesn't matter how much faster
it is, your replacement function isn't useful until you do the work to
demonstrate correctness.

You have done literally zero so far, not even the bare minimum of
testing the output. As a result your first version was demonstrated to
be completely broken by literally the most basic of possible tests, a
mere 10 lines of code.

That you left this to others to do tells me that you fundamentally don't
understand the environment in which you're trying to operate, and that
you don't respect the time of the people you're trying to convince.

Please stop wasting our time.

-d


Re: Picky, but much more efficient arc4random_uniform!

2022-05-15 Thread Damien Miller
On Sat, 14 May 2022, Luke Small wrote:

> Look at my code. I don’t even use a modulus operator. I perform hit and
> miss with a random bitstream.
> 
> How can I have a bias of something I don’t do? I return a bitstream which
> meets the parameters of being a value less than the upper bound. Much like
> arc4random_buf().
> 
> If I use arc4random_uniform() repeatedly to create a random distribution of
> say numbers less than 0x1000 or even something weird like 0x1300 will the
> random distribution be better with arc4random_uniform() or with mine? For
> 0x1000 mine will simply pluck 12 bits of random data straight from the
> arc4random() (and preserve the remaining 20 bits for later) on the first
> try, just like it’s arc4random_buf().
> 
> arc4random_uniform() will perform a modulus of a 32 bit number which adds
> data to the bitstream. Does it make it better? Perhaps it makes it harder
> to guess the source bits.
> 
> I don’t know; and I’m not going to pretend to be a cryptologist. But I’m
> looking at modulo bias.
> 
> I didn’t know what it was, before, but I basically “rejection sample”:
> 
> https://research.kudelskisecurity.com/2020/07/28/the-definitive-guide-to-modulo-bias-and-how-to-avoid-it/

No, you aren't:

> for (;;) {
> if (rand_bits < bits) {
> rand_holder |= ((uint64_t)arc4random()) <<
> rand_bits;
> 
> /* 
>  * rand_bits will be a number between 0 and 31 here
>  * so the 0x20 bit will be empty
>  * rand_bits += 32;
>  */ 
> rand_bits |= 32;
> }
> 
> ret = rand_holder & uuu;
> rand_holder >>= bits;
> rand_bits -= bits;
> 
> if (ret < upper_bound)
> return ret;
> }

This isn't rejection sampling. This is reusing part of the rejected
sample.

Think of it like this: you want to uniformly generate a number in the
range [2:10] by rolling 2x 6-sided dice. What do you do when you roll
11 or 12? You can't just reroll one of the dice because the other dice
is constrained to be have rolled either 5 or 6, and so proceeding with
it would force the output to be in the range [6:11] for these ~5.6%
of initial rolls. Your output is no longer uniform.

BTW the existing code already implements the prefered approach of the
article you quoted.

-d


Re: Picky, but much more efficient arc4random_uniform!

2022-05-15 Thread Damien Miller
On Sun, 15 May 2022, Luke Small wrote:

> Do I really have to use specific terminology to make a point?
>
> I'm not educated enough on chacha20 enough to know whether, like I
> pointed out, whether choosing 5 bits from the middle of (or even from
> the tail end of one and the beginning of another) 32 bit pseudorandom
> cipher is "correct."

You don't need to understand chacha20 to understand.
arc4random_uniform() I certainly didn't when I wrote it.

The underlying CSPRNG is irrelevant to how arc4random_uniform() works.
It it treated as an oracle that provides 32 random bit upon request. You
could swap it out for 32 coin-tossing monkeys and the implementation
wouldn't need to change.

It requests another 32 bit random value for each attempt at satisfying
the bounds check because they need to be independent - reusing parts of
a previous attempt is highly likely to introduce biases.

It's almost certainly possible to make this function faster, but it's
also very easy to get it wrong (e.g. I made one stupid math error in its
early implementation, forever immortalised by CVS). The existing code
has the advantage of being very obvious in how it works and therefore
has a very low risk of being wrong.

If someone is proposing to move to something less obvious then it's
incumbent upon them to do the work to prove that their alternative is
just as correct.

> ...correct correct correct. Did I use that buzzword enough?

Highly experienced people are taking he time to give you detailed,
critical feedback. This can be hard to receive, but if you ever want to
improve then you should consider it and try to engage constructively.

-d



Re: Reserved address behavior (alternate broadcast and 240/4)

2022-05-04 Thread Damien Miller
On Wed, 4 May 2022, Seth David Schoen wrote:

[snip]

> Anyway, one thing we would like to propose that OpenBSD update is the
> in_canforward treatment of 240/4 (former class E) addresses.  Apparently
> mainly as a result of proposals in 2008 to make these addresses more
> usable, most OSes now no longer treat these specially (although they
> can't yet be allocated as public address space).  They have been seeing
> some considerable use as unofficial private address space.  OpenBSD is
> one of the numerous systems that already permits these addresses to be
> assigned to an interface and used by a socket, but there's one remaining
> discrepancy in the in_canforward definition.
> 
> This has some odd consequences.  For instance, if an OpenBSD system
> has an interface numbered with an address in 240/4, it can initiate
> and receive TCP connections using that address, and it can ping other
> hosts using that address, but it won't respond to pings from other
> hosts.  This patch cleans this up:
> 
> 
> Index: in.c
> ===
> RCS file: /cvs/src/sys/netinet/in.c,v
> retrieving revision 1.173
> diff -u -p -r1.173 in.c
> --- in.c  28 Mar 2022 16:31:26 -  1.173
> +++ in.c  5 May 2022 01:05:04 -
> @@ -103,7 +103,7 @@ in_canforward(struct in_addr in)
>  {
>   u_int32_t net;
>  
> - if (IN_EXPERIMENTAL(in.s_addr) || IN_MULTICAST(in.s_addr))
> + if (IN_MULTICAST(in.s_addr))

This seems reasonanble. It looks like this is the only use of
IN_EXPERIMENTAL() in base, so maybe we should remove the definition too?

There is the posibility that it might affect some ports, but I'd argue
that unless they similarly adapt then they would already be broken (at
least wrt 240/4 addresses).

Alternetely, we could retain it as "#define IN_EXPERIMENTAL() 0"

-d



Re: ssh: sshkey.c: reduce code duplication

2022-05-04 Thread Damien Miller
On Wed, 4 May 2022, Martin Vahlensieck wrote:

> Hi
> 
> I noticed that sshkey_unshield_private contains a exact duplicate
> of the code in private2_check_padding.  So by pulling
> private2_check_padding up, the code can be reused.  Or is there
> a reason for this split?

Thanks - this has been applied too.

-d



Re: ssh: channels.c: Fix comment and add a const

2022-05-04 Thread Damien Miller
applied

On Wed, 4 May 2022, Martin Vahlensieck wrote:

> Hi
> 
> channel_new no longer frees remote_name.  So update the comment
> accordingly.  As remote_name is not modified, it can be const
> as well.
> 
> Best,
> 
> Martin
> 
> Index: channels.c
> ===
> RCS file: /home/reposync/cvs/src/usr.bin/ssh/channels.c,v
> retrieving revision 1.418
> diff -u -p -r1.418 channels.c
> --- channels.c4 May 2022 07:31:22 -   1.418
> +++ channels.c4 May 2022 19:02:14 -
> @@ -349,12 +349,11 @@ channel_register_fds(struct ssh *ssh, Ch
>  }
>  
>  /*
> - * Allocate a new channel object and set its type and socket. This will cause
> - * remote_name to be freed.
> + * Allocate a new channel object and set its type and socket.
>   */
>  Channel *
>  channel_new(struct ssh *ssh, char *ctype, int type, int rfd, int wfd, int 
> efd,
> -u_int window, u_int maxpack, int extusage, char *remote_name, int 
> nonblock)
> +u_int window, u_int maxpack, int extusage, const char *remote_name, int 
> nonblock)
>  {
>   struct ssh_channels *sc = ssh->chanctxt;
>   u_int i, found;
> Index: channels.h
> ===
> RCS file: /home/reposync/cvs/src/usr.bin/ssh/channels.h,v
> retrieving revision 1.142
> diff -u -p -r1.142 channels.h
> --- channels.h30 Mar 2022 21:10:25 -  1.142
> +++ channels.h6 Apr 2022 20:26:56 -
> @@ -272,7 +272,7 @@ Channel   *channel_by_id(struct ssh *, int
>  Channel  *channel_by_remote_id(struct ssh *, u_int);
>  Channel  *channel_lookup(struct ssh *, int);
>  Channel *channel_new(struct ssh *, char *, int, int, int, int,
> - u_int, u_int, int, char *, int);
> + u_int, u_int, int, const char *, int);
>  void  channel_set_fds(struct ssh *, int, int, int, int, int,
>   int, int, u_int);
>  void  channel_free(struct ssh *, Channel *);
> 
> 



Re: ssh: mux.c: mark argument as const

2022-05-04 Thread Damien Miller
applied - thanks

On Wed, 4 May 2022, Martin Vahlensieck wrote:

> Index: mux.c
> ===
> RCS file: /home/reposync/cvs/src/usr.bin/ssh/mux.c,v
> retrieving revision 1.92
> diff -u -p -r1.92 mux.c
> --- mux.c 11 Jan 2022 01:26:47 -  1.92
> +++ mux.c 13 Jan 2022 16:27:14 -
> @@ -227,7 +227,7 @@ mux_master_control_cleanup_cb(struct ssh
>  
>  /* Check mux client environment variables before passing them to mux master. 
> */
>  static int
> -env_permitted(char *env)
> +env_permitted(const char *env)
>  {
>   int i, ret;
>   char name[1024], *cp;
> 
> 



Re: Security support status of xnf(4) and xbf(4)

2022-03-27 Thread Damien Miller
On Fri, 25 Mar 2022, Demi Marie Obenour wrote:

> Linux’s netfront and blkfront drivers recently had a security
> vulnerability (XSA-396) that allowed a malicious backend to potentially
> compromise them.  In follow-up audits, I found that OpenBSD’s xnf(4)
> currently trusts the backend domain.  I reported this privately to Theo
> de Raadt, who indicated that OpenBSD does not consider this to be a
> security concern.
> 
> This is obviously a valid position for the OpenBSD project to take, but
> it is surprising to some (such as myself) from the broader Xen
> ecosystem.  Standard practice in the Xen world is that bugs in frontends
> that allow a malicious backend to cause mischief *are* considered
> security bugs unless there is explicit documentation to the contrary.
> As such, I believe this deserves to be noted in xnf(4) and xbf(4)’s man
> pages.  If the OpenBSD project agrees, I am willing to write a patch,
> but I have no experience with mandoc so it might take a few tries.

Hang on, what is a "malicious backend" in this context? Is it something
other than the Xen Hypervisor? If not, then it seems not to be a useful
attack model, as the hypervisor typically has near-complete access to
guests' memory and CPU state.

-d


Re: Mention Smart Battery Data Spec in smbus.h

2022-03-05 Thread Damien Miller
without commenting on the substance of this change, it should definitely
not be added to the copyright block

On Fri, 4 Mar 2022, patrick keshishian wrote:

> Hello,
> 
> I took a wrong turn, and got interested in where the SMBATT_CMD_*
> defines were sourced.
> 
> Adding a reference to Smart Battery Data Spec might save someone
> else the time searching through ACPI spec, then SMBus spec, to
> finally arriving at the answer.
> 
> Is the following diff acceptable?
> I believe this is the correct/definitive source.
> 
> Thanks,
> --patrick
> 
> 
> Index: smbus.h
> ===
> RCS file: /cvs/obsd/src/sys/dev/acpi/smbus.h,v
> retrieving revision 1.1
> diff -u -p -u -p -r1.1 smbus.h
> --- smbus.h   22 Feb 2017 16:39:56 -  1.1
> +++ smbus.h   5 Mar 2022 07:37:06 -
> @@ -2,6 +2,9 @@
>   * Copyright (c) 2005 Hans Petter Selasky
>   * All rights reserved.
>   *
> + * Smart Battery Data Specification
> + * http://sbs-forum.org/specs/sbdat110.pdf
> + *
>   * Redistribution and use in source and binary forms, with or without
>   * modification, are permitted provided that the following conditions
>   * are met:
> 
> 



Re: ssh/sshd change in snaps

2021-11-16 Thread Damien Miller
On Wed, 17 Nov 2021, Damien Miller wrote:

> On Tue, 16 Nov 2021, Damien Miller wrote:
> 
> > Another couple of fixes in tomorrow's snaps. One to avoid errors like:
> > 
> > > channel 3: chan_read_failed for istate 3
> > 
> > Another avoids a situation where sshd could get stuck spinning on poll()
> > if it fails.
> > 
> > (Both uncovered by web browsing through a SOCKS dynamic forwarder)
> 
> BTW I have uploaded the diff for this change to github in case anyone
> is curious:
> 
> https://patch-diff.githubusercontent.com/raw/djmdjm/openssh-wip/pull/6

sorry, bad link. Correct one is:

https://github.com/djmdjm/openssh-wip/pull/6.diff
https://github.com/djmdjm/openssh-wip/pull/6



Re: ssh/sshd change in snaps

2021-11-16 Thread Damien Miller
On Tue, 16 Nov 2021, Damien Miller wrote:

> Another couple of fixes in tomorrow's snaps. One to avoid errors like:
> 
> > channel 3: chan_read_failed for istate 3
> 
> Another avoids a situation where sshd could get stuck spinning on poll()
> if it fails.
> 
> (Both uncovered by web browsing through a SOCKS dynamic forwarder)

BTW I have uploaded the diff for this change to github in case anyone
is curious:

https://patch-diff.githubusercontent.com/raw/djmdjm/openssh-wip/pull/6

or in patch(1) format:

https://patch-diff.githubusercontent.com/raw/djmdjm/openssh-wip/pull/6.diff



Re: ssh/sshd change in snaps

2021-11-15 Thread Damien Miller
On Sat, 13 Nov 2021, Damien Miller wrote:

> On Thu, 11 Nov 2021, Damien Miller wrote:
> 
> > Hi,
> > 
> > Snaps is now carrying a change to ssh/sshd that converts their
> > mainloops from select(2) to poll/ppoll(2). This change should be
> > completely transparent, but please be on the lookout for any weird
> > behaviour. Bugs in the revised mainloop are most likely to appear
> > as crashes, hangs or ssh/sshd using lots of CPU when idle.
> > 
> > Please report anything you see here, to openssh@ or to me.
> 
> FYI this has gone through one interation of bugfixes since it first went
> in. If you are running a snap from yesterday and experience problems then
> please upgrade to a newer one and retest.

Another couple of fixes in tomorrow's snaps. One to avoid errors like:

> channel 3: chan_read_failed for istate 3

Another avoids a situation where sshd could get stuck spinning on poll()
if it fails.

(Both uncovered by web browsing through a SOCKS dynamic forwarder)

-d



Re: ssh/sshd change in snaps

2021-11-12 Thread Damien Miller
On Thu, 11 Nov 2021, Damien Miller wrote:

> Hi,
> 
> Snaps is now carrying a change to ssh/sshd that converts their
> mainloops from select(2) to poll/ppoll(2). This change should be
> completely transparent, but please be on the lookout for any weird
> behaviour. Bugs in the revised mainloop are most likely to appear
> as crashes, hangs or ssh/sshd using lots of CPU when idle.
> 
> Please report anything you see here, to openssh@ or to me.

FYI this has gone through one interation of bugfixes since it first went
in. If you are running a snap from yesterday and experience problems then
please upgrade to a newer one and retest.

-d



ssh/sshd change in snaps

2021-11-10 Thread Damien Miller
Hi,

Snaps is now carrying a change to ssh/sshd that converts their
mainloops from select(2) to poll/ppoll(2). This change should be
completely transparent, but please be on the lookout for any weird
behaviour. Bugs in the revised mainloop are most likely to appear
as crashes, hangs or ssh/sshd using lots of CPU when idle.

Please report anything you see here, to openssh@ or to me.

Thanks,
Damien



Re: New hw.perfpolicy behavior

2021-11-03 Thread Damien Miller
On Wed, 3 Nov 2021, Stuart Henderson wrote:

> > See also https://en.wikichip.org/wiki/race-to-sleep - it's generally
> > more energy efficient to run the CPU at full speed so it can finish its
> > work faster and get back to a low-power state sooner
> 
> So there's not really any point in doing this scaling on laptops either
> then, and it could be counterproductive? Would it actually be better for
> the decision to be "has mwait or an alternative mechanism on other arches"
> and only if that's absent consider whether it has ac power?

It really depends on the workload. If you're compiling something then
going full tilt and then sleeping would probably be better, if you've
got a web browser running JS in the background then maybe cpufreq
scaling will win...

-d



Re: New hw.perfpolicy behavior

2021-11-02 Thread Damien Miller
On Tue, 2 Nov 2021, Theo de Raadt wrote:

> Paul de Weerd  wrote:
> 
> > A recent commit by Theo changed the hw.perfpolicy behavior to always
> > run at full speed when AC power is on.  This means that my workstation
> > (and servers, once I upgrade them) now consumes significantly more
> > power, even though they usually idle.
> 
> Did you measure how much more power?
> 
> You must measure, to make such a claim.
> 
> Your OptiPlex 9020 is probably a modern i5/i7, which probably contains
> C states similar to this:
> 
> acpicpu0 at acpi0: C2(200@148 mwait.1@0x33), C1(1000@1 mwait.1), PSS
> 
> Which means when the idle loop calls the "mwait" instruction, the cpu
> will 'instantly' slow down to a lower clock and make other power use
> reductions, until an interrupt happens and requires labour again.

See also https://en.wikichip.org/wiki/race-to-sleep - it's generally
more energy efficient to run the CPU at full speed so it can finish its
work faster and get back to a low-power state sooner



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: [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.




  1   2   >