Re: pax(1): Don't open files that will be skipped

[Jeremy Evans]

On 12/23 08:36, Jeremy Evans wrote:
> On 11/22 11:15, Jeremy Evans wrote:
> > On 10/19 09:34, Jeremy Evans wrote:
> > > Currently, when creating an archive file with pax(1), pax will attempt
> > > to open a file even if the file will be skipped due to an -s
> > > replacement with the empty string. With this change, pax will not
> > > attempt to open files that it knows will be skipped.
> > > 
> > > When doing direct copies to a directory (-rw), pax already skips
> > > the file before attempting to open it. So this makes the behavior
> > > more consistent.
> > > 
> > > My main reason for this change is to be able to silence the following
> > > warning when backing up:
> > > 
> > >   pax: Unable to open /etc/spwd.db to read: Operation not permitted
> > > 
> > > One other possible benefit is if you are skipping a lot of files,
> > > avoiding the open syscall for each file may make pax faster.
> > > 
> > > OKs?
> > 
> > Ping.  I still think this is worth doing.  I've tested and it is also
> > measurably faster (25%) if you are excluding a lot of files:
> > 
> > $ doas time obj/pax -f /dev/null -w -x cpio -s '/^.*$//' /some-path
> > 2.65 real 1.95 user 0.73 sys
> > $ doas time pax -f /dev/null -w -x cpio -s '/^.*$//' /some-path
> > 3.34 real 2.04 user 1.30 sys
> > 
> > OKs?

Still looking for a review/OK for this.  Resending as there was a
recent commit to pax during g2k23.

Thanks,
Jeremy

Index: ar_subs.c
===
RCS file: /cvs/src/bin/pax/ar_subs.c,v
retrieving revision 1.50
diff -u -p -r1.50 ar_subs.c
--- ar_subs.c   24 Oct 2021 21:24:21 -  1.50
+++ ar_subs.c   7 Jul 2023 00:33:18 -
@@ -441,6 +441,23 @@ wr_archive(ARCHD *arcn, int is_app)
if (hlk && (chk_lnk(arcn) < 0))
break;
 
+   /*
+* Modify the name as requested by the user
+*/
+   if ((res = mod_name(arcn)) < 0) {
+   /*
+* pax finished, purge link table entry and stop
+*/
+   purg_lnk(arcn);
+   break;
+   } else if (res > 0) {
+   /*
+* skipping file, purge link table entry
+*/
+   purg_lnk(arcn);
+   continue;
+   }
+
if (PAX_IS_REG(arcn->type) || (arcn->type == PAX_HRG)) {
/*
 * we will have to read this file. by opening it now we
@@ -456,20 +473,7 @@ wr_archive(ARCHD *arcn, int is_app)
}
}
 
-   /*
-* Now modify the name as requested by the user
-*/
-   if ((res = mod_name(arcn)) < 0) {
-   /*
-* name modification says to skip this file, close the
-* file and purge link table entry
-*/
-   rdfile_close(arcn, );
-   purg_lnk(arcn);
-   break;
-   }
-
-   if ((res > 0) || (docrc && (set_crc(arcn, fd) < 0))) {
+   if (docrc && (set_crc(arcn, fd) < 0)) {
/*
 * unable to obtain the crc we need, close the file,
 * purge link table entry

Re: pax(1): Don't open files that will be skipped

[Jeremy Evans]

On 11/22 11:15, Jeremy Evans wrote:
> On 10/19 09:34, Jeremy Evans wrote:
> > Currently, when creating an archive file with pax(1), pax will attempt
> > to open a file even if the file will be skipped due to an -s
> > replacement with the empty string. With this change, pax will not
> > attempt to open files that it knows will be skipped.
> > 
> > When doing direct copies to a directory (-rw), pax already skips
> > the file before attempting to open it. So this makes the behavior
> > more consistent.
> > 
> > My main reason for this change is to be able to silence the following
> > warning when backing up:
> > 
> >   pax: Unable to open /etc/spwd.db to read: Operation not permitted
> > 
> > One other possible benefit is if you are skipping a lot of files,
> > avoiding the open syscall for each file may make pax faster.
> > 
> > OKs?
> 
> Ping.  I still think this is worth doing.  I've tested and it is also
> measurably faster (25%) if you are excluding a lot of files:
> 
> $ doas time obj/pax -f /dev/null -w -x cpio -s '/^.*$//' /some-path
> 2.65 real 1.95 user 0.73 sys
> $ doas time pax -f /dev/null -w -x cpio -s '/^.*$//' /some-path
> 3.34 real 2.04 user 1.30 sys
> 
> OKs?

Still looking for a review/OK for this.

Thanks,
Jeremy

> 
> Thanks,
> Jeremy
> 
> > 
> > Jeremy
> > 
> > Index: ar_subs.c
> > ===
> > RCS file: /cvs/src/bin/pax/ar_subs.c,v
> > retrieving revision 1.49
> > diff -u -p -r1.49 ar_subs.c
> > --- ar_subs.c   28 Jun 2019 13:34:59 -  1.49
> > +++ ar_subs.c   19 Oct 2021 16:05:31 -
> > @@ -441,6 +441,23 @@ wr_archive(ARCHD *arcn, int is_app)
> > if (hlk && (chk_lnk(arcn) < 0))
> > break;
> >  
> > +   /*
> > +* Modify the name as requested by the user
> > +*/
> > +   if ((res = mod_name(arcn)) < 0) {
> > +   /*
> > +* pax finished, purge link table entry and stop
> > +*/
> > +   purg_lnk(arcn);
> > +   break;
> > +   } else if (res > 0) {
> > +   /*
> > +* skipping file, purge link table entry
> > +*/
> > +   purg_lnk(arcn);
> > +   continue;
> > +   }
> > +
> > if (PAX_IS_REG(arcn->type) || (arcn->type == PAX_HRG)) {
> > /*
> >  * we will have to read this file. by opening it now we
> > @@ -456,20 +473,7 @@ wr_archive(ARCHD *arcn, int is_app)
> > }
> > }
> >  
> > -   /*
> > -* Now modify the name as requested by the user
> > -*/
> > -   if ((res = mod_name(arcn)) < 0) {
> > -   /*
> > -* name modification says to skip this file, close the
> > -* file and purge link table entry
> > -*/
> > -   rdfile_close(arcn, );
> > -   purg_lnk(arcn);
> > -   break;
> > -   }
> > -
> > -   if ((res > 0) || (docrc && (set_crc(arcn, fd) < 0))) {
> > +   if (docrc && (set_crc(arcn, fd) < 0)) {
> > /*
> >  * unable to obtain the crc we need, close the file,
> >  * purge link table entry

Re: ksh: diff to add tab completion for '..'

[Jeremy Evans]

On Fri, Nov 26, 2021 at 5:57 AM Luís Henriques  wrote:

> On Sun, Nov 21, 2021 at 03:36:33PM +, Luís Henriques wrote:
> > Hi!
> >
> > I always found it annoying that, in ksh, doing:
> >
> >   $ ls ..
> >
> > followed by TAB doesn't allow me to list the options (i.e. show
> files/dirs
> > in '..').  I need to do add a trailing '/' to this 'ls' command in order
> > to have the completions listed.
>
> ping.  I'm just wondering if this is something worth pursuing or if people
> aren't simply interested and I should drop it.
>


`ls .` lists files in the current directory starting with `.`.  `ls
./` lists files in the current directory (other than files starting
with `.`, which are hidden). So I think it makes sense that `ls ..`
would list files in the current directory starting with `..`, which is what
it currently does.

Thanks,
Jeremy

Re: pax(1): Don't open files that will be skipped

[Jeremy Evans]

On 10/19 09:34, Jeremy Evans wrote:
> Currently, when creating an archive file with pax(1), pax will attempt
> to open a file even if the file will be skipped due to an -s
> replacement with the empty string. With this change, pax will not
> attempt to open files that it knows will be skipped.
> 
> When doing direct copies to a directory (-rw), pax already skips
> the file before attempting to open it. So this makes the behavior
> more consistent.
> 
> My main reason for this change is to be able to silence the following
> warning when backing up:
> 
>   pax: Unable to open /etc/spwd.db to read: Operation not permitted
> 
> One other possible benefit is if you are skipping a lot of files,
> avoiding the open syscall for each file may make pax faster.
> 
> OKs?

Ping.  I still think this is worth doing.  I've tested and it is also
measurably faster (25%) if you are excluding a lot of files:

$ doas time obj/pax -f /dev/null -w -x cpio -s '/^.*$//' /some-path
2.65 real 1.95 user 0.73 sys
$ doas time pax -f /dev/null -w -x cpio -s '/^.*$//' /some-path
3.34 real 2.04 user 1.30 sys

OKs?

Thanks,
Jeremy

> 
> Jeremy
> 
> Index: ar_subs.c
> ===
> RCS file: /cvs/src/bin/pax/ar_subs.c,v
> retrieving revision 1.49
> diff -u -p -r1.49 ar_subs.c
> --- ar_subs.c 28 Jun 2019 13:34:59 -  1.49
> +++ ar_subs.c 19 Oct 2021 16:05:31 -
> @@ -441,6 +441,23 @@ wr_archive(ARCHD *arcn, int is_app)
>   if (hlk && (chk_lnk(arcn) < 0))
>   break;
>  
> + /*
> +  * Modify the name as requested by the user
> +  */
> + if ((res = mod_name(arcn)) < 0) {
> + /*
> +  * pax finished, purge link table entry and stop
> +  */
> + purg_lnk(arcn);
> + break;
> + } else if (res > 0) {
> + /*
> +  * skipping file, purge link table entry
> +  */
> + purg_lnk(arcn);
> + continue;
> + }
> +
>   if (PAX_IS_REG(arcn->type) || (arcn->type == PAX_HRG)) {
>   /*
>* we will have to read this file. by opening it now we
> @@ -456,20 +473,7 @@ wr_archive(ARCHD *arcn, int is_app)
>   }
>   }
>  
> - /*
> -  * Now modify the name as requested by the user
> -  */
> - if ((res = mod_name(arcn)) < 0) {
> - /*
> -  * name modification says to skip this file, close the
> -  * file and purge link table entry
> -  */
> - rdfile_close(arcn, );
> - purg_lnk(arcn);
> - break;
> - }
> -
> - if ((res > 0) || (docrc && (set_crc(arcn, fd) < 0))) {
> + if (docrc && (set_crc(arcn, fd) < 0)) {
>   /*
>* unable to obtain the crc we need, close the file,
>* purge link table entry

pax(1): Don't open files that will be skipped

[Jeremy Evans]

Currently, when creating an archive file with pax(1), pax will attempt
to open a file even if the file will be skipped due to an -s
replacement with the empty string. With this change, pax will not
attempt to open files that it knows will be skipped.

When doing direct copies to a directory (-rw), pax already skips
the file before attempting to open it. So this makes the behavior
more consistent.

My main reason for this change is to be able to silence the following
warning when backing up:

  pax: Unable to open /etc/spwd.db to read: Operation not permitted

One other possible benefit is if you are skipping a lot of files,
avoiding the open syscall for each file may make pax faster.

OKs?

Jeremy

Index: ar_subs.c
===
RCS file: /cvs/src/bin/pax/ar_subs.c,v
retrieving revision 1.49
diff -u -p -r1.49 ar_subs.c
--- ar_subs.c   28 Jun 2019 13:34:59 -  1.49
+++ ar_subs.c   19 Oct 2021 16:05:31 -
@@ -441,6 +441,23 @@ wr_archive(ARCHD *arcn, int is_app)
if (hlk && (chk_lnk(arcn) < 0))
break;
 
+   /*
+* Modify the name as requested by the user
+*/
+   if ((res = mod_name(arcn)) < 0) {
+   /*
+* pax finished, purge link table entry and stop
+*/
+   purg_lnk(arcn);
+   break;
+   } else if (res > 0) {
+   /*
+* skipping file, purge link table entry
+*/
+   purg_lnk(arcn);
+   continue;
+   }
+
if (PAX_IS_REG(arcn->type) || (arcn->type == PAX_HRG)) {
/*
 * we will have to read this file. by opening it now we
@@ -456,20 +473,7 @@ wr_archive(ARCHD *arcn, int is_app)
}
}
 
-   /*
-* Now modify the name as requested by the user
-*/
-   if ((res = mod_name(arcn)) < 0) {
-   /*
-* name modification says to skip this file, close the
-* file and purge link table entry
-*/
-   rdfile_close(arcn, );
-   purg_lnk(arcn);
-   break;
-   }
-
-   if ((res > 0) || (docrc && (set_crc(arcn, fd) < 0))) {
+   if (docrc && (set_crc(arcn, fd) < 0)) {
/*
 * unable to obtain the crc we need, close the file,
 * purge link table entry

Re: Correctly set SSL error if x509_verify fails

[Jeremy Evans]

On 10/23 09:13, Theo Buehler wrote:
> On Thu, Oct 22, 2020 at 08:44:29PM -0700, Jeremy Evans wrote:
> > I was trying to diagnose a certificate validation failure in Ruby's
> > openssl extension tests with LibreSSL 3.2.2, and it was made more
> > difficult because the verification error type was dropped, resulting
> > in a SSL_R_CERTIFICATE_VERIFY_FAILED error where
> > SSL_get_verify_result returned X509_V_OK.
> 
> Could you perhaps isolate a test case or explain the reproducer in a bit
> more detail? I tried running ruby 2.7's regress from ports but that
> always resulted in a SIGABRT (usually while running test_ftp.rb with or
> without your diff).

Theo,

Sorry about the lack of detail.  This particular issue I found while
trying to get the tests in the truby/openssl repository to pass with
LibreSSL 3.2.2.  In particular, this test:

https://github.com/ruby/openssl/blob/8c6cd23f2a83db1e9f310c8158add459a18b55ad/test/openssl/test_ssl.rb#L918

What tests for hostname verification with wildcards in subjectAltName.
The test uses a subjectAltName with:

DNS:a.example.com,DNS:*.b.example.com,DNS:c*.example.com,DNS:d.*.example.com

Apparently with LibreSSL 3.2.2, using either c*.example.com or
d.*.example.com in the subjectAltName causes all hostnames to be
invalid, even those that should be valid such as a.example.com and
foo.b.example.com.  

RFC 6125 Section 6.4.3 discusses this issue.  Definitely d.*.example.com
is invalid and should not be used (6.4.3.1).  For c*.example.com, it's
up to the client whether to support that (6.4.3.3).  My expectation is
that subjectAltNames with wildcards that LibreSSL does not support would
be ignored as it they were not present.  LibreSSL's actual behavior in
this case is to fail verification for all hostnames. 

I don't have strong feelings about how to treat this particular case.  I
only posted a patch because debugging the issue was more difficult due to
the incorrect error code returned (X509_V_OK).

> With my diff below I once got past this abort and saw this:
> 
> OpenSSL::TestSSL#test_verify_result
> [/usr/ports/pobj/ruby-2.7.2/ruby-2.7.2/test/openssl/utils.rb:279]:
> exceptions on 1 threads:
> <19> expected but was
> <20>.
> 
> Did you see <0> here instead?

That was probably another issue.  There were a lot of changes I needed
to make to the ruby/openssl tests to get them to pass with LibreSSL
3.2.2.  See https://github.com/ruby/openssl/pull/404 for the full pull
request I have to make the tests pass.

I'd love to work with a LibreSSL developer to go through the
ruby/openssl tests where behavior is different for LibreSSL and
determine if the differences are expected or not.  Hopefully at a future
hackathon.

> > I think this patch will fix it.  Compile tested only.  OKs, or is there
> > a better way to fix it?
> 
> This will probably also address the issue with the haproxy test reported
> on the libressl list:
> 
> https://marc.info/?l=libressl=160339671313132=2
> https://github.com/haproxy/haproxy/issues/916
> 
> Regarding your diff, I think setting the error on the store context
> should not be the responsibility of x509_verify()'s caller. After all,
> x509_verify() will likely end up being public API.
> 
> The below diff should have the same effect as yours.

>From my reading of x509_verify_ctx_new_from_xsc, that appears correct
and is certainly a better way to fix the issue than my patch.

Thanks,
Jeremy

Correctly set SSL error if x509_verify fails

[Jeremy Evans]

I was trying to diagnose a certificate validation failure in Ruby's
openssl extension tests with LibreSSL 3.2.2, and it was made more
difficult because the verification error type was dropped, resulting
in a SSL_R_CERTIFICATE_VERIFY_FAILED error where
SSL_get_verify_result returned X509_V_OK.

I think this patch will fix it.  Compile tested only.  OKs, or is there
a better way to fix it?

Thanks,
Jeremy

Index: x509_vfy.c
===
RCS file: /cvs/src/lib/libcrypto/x509/x509_vfy.c,v
retrieving revision 1.81
diff -u -p -r1.81 x509_vfy.c
--- x509_vfy.c  26 Sep 2020 02:06:28 -  1.81
+++ x509_vfy.c  23 Oct 2020 03:34:10 -
@@ -680,6 +680,9 @@ X509_verify_cert(X509_STORE_CTX *ctx)
if ((vctx = x509_verify_ctx_new_from_xsc(ctx, roots)) != NULL) {
ctx->error = X509_V_OK; /* Initialize to OK */
chain_count = x509_verify(vctx, NULL, NULL);
+   if (vctx->error) {
+   ctx->error = vctx->error;
+   }
}
 
sk_X509_pop_free(roots, X509_free);

Add ASN1_dup function prototype back to openssl/asn1.h

[Jeremy Evans]

The ASN1_dup function prototype was removed from libcrypto on October 24
when the major ASN1 cleanup happened.  However, the function itself was
not removed, it is still present in asn1/a_dup.c and the function is
listed in Symbols.list.  This results in issues if there is any code
calling the function.  The prototype is:

void *ASN1_dup(i2d_of_void *i2d, d2i_of_void *d2i, void *x);

If the prototype is not declared in the header, the return
value and all arguments are expected to be of type int, and on I32LP64
platforms, that means that if ASN1_dup is called, it will probably
return an invalid pointer.

The removal of the ASN1_dup prototype causes a segfault when using
the ruby openssl extension (which calls the function). I've tested
that this fixes the issue.

This patch keeps the !LIBRESSL_INTERNAL guard around the defintion,
but that could be removed.

OKs?

Alternatively, if we really do want to delete the function, we
should delete the definition in addition to the prototype, and
remove the function from Symbols.list. In that case, please let me know
what an appopriate replacement would be, so I can inform the ruby
openssl extension developers.

Thanks,
Jeremy

Index: asn1.h
===
RCS file: /cvs/src/lib/libcrypto/asn1/asn1.h,v
retrieving revision 1.52
diff -u -p -r1.52 asn1.h
--- asn1.h  9 Nov 2018 03:42:30 -   1.52
+++ asn1.h  29 Nov 2018 21:37:30 -
@@ -825,6 +825,12 @@ int ASN1_object_size(int constructed, in
 
 void *ASN1_item_dup(const ASN1_ITEM *it, void *x);
 
+#ifndef LIBRESSL_INTERNAL
+
+void *ASN1_dup(i2d_of_void *i2d, d2i_of_void *d2i, void *x);
+
+#endif /* !LIBRESSL_INTERNAL */
+
 void *ASN1_d2i_fp(void *(*xnew)(void), d2i_of_void *d2i, FILE *in, void **x);
 
 #define ASN1_d2i_fp_of(type,xnew,d2i,in,x) \

Re: Show -o and -a in ssh-keygen(1) synopsis

[Jeremy Evans]

On 08/14 04:05, Darren Tucker wrote:
> On 4 August 2018 at 18:15, Jeremy Evans  wrote:
> > I think the documentation for -e should be updated to specify it only
> > exports public keys (assuming I'm reading the code correctly), or
> > ssh-keygen should be updated to write private keys for the RFC4716
> > format if the input file is a private key (since that's what the
> > documentation currently implies).  But that should probably be a
> > separate commit.
> 
> I'll check the history but my recollection was that it was supposed to
> be able to export private keys as RFC4716 format.

OK.
 
> > I also noticed that the -f flag with -A was documented in ssh-keygen(1)
> > but not in usage, so I updated usage to match ssh-keygen(1).
> >
> > OKs for the diff below?

After I sent this email, djm@ made changes in ssh-keygen.1 1.148 and
ssh-keygen.c 1.319 to ignore the -o option and make new format
private keys the default, so I think the previous diff to document
-o is no longer useful.

Here's a new diff to document -a, which I think is still useful. OKs?

Index: ssh-keygen.1
===
RCS file: /cvs/src/usr.bin/ssh/ssh-keygen.1,v
retrieving revision 1.148
diff -u -p -r1.148 ssh-keygen.1
--- ssh-keygen.18 Aug 2018 01:16:01 -   1.148
+++ ssh-keygen.113 Aug 2018 18:29:33 -
@@ -45,6 +45,7 @@
 .Bk -words
 .Nm ssh-keygen
 .Op Fl q
+.Op Fl a Ar rounds
 .Op Fl b Ar bits
 .Op Fl t Cm dsa | ecdsa | ed25519 | rsa
 .Op Fl N Ar new_passphrase
@@ -52,6 +53,7 @@
 .Op Fl f Ar output_keyfile
 .Nm ssh-keygen
 .Fl p
+.Op Fl a Ar rounds
 .Op Fl P Ar old_passphrase
 .Op Fl N Ar new_passphrase
 .Op Fl f Ar keyfile
Index: ssh-keygen.c
===
RCS file: /cvs/src/usr.bin/ssh/ssh-keygen.c,v
retrieving revision 1.319
diff -u -p -r1.319 ssh-keygen.c
--- ssh-keygen.c8 Aug 2018 01:16:01 -   1.319
+++ ssh-keygen.c13 Aug 2018 18:29:34 -
@@ -2282,9 +2282,10 @@ static void
 usage(void)
 {
fprintf(stderr,
-   "usage: ssh-keygen [-q] [-b bits] [-t dsa | ecdsa | ed25519 | 
rsa]\n"
+   "usage: ssh-keygen [-q] [-a rounds] [-b bits] [-t dsa | ecdsa | 
ed25519 | rsa]\n"
"  [-N new_passphrase] [-C comment] [-f 
output_keyfile]\n"
-   "   ssh-keygen -p [-P old_passphrase] [-N new_passphrase] [-f 
keyfile]\n"
+   "   ssh-keygen -p [-a rounds] [-P old_passphrase] [-N 
new_passphrase]\n"
+   "  [-f keyfile]\n"
"   ssh-keygen -i [-m key_format] [-f input_keyfile]\n"
"   ssh-keygen -e [-m key_format] [-f input_keyfile]\n"
"   ssh-keygen -y [-f input_keyfile]\n"

> ok dtucker except for:
> 
> > +.Op Fl oq
> 
> this doesn't look right? -o and -q are distinct orthogonal flags.
> 
> [...]
> > +   "usage: ssh-keygen [-oq] [-a rounds] [-b bits] [-t dsa | ecdsa 
> > | ed25519 | rsa]\n"
> 
> ditto.

Are orthogonal flags without arguments not supposed to be combined?  It
seems most of our man pages combine orthogonal flags without arguments.
Some examples:

ls [-1AaCcdFfgHhikLlmnopqRrSsTtux] [file ...]
col [-bfhx] [-l num]
ex [-FRrSsv] [-c cmd] [-t tag] [-w size] [file ...]

I'm not an expert on our documentation, but it appears the rule is that
arguments are separated if they accept arguments, and combined if they
do not accept arguments.  If that is not accurate, hopefully jmc@ can
correct me.

Thanks,
Jeremy

Re: Show -o and -a in ssh-keygen(1) synopsis

[Jeremy Evans]

On 08/03 09:28, Jeremy Evans wrote:
> The ssh-keygen -o flag wasn't listed in the synopsis, and -a was only
> listed with -T (where it specifies the number of primality tests), not
> for specifying the number of KDF rounds of new-format private key files.
> 
> I only tested creating a new private key and conversion of existing
> keys with -p. I didn't test usage with -i, but I'm assuming that -o
> and -a would also apply there.

jmc@ pointed out that usage should be updated.  I also tried to test the
-i flag, but it appears that -e will only export public keys (even if
given a file containing a private key), and -i only writes private keys
using the PEM_write_*PrivateKey LibreSSL functions, which I don't think
handle the new format.

I checked -A and that also respects -o, so I documented that.  I'm
not sure how much it matters as the host keys -A generates are not
password protected, but maybe there are other reasons to use the
newer format.

I think the documentation for -e should be updated to specify it only
exports public keys (assuming I'm reading the code correctly), or
ssh-keygen should be updated to write private keys for the RFC4716
format if the input file is a private key (since that's what the
documentation currently implies).  But that should probably be a
separate commit.

I also noticed that the -f flag with -A was documented in ssh-keygen(1)
but not in usage, so I updated usage to match ssh-keygen(1).

OKs for the diff below?

Thanks,
Jeremy

Index: ssh-keygen.1
===
RCS file: /cvs/src/usr.bin/ssh/ssh-keygen.1,v
retrieving revision 1.147
diff -u -p -r1.147 ssh-keygen.1
--- ssh-keygen.112 Mar 2018 00:52:01 -  1.147
+++ ssh-keygen.14 Aug 2018 08:04:18 -
@@ -44,7 +44,8 @@
 .Sh SYNOPSIS
 .Bk -words
 .Nm ssh-keygen
-.Op Fl q
+.Op Fl oq 
+.Op Fl a Ar rounds
 .Op Fl b Ar bits
 .Op Fl t Cm dsa | ecdsa | ed25519 | rsa
 .Op Fl N Ar new_passphrase
@@ -52,6 +53,8 @@
 .Op Fl f Ar output_keyfile
 .Nm ssh-keygen
 .Fl p
+.Op Fl o
+.Op Fl a Ar rounds
 .Op Fl P Ar old_passphrase
 .Op Fl N Ar new_passphrase
 .Op Fl f Ar keyfile
@@ -126,6 +129,8 @@
 .Op Fl f Ar input_keyfile
 .Nm ssh-keygen
 .Fl A
+.Op Fl o 
+.Op Fl a Ar rounds
 .Op Fl f Ar prefix_path
 .Nm ssh-keygen
 .Fl k
Index: ssh-keygen.c
===
RCS file: /cvs/src/usr.bin/ssh/ssh-keygen.c,v
retrieving revision 1.318
diff -u -p -r1.318 ssh-keygen.c
--- ssh-keygen.c9 Jul 2018 21:59:10 -   1.318
+++ ssh-keygen.c4 Aug 2018 08:04:18 -
@@ -2282,9 +2282,10 @@ static void
 usage(void)
 {
fprintf(stderr,
-   "usage: ssh-keygen [-q] [-b bits] [-t dsa | ecdsa | ed25519 | 
rsa]\n"
+   "usage: ssh-keygen [-oq] [-a rounds] [-b bits] [-t dsa | ecdsa | 
ed25519 | rsa]\n"
"  [-N new_passphrase] [-C comment] [-f 
output_keyfile]\n"
-   "   ssh-keygen -p [-P old_passphrase] [-N new_passphrase] [-f 
keyfile]\n"
+   "   ssh-keygen -p [-o] [-a rounds] [-P old_passphrase] [-N 
new_passphrase]\n"
+   "  [-f keyfile]\n"
"   ssh-keygen -i [-m key_format] [-f input_keyfile]\n"
"   ssh-keygen -e [-m key_format] [-f input_keyfile]\n"
"   ssh-keygen -y [-f input_keyfile]\n"
@@ -2309,7 +2310,7 @@ usage(void)
"  [-D pkcs11_provider] [-n principals] [-O 
option]\n"
"  [-V validity_interval] [-z serial_number] file 
...\n"
"   ssh-keygen -L [-f input_keyfile]\n"
-   "   ssh-keygen -A\n"
+   "   ssh-keygen -A [-o] [-a rounds] [-f prefix_path]\n"
"   ssh-keygen -k -f krl_file [-u] [-s ca_public] [-z 
version_number]\n"
"  file ...\n"
"   ssh-keygen -Q -f krl_file file ...\n");

Show -o and -a in ssh-keygen(1) synopsis

[Jeremy Evans]

The ssh-keygen -o flag wasn't listed in the synopsis, and -a was only
listed with -T (where it specifies the number of primality tests), not
for specifying the number of KDF rounds of new-format private key files.

I only tested creating a new private key and conversion of existing
keys with -p. I didn't test usage with -i, but I'm assuming that -o
and -a would also apply there.

OK?

Thanks,
Jeremy

Index: ssh-keygen.1
===
RCS file: /cvs/src/usr.bin/ssh/ssh-keygen.1,v
retrieving revision 1.147
diff -u -p -r1.147 ssh-keygen.1
--- ssh-keygen.112 Mar 2018 00:52:01 -  1.147
+++ ssh-keygen.14 Aug 2018 04:20:08 -
@@ -44,7 +44,8 @@
 .Sh SYNOPSIS
 .Bk -words
 .Nm ssh-keygen
-.Op Fl q
+.Op Fl oq 
+.Op Fl a Ar rounds
 .Op Fl b Ar bits
 .Op Fl t Cm dsa | ecdsa | ed25519 | rsa
 .Op Fl N Ar new_passphrase
@@ -52,11 +53,15 @@
 .Op Fl f Ar output_keyfile
 .Nm ssh-keygen
 .Fl p
+.Op Fl o
+.Op Fl a Ar rounds
 .Op Fl P Ar old_passphrase
 .Op Fl N Ar new_passphrase
 .Op Fl f Ar keyfile
 .Nm ssh-keygen
 .Fl i
+.Op Fl o
+.Op Fl a Ar rounds
 .Op Fl m Ar key_format
 .Op Fl f Ar input_keyfile
 .Nm ssh-keygen

Re: 3rd party Xbox 360 USB controller support

[Jeremy Evans]

On 12/07 12:52, Martin Pieuchot wrote:
> On 05/12/15(Sat) 15:22, Christian Heckendorf wrote:
> > The previous thread[1] discussing these controllers includes two
> > patches but they seem to have been merged for the commit in a way
> > that limits support to only Microsoft controllers. 3rd party Xbox 360
> > controllers have their own vendor and product IDs but use the same
> > subclass and protocol as the Microsoft controllers.
> > 
> > Here's a diff based on the first patch that will match controllers
> > and assign the report descriptor more generally using subclass/protocol
> > rather than vendor/product. Is it more correct to create an array
> > of known vendors/products and match against a call to usb_lookup()?
> 
> It's fine since the same check is used in uhidev_use_rdesc().
> 
> Could you try this on a Microsoft controller?  Does it still work?
> Jeremy could you check this out?

It still attaches when using a Microsoft controller:

uhidev2 at uhub2 port 1 configuration 1 interface 0 "\M-)Microsoft Corporation 
Controller" rev 2.00/1.14 addr 3
uhidev2: iclass 255/93
uhid2 at uhidev2: input=20, output=0, feature=0
ugen0 at uhub2 port 1 configuration 1 "\M-)Microsoft Corporation Controller" 
rev 2.00/1.14 addr 3

The SDL joystick program I use for testing works fine with the patch.

Thanks,
Jeremy


> 
> > 
> > [1] http://marc.info/?l=openbsd-tech=138229619410284=2
> > 
> > Thanks,
> > Christian
> > 
> > 
> > Index: uhidev.c
> > ===
> > RCS file: /cvs/src/sys/dev/usb/uhidev.c,v
> > retrieving revision 1.70
> > diff -u -p -r1.70 uhidev.c
> > --- uhidev.c28 Feb 2015 08:42:41 -  1.70
> > +++ uhidev.c5 Dec 2015 20:14:49 -
> > @@ -62,7 +62,10 @@
> >  #ifndef SMALL_KERNEL
> >  /* Replacement report descriptors for devices shipped with broken ones */
> >  #include 
> > -int uhidev_use_rdesc(struct uhidev_softc *, int, int, void **, int *);
> > +int uhidev_use_rdesc(struct uhidev_softc *, usb_interface_descriptor_t *,
> > +   int, int, void **, int *);
> > +#define UISUBCLASS_XBOX360_CONTROLLER 0x5d
> > +#define UIPROTO_XBOX360_GAMEPAD 0x01
> >  #endif /* !SMALL_KERNEL */
> >  
> >  #define DEVNAME(sc)((sc)->sc_dev.dv_xname)
> > @@ -118,10 +121,10 @@ uhidev_match(struct device *parent, void
> > if (id == NULL)
> > return (UMATCH_NONE);
> >  #ifndef SMALL_KERNEL
> > -   if (uaa->vendor == USB_VENDOR_MICROSOFT &&
> > -   uaa->product == USB_PRODUCT_MICROSOFT_XBOX360_CONTROLLER &&
> > -   id->bInterfaceNumber == 0)
> > -   return (UMATCH_VENDOR_PRODUCT);
> > +   if (id->bInterfaceClass == UICLASS_VENDOR &&
> > +   id->bInterfaceSubClass == UISUBCLASS_XBOX360_CONTROLLER &&
> > +   id->bInterfaceProtocol == UIPROTO_XBOX360_GAMEPAD)
> > +   return (UMATCH_IFACECLASS_IFACESUBCLASS_IFACEPROTO);
> >  #endif /* !SMALL_KERNEL */
> > if (id->bInterfaceClass != UICLASS_HID)
> > return (UMATCH_NONE);
> > @@ -191,7 +194,7 @@ uhidev_attach(struct device *parent, str
> > }
> >  
> >  #ifndef SMALL_KERNEL
> > -   if (uhidev_use_rdesc(sc, uaa->vendor, uaa->product, , ))
> > +   if (uhidev_use_rdesc(sc, id, uaa->vendor, uaa->product, , ))
> > return;
> >  #endif /* !SMALL_KERNEL */
> >  
> > @@ -275,8 +278,8 @@ uhidev_attach(struct device *parent, str
> >  
> >  #ifndef SMALL_KERNEL
> >  int
> > -uhidev_use_rdesc(struct uhidev_softc *sc, int vendor, int product,
> > -void **descp, int *sizep)
> > +uhidev_use_rdesc(struct uhidev_softc *sc, usb_interface_descriptor_t *id,
> > +   int vendor, int product, void **descp, int *sizep)
> >  {
> > static uByte reportbuf[] = {2, 2};
> > const void *descptr = NULL;
> > @@ -300,8 +303,9 @@ uhidev_use_rdesc(struct uhidev_softc *sc
> > default:
> > break;
> > }
> > -   } else if (vendor == USB_VENDOR_MICROSOFT &&
> > -   product == USB_PRODUCT_MICROSOFT_XBOX360_CONTROLLER) {
> > +   } else if ((id->bInterfaceClass == UICLASS_VENDOR &&
> > +  id->bInterfaceSubClass == UISUBCLASS_XBOX360_CONTROLLER &&
> > +  id->bInterfaceProtocol == UIPROTO_XBOX360_GAMEPAD)) {
> > /* The Xbox 360 gamepad has no report descriptor. */
> > size = sizeof(uhid_xb360gp_report_descr);
> > descptr = uhid_xb360gp_report_descr;
> > 

OpenBSD::Tame perl wrapper for tame(2)

[Jeremy Evans]

This allows tame(2) to be used from perl.  I almost never write perl and
this is my first time using perl-XS, so apologies if anything is wrong.
I'm not sure how generally useful this will be currently in the base
system, so this may be premature, but if we want it later this should
hopefully give us a good base to start.

Thanks to guenther@, most of the perl-XS specific stuff is taken from his
work on OpenBSD::MkTemp.

Thoughts?

Thanks,
Jeremy

Index: gnu/usr.bin/perl/cpan/OpenBSD-Tame/README
===
RCS file: gnu/usr.bin/perl/cpan/OpenBSD-Tame/README
diff -N gnu/usr.bin/perl/cpan/OpenBSD-Tame/README
--- /dev/null   1 Jan 1970 00:00:00 -
+++ gnu/usr.bin/perl/cpan/OpenBSD-Tame/README   21 Jul 2015 17:01:16 -
@@ -0,0 +1,28 @@
+OpenBSD-Tame version 0.01
+===
+
+A simple wrapper for the tame(2) system call for restricting
+system operations.
+
+INSTALLATION
+
+To install this module type the following:
+
+   perl Makefile.PL
+   make
+   make test
+   make install
+
+DEPENDENCIES
+
+None.
+
+COPYRIGHT AND LICENCE
+
+Copyright (C) 2015 by Jeremy Evans
+
+This library is free software; you can redistribute it and/or modify
+it under the same terms as Perl itself, either Perl version 5.12.2 or,
+at your option, any later version of Perl 5 you may have available.
+
+
Index: gnu/usr.bin/perl/cpan/OpenBSD-Tame/Tame.xs
===
RCS file: gnu/usr.bin/perl/cpan/OpenBSD-Tame/Tame.xs
diff -N gnu/usr.bin/perl/cpan/OpenBSD-Tame/Tame.xs
--- /dev/null   1 Jan 1970 00:00:00 -
+++ gnu/usr.bin/perl/cpan/OpenBSD-Tame/Tame.xs  21 Jul 2015 18:26:52 -
@@ -0,0 +1,39 @@
+#include EXTERN.h
+#include perl.h
+#include XSUB.h
+
+#include sys/tame.h
+
+MODULE = OpenBSD::Tame PACKAGE = OpenBSD::Tame
+
+# result = tame_real(0);
+int
+tame_real(int flags)
+   CODE:
+   RETVAL = tame(flags | TAME_MALLOC);
+   OUTPUT:
+   RETVAL
+
+# %fh = tame_flags_map( )
+SV *
+tame_flags_map()
+   INIT:
+   HV * rh;
+   rh = (HV *)sv_2mortal((SV *)newHV());
+   CODE:
+   hv_store(rh, abort, 5, newSVnv(TAME_ABORT), 0);
+   hv_store(rh, cmsg, 4, newSVnv(TAME_CMSG), 0);
+   hv_store(rh, cpath, 5, newSVnv(TAME_CPATH), 0);
+   hv_store(rh, dns, 3, newSVnv(TAME_DNS), 0);
+   hv_store(rh, getpw, 5, newSVnv(TAME_GETPW), 0);
+   hv_store(rh, inet, 4, newSVnv(TAME_INET), 0);
+   hv_store(rh, ioctl, 5, newSVnv(TAME_IOCTL), 0);
+   hv_store(rh, proc, 4, newSVnv(TAME_PROC), 0);
+   hv_store(rh, rpath, 5, newSVnv(TAME_RPATH), 0);
+   hv_store(rh, rw, 2, newSVnv(TAME_RW), 0);
+   hv_store(rh, tmppath, 7, newSVnv(TAME_TMPPATH), 0);
+   hv_store(rh, unix, 4, newSVnv(TAME_UNIX), 0);
+   hv_store(rh, wpath, 5, newSVnv(TAME_WPATH), 0);
+ RETVAL = newRV((SV *)rh);
+  OUTPUT:
+ RETVAL
Index: gnu/usr.bin/perl/cpan/OpenBSD-Tame/lib/OpenBSD/Tame.pm
===
RCS file: gnu/usr.bin/perl/cpan/OpenBSD-Tame/lib/OpenBSD/Tame.pm
diff -N gnu/usr.bin/perl/cpan/OpenBSD-Tame/lib/OpenBSD/Tame.pm
--- /dev/null   1 Jan 1970 00:00:00 -
+++ gnu/usr.bin/perl/cpan/OpenBSD-Tame/lib/OpenBSD/Tame.pm  21 Jul 2015 
17:52:39 -
@@ -0,0 +1,81 @@
+package OpenBSD::Tame;
+
+use 5.012002;
+use strict;
+use warnings;
+
+use Exporter 'import';
+use Carp;
+
+our @EXPORT_OK = qw( tame );
+our @EXPORT = qw( tame );
+our $VERSION = '0.01';
+
+require XSLoader;
+XSLoader::load('OpenBSD::Tame', $VERSION);
+
+my $flags_map = tame_flags_map();
+
+sub tame
+{
+  my $tame_flags = 0;
+
+  foreach my $flag (@_) {
+$tame_flags |= $flags_map-{$flag}  || croak(invalid tame option: $flag);
+  }
+
+  tame_real($tame_flags) = 0 || croak(attempt to raise tame permissions);
+}
+
+1;
+__END__
+=head1 NAME
+
+OpenBSD::Tame - Perl access to tame()
+
+=head1 SYNOPSIS
+
+  use OpenBSD::Tame;
+
+  tame(abort, rpath)
+  tame(rpath, cpath, wpath)
+  tame(dns, unix, inet)
+
+
+=head1 DESCRIPTION
+
+This module provides access to the tame(2) system call for restricting
+system operations.
+
+tame() must be called with arguments specifying which tame(2) options
+should be allowed.  The only tame(2) option allowed by default is
+TAME_MALLOC, all other tame(2) options must be specified explicitly.
+The available options are: abort, cmsg, dns, getpw, inet, ioctl,
+proc, rpath, rw, tmppath, unix, wpath.
+
+=head2 EXPORT
+
+  tame(abort, rpath)
+
+=head2 Exportable functions
+
+  tame(abort, rpath)
+
+=head1 SEE ALSO
+
+tame(2)
+
+=head1 AUTHOR
+
+Jeremy Evans, Eltjer...@openbsd.orgegt
+
+=head1 COPYRIGHT AND LICENSE
+
+Copyright (C) 2015 by Jeremy Evans
+
+This library is free software; you can redistribute it and/or modify
+it under the same terms as Perl itself, either

tame(1), like nice(1) but for permissions

[Jeremy Evans]

I'm not sure if this makes sense, since tame(2) was designed to operate
on processes after they have already been initialized, and this would
set the allowed operations before initializing the process.

It's a fairly simple change to get the basics working as shown here,
but it's currently not very useful as more complex programs generally
can't start even if given all current tame(2) permissions.

I mostly did this to get more experience working in the kernel, not
because I think it is a good idea, but I welcome feedback all the same.

First the manual, followed by the code, then the kernel and tame(2)
manpage diff.

Thanks,
Jeremy

   TAME(1) General Commands ManualTAME(1) 

   NAME

   tame - restrict system operations for process

   SYNOPSIS

   tame [-aCcdghIiRSptuw] utility [argument ...]  

   DESCRIPTION

   tame restricts system operations using the tame(2) system call, then
   executes the utility with the given arguments. If the utility attempts to
   perform an operation which was not permitted, it will be killed by the
   system with SIGKILL.
   By default, tame restricts almost all system operations for the executed
   process, allowing only the execution of processes (TAME_EXEC), use of
   stdio (TAME_STDIO), and reading the file system (TAME_RPATH). All flags
   with the exception of -h, -R, and -S allow additional system operations.
   The options that allow additional system operations are as follows, with
   the tame(2) option that they enable:

   -a
   TAME_ABORT

   -C
   TAME_CMSG

   -c
   TAME_CPATH

   -d
   TAME_DNS

   -g
   TAME_GETPW

   -I
   TAME_IOCTL

   -i
   TAME_INET

   -p
   TAME_PROC

   -t
   TAME_TMPPATH

   -u
   TAME_UNIX

   -w
   TAME_WPATH

   The following options restrict system operations that are allowed by
   default:

   -R
   TAME_RW

   -r
   TAME_RPATH

   The -h option displays the usage.
   If the -r option is used, utilty must be the full path to the utility,
   tame will no longer search the PATH to find it, as it will have already
   restricted the permissions that would allow that.

   EXIT STATUS

   The tame utility exits with one of the following values:

   125
   An error occurred.

   126
   The utility was not found or could not be invoked.

   Otherwise, the exit status of tame shall be that of utility.

   EXAMPLES

   Only allow overwriting files that already exist, do not allow creating new
   files:

 $ tame -w cp from to

   SEE ALSO

   tame(2)

   HISTORY

   The tame() system call appeared in OpenBSD 5.8.

   $Mdocdate: $ OpenBSD 5.8   

tame.c:

/*  $OpenBSD: $ */

/*
 * Copyright (c) 2015 Jeremy Evans jer...@openbsd.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.
 */

#include err.h
#include stdio.h
#include stdlib.h
#include sys/tame.h
#include unistd.h

__dead void usage(int);

int
main(int argc, char *argv[])
{
char ch;
char *file;
/* TAME_RPATH needed to find process to execute
 * TAME_EXEC needed to execute the process
 * TAME_STDIO needed by almost all processes
 */
int tame_flags = TAME_RPATH | TAME_STDIO | TAME_EXEC;

while ((ch = getopt(argc, argv, aCcdghIiRrptuw)) != -1)
switch (ch) {
case 'a':
tame_flags |= TAME_ABORT;
break;
case 'C':
tame_flags |= TAME_CMSG;
break;
case 'c':
tame_flags |= TAME_CPATH;
break;
case 'd':
tame_flags |= TAME_DNS;
break;
case 'g':
tame_flags |= TAME_GETPW;
break;
case 'I':
tame_flags |= TAME_IOCTL;
break;
case 'i':
tame_flags |= TAME_INET;
break;
case 'p':
tame_flags |= TAME_PROC;
break

Re: tame(1), like nice(1) but for permissions

[Jeremy Evans]

On 07/20 09:36, Nicholas Marriott wrote:
 Hi
 
 I'm not sure I can think of many uses for this, tame is not something
 you are intended to just apply blindly, do you have any use cases?

Well, there is the example in the man page. :) But no, currently it's
not very useful, as more complex programs such as sh(1) or perl(1) won't
start even if given all current tame(2) permissions.  For this to be
useful, you'd need to give TAME_EXEC additional permissions such that
you could do `tame -tp sh`, and get a sh that could execute processes,
but not write to the file system or do network access.

And like I said originally, I'm not sure this is a good idea.  It was
just a way for me to get more kernel experience.

 I think the -aCcdghIiRSptuw approach is a bad idea and it would be
 better to do it with named flags like -o abort,cmsg,cpath. Maybe take a
 look at getsubopt(3), although I don't know if that API is in vogue
 anymore.

If this is worthy of more work, the command line options can certainly
be changed.  I just used getopt(3) since it seemed like the easiest
way to handle it.
 
 Also adding TAME_EXEC seems like a different change entirely?

Without TAME_EXEC, you can't call execve(2) to create another process.
There currently isn't a tame(2) permission that allows execing, one had
to be added.

Thanks,
Jeremy

Re: Fix socketpair(2) handling of unix datagram sockets using cloexec/nonblock

[Jeremy Evans]

On 07/16 05:05, Philip Guenther wrote:
 On Thu, Jul 16, 2015 at 4:54 PM, Jeremy Evans jer...@openbsd.org wrote:
  Fix socketpair(2) on Unix datagram sockets that use SOCK_CLOEXEC or
  SOCK_NONBLOCK.
 
  This fixes a failure in the ruby test suite.
 
  OK?
 
 No, that'll have false positives on SOCK_RAW sockets.  You need to
 mask things, perhaps a diff like this:

This works and is definitely more correct.  OK jeremy@

Thanks,
Jeremy

 
 --- sys/socket.h21 Jan 2015 02:23:14 -  1.87
 +++ sys/socket.h17 Jul 2015 00:03:48 -
 @@ -68,6 +68,9 @@ typedef   __sa_family_t   sa_family_t;/* so
  #defineSOCK_RAW3   /* raw-protocol interface */
  #defineSOCK_RDM4   /* reliably-delivered message 
 */
  #defineSOCK_SEQPACKET  5   /* sequenced packet stream */
 +#ifdef _KERNEL
 +#defineSOCK_TYPE_MASK  0x000F  /* mask that covers the above 
 */
 +#endif
 
  /*
   * Socket creation flags
 Index: kern/uipc_syscalls.c
 ===
 RCS file: /data/src/openbsd/src/sys/kern/uipc_syscalls.c,v
 retrieving revision 1.102
 diff -u -p -r1.102 uipc_syscalls.c
 --- kern/uipc_syscalls.c21 May 2015 13:35:15 -  1.102
 +++ kern/uipc_syscalls.c17 Jul 2015 00:04:02 -
 @@ -403,7 +403,7 @@ sys_socketpair(struct proc *p, void *v,
 }
 if ((error = soconnect2(so1, so2)) != 0)
 goto free4;
 -   if (SCARG(uap, type) == SOCK_DGRAM) {
 +   if ((SCARG(uap, type)  SOCK_TYPE_MASK) == SOCK_DGRAM) {
 /*
  * Datagram socket connection is asymmetric.
  */

Fix socketpair(2) handling of unix datagram sockets using cloexec/nonblock

[Jeremy Evans]

Fix socketpair(2) on Unix datagram sockets that use SOCK_CLOEXEC or
SOCK_NONBLOCK.

This fixes a failure in the ruby test suite.

OK?

Thanks,
Jeremy

Index: kern/uipc_syscalls.c
===
RCS file: /cvs/src/sys/kern/uipc_syscalls.c,v
retrieving revision 1.97
diff -u -p -r1.97 uipc_syscalls.c
--- kern/uipc_syscalls.c13 Dec 2014 21:05:33 -  1.97
+++ kern/uipc_syscalls.c16 Jul 2015 23:29:40 -
@@ -406,7 +406,7 @@ sys_socketpair(struct proc *p, void *v, 
}
if ((error = soconnect2(so1, so2)) != 0)
goto free4;
-   if (SCARG(uap, type) == SOCK_DGRAM) {
+   if (SCARG(uap, type)  SOCK_DGRAM) {
/*
 * Datagram socket connection is asymmetric.
 */

Re: Add Xbox 360 Controller USB support

[Jeremy Evans]

On 10/20 03:52, Martin Pieuchot wrote:
 Hi Jeremy,
 
 On 18/10/13(Fri) 09:11, Jeremy Evans wrote:
  This was originally submitted by Joe Gidi in November 2010, based on a
  FreeBSD commit by Ed Schouten from back in December 2005.  See
  http://marc.info/?l=openbsd-techm=128924886803756w=2 for previous
  thread.  The only comment was from tedu@, that SMALL_KERNEL should be
  added, which I've done but I'm not sure correctly.
  
  Tested on amd64 using usbhidctl and some SDL-based emulators (mednafen,
  snes9x-gtk, desmume).  The controller works fine except that the
  directional pad is not processed as a hat but as a series of buttons
  (that usbhidctl sees but SDL seems not to recognize).  Also, the bottom
  L/R triggers are axes instead of buttons.
  
  I've built a release with this, and checked that the amd64 floppy
  still fits.
 
 Good stuff, some comments below.

Martin,

Thanks for responding.  Here's a new diff that incorporates most of your
suggestions.  Unfortunately, using the existing quirks infrastructure
doesn't work correctly, because the quirks API is based on device
manufacturer and vendor, and matching on that instead of interface
subclass and protocol appears to not work correctly.  When you plug in
the controller with my original diff and the first diff below, you get:

uhidev0 at uhub3 port 3 configuration 1 interface 0 \M-)Microsoft Corporation 
Controller rev 2.00/1.10 addr 2
uhidev0: iclass 255/93
uhid0 at uhidev0: input=20, output=0, feature=0
ugen0 at uhub3 port 3 configuration 1 \M-)Microsoft Corporation Controller 
rev 2.00/1.10 addr 2
 
As you can see, the controller shows up as both a uhid and a ugen.

With the second diff below, when you plugin the controller, it
attaches as follows:

uhidev0 at uhub3 port 3 configuration 1 interface 0 \M-)Microsoft Corporation 
Controller rev 2.00/1.10 addr 2
uhidev0: iclass 255/93
uhid0 at uhidev0: input=20, output=0, feature=0
uhidev1 at uhub3 port 3 configuration 1 interface 1 \M-)Microsoft Corporation 
Controller rev 2.00/1.10 addr 2
uhidev1: iclass 255/93
uhid1 at uhidev1: input=20, output=0, feature=0
uhidev2 at uhub3 port 3 configuration 1 interface 2 \M-)Microsoft Corporation 
Controller rev 2.00/1.10 addr 2
uhidev2: iclass 255/93
uhid2 at uhidev2: input=20, output=0, feature=0
uhidev3 at uhub3 port 3 configuration 1 interface 3 \M-)Microsoft Corporation 
Controller rev 2.00/1.10 addr 2
uhidev3: no input interrupt endpoint

In this case uhid0 appears to work, but uhid1 and uhid2 do not:

$ usbhidctl -f 1 -a 
usbhidctl: USB_GET_REPORT (probably not supported by device): Input/output error

Anyway, let me know what you think about the first diff, or if I'm doing
something wrong with the second diff that is causing it to attach
multiple times.

Thanks,
Jeremy

Working diff:

Index: uhidev.c
===
RCS file: /cvs/src/sys/dev/usb/uhidev.c,v
retrieving revision 1.46
diff -u -p -r1.46 uhidev.c
--- uhidev.c20 Sep 2013 15:34:50 -  1.46
+++ uhidev.c20 Oct 2013 18:23:37 -
@@ -55,8 +55,13 @@
 
 #include dev/usb/uhidev.h
 
-/* Report descriptor for broken Wacom Graphire */
+#ifndef SMALL_KERNEL
+/* Replacement report descriptors for devices shipped with broken ones */
 #include dev/usb/ugraphire_rdesc.h
+#include dev/usb/uxb360gp_rdesc.h
+#defineUISUBCLASS_XBOX360_CONTROLLER   0x5d
+#defineUIPROTO_XBOX360_GAMEPAD 0x01
+#endif /* !SMALL_KERNEL */
 
 #ifdef UHIDEV_DEBUG
 #define DPRINTF(x) do { if (uhidevdebug) printf x; } while (0)
@@ -99,8 +104,17 @@ uhidev_match(struct device *parent, void
if (uaa-iface == NULL)
return (UMATCH_NONE);
id = usbd_get_interface_descriptor(uaa-iface);
-   if (id == NULL || id-bInterfaceClass != UICLASS_HID)
-   return (UMATCH_NONE);
+if (id == NULL)
+return (UMATCH_NONE);
+if  (id-bInterfaceClass != UICLASS_HID) {
+#ifndef SMALL_KERNEL
+/* The Xbox 360 gamepad doesn't use the HID class. */
+if (id-bInterfaceClass != UICLASS_VENDOR ||
+id-bInterfaceSubClass != UISUBCLASS_XBOX360_CONTROLLER ||
+id-bInterfaceProtocol != UIPROTO_XBOX360_GAMEPAD)
+#endif /* !SMALL_KERNEL */
+return (UMATCH_NONE);
+}
if (usbd_get_quirks(uaa-device)-uq_flags  UQ_BAD_HID)
return (UMATCH_NONE);
if (uaa-matchlvl)
@@ -180,6 +194,7 @@ uhidev_attach(struct device *parent, str
 
/* XXX need to extend this */
descptr = NULL;
+#ifndef SMALL_KERNEL
if (uaa-vendor == USB_VENDOR_WACOM) {
static uByte reportbuf[] = {2, 2, 2};
 
@@ -200,7 +215,15 @@ uhidev_attach(struct device *parent, str
/* Keep descriptor */
break;
}
+   } else if (id-bInterfaceClass == UICLASS_VENDOR 
+   id-bInterfaceSubClass

Add Xbox 360 Controller USB support

[Jeremy Evans]

This was originally submitted by Joe Gidi in November 2010, based on a
FreeBSD commit by Ed Schouten from back in December 2005.  See
http://marc.info/?l=openbsd-techm=128924886803756w=2 for previous
thread.  The only comment was from tedu@, that SMALL_KERNEL should be
added, which I've done but I'm not sure correctly.

Tested on amd64 using usbhidctl and some SDL-based emulators (mednafen,
snes9x-gtk, desmume).  The controller works fine except that the
directional pad is not processed as a hat but as a series of buttons
(that usbhidctl sees but SDL seems not to recognize).  Also, the bottom
L/R triggers are axes instead of buttons.

I've built a release with this, and checked that the amd64 floppy
still fits.

OKs?

Thanks,
Jeremy

Index: uhidev.c
===
RCS file: /cvs/src/sys/dev/usb/uhidev.c,v
retrieving revision 1.46
diff -u -p -r1.46 uhidev.c
--- uhidev.c20 Sep 2013 15:34:50 -  1.46
+++ uhidev.c17 Oct 2013 23:32:55 -
@@ -55,8 +55,11 @@
 
 #include dev/usb/uhidev.h
 
-/* Report descriptor for broken Wacom Graphire */
+/* Replacement report descriptors for devices shipped with broken ones */
 #include dev/usb/ugraphire_rdesc.h
+#ifndef SMALL_KERNEL
+#include dev/usb/uxb360gp_rdesc.h
+#endif /* !SMALL_KERNEL */
 
 #ifdef UHIDEV_DEBUG
 #define DPRINTF(x) do { if (uhidevdebug) printf x; } while (0)
@@ -99,8 +102,17 @@ uhidev_match(struct device *parent, void
if (uaa-iface == NULL)
return (UMATCH_NONE);
id = usbd_get_interface_descriptor(uaa-iface);
-   if (id == NULL || id-bInterfaceClass != UICLASS_HID)
-   return (UMATCH_NONE);
+if (id == NULL)
+return (UMATCH_NONE);
+if  (id-bInterfaceClass != UICLASS_HID) {
+#ifndef SMALL_KERNEL
+/* The Xbox 360 gamepad doesn't use the HID class. */
+if (id-bInterfaceClass != UICLASS_VENDOR ||
+id-bInterfaceSubClass != UISUBCLASS_XBOX360_CONTROLLER ||
+id-bInterfaceProtocol != UIPROTO_XBOX360_GAMEPAD)
+#endif /* !SMALL_KERNEL */
+return (UMATCH_NONE);
+}
if (usbd_get_quirks(uaa-device)-uq_flags  UQ_BAD_HID)
return (UMATCH_NONE);
if (uaa-matchlvl)
@@ -200,7 +212,16 @@ uhidev_attach(struct device *parent, str
/* Keep descriptor */
break;
}
+#ifndef SMALL_KERNEL
+   } else if (id-bInterfaceClass == UICLASS_VENDOR 
+   id-bInterfaceSubClass == UISUBCLASS_XBOX360_CONTROLLER 
+   id-bInterfaceProtocol == UIPROTO_XBOX360_GAMEPAD) {
+   /* The Xbox 360 gamepad has no report descriptor. */
+   size = sizeof uhid_xb360gp_report_descr;
+   descptr = uhid_xb360gp_report_descr;
+#endif /* !SMALL_KERNEL */
}
+
 
if (descptr) {
desc = malloc(size, M_USBDEV, M_NOWAIT);
Index: usb.h
===
RCS file: /cvs/src/sys/dev/usb/usb.h,v
retrieving revision 1.44
diff -u -p -r1.44 usb.h
--- usb.h   17 Apr 2013 11:53:10 -  1.44
+++ usb.h   17 Oct 2013 18:11:59 -
@@ -491,7 +491,8 @@ typedef struct {
 #define  UIPROTO_IRDA  0
 
 #define UICLASS_VENDOR 0xff
-
+#define  UISUBCLASS_XBOX360_CONTROLLER 0x5d
+#define  UIPROTO_XBOX360_GAMEPAD   0x01
 
 #define USB_HUB_MAX_DEPTH 5
 
Index: uxb360gp_rdesc.h
===
RCS file: uxb360gp_rdesc.h
diff -N uxb360gp_rdesc.h
--- /dev/null   1 Jan 1970 00:00:00 -
+++ uxb360gp_rdesc.h17 Oct 2013 18:23:56 -
@@ -0,0 +1,126 @@
+/*-
+ * Copyright (c) 2005 Ed Schouten e...@freebsd.org
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *notice, this list of conditions and the following disclaimer in the
+ *documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND
+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED.  IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE
+ * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
+ * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
+ * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
+ * LIABILITY, OR TORT 

Make install(1) not delete files when installing to same directory

[Jeremy Evans]

Currently, install(1) deletes files when you are installing them to the
same directory, if you only specify the directory name and not the full
path:

  $ echo foo  foo; install foo .; cat foo
  install: ./foo: Bad address
  cat: foo: No such file or directory

This goes against the manual page, which states: The install utility
attempts to prevent moving a file onto itself.

The check for the same inode number is only done when the full path is
given:

  $ echo foo  foo; install foo ./foo; cat foo
  install: foo and ./foo are the same file
  foo

The diff below attempts to handle the situation correctly if only the
directory name is given:

  $ echo foo  foo; install foo .; cat foo
  install: foo and ./foo are the same file
  foo

I'm not sure if the approach I'm using in the diff is correct. If a
directory name is given, I'm checking the inode for that directory
against the directory of the source, and exiting with an error if
they are the same. Would it be better to check if a file with the same
base name exists in the destination directory, and then check the inode
of that file against the file name?

If multiple from files are given, this will exit without processing all
of the files.  Is it better to just warn for each and change the exit
code at the end, so that all files are processed?  I can do that, but
I'm not sure which behavior is actually desired.  I think either
behavior is better than the current behavior of deleting the file.

This fixes an issue that causes the databases/ruby-ldap port to not
build if it loses a race.

Jeremy

Index: xinstall.c
===
RCS file: /cvs/src/usr.bin/xinstall/xinstall.c,v
retrieving revision 1.51
diff -u -p -r1.51 xinstall.c
--- xinstall.c  11 Apr 2012 14:19:35 -  1.51
+++ xinstall.c  11 Sep 2012 23:09:03 -
@@ -40,6 +40,7 @@
 #include errno.h
 #include fcntl.h
 #include grp.h
+#include libgen.h
 #include paths.h
 #include pwd.h
 #include stdio.h
@@ -167,8 +168,14 @@ main(int argc, char *argv[])
 
no_target = stat(to_name = argv[argc - 1], to_sb);
if (!no_target  S_ISDIR(to_sb.st_mode)) {
-   for (; *argv != to_name; ++argv)
+   for (; *argv != to_name; ++argv) {
+   if (stat(dirname(*argv), from_sb))
+   err(EX_OSERR, %s, dirname(*argv));
+   if (to_sb.st_dev == from_sb.st_dev 
+   to_sb.st_ino == from_sb.st_ino)
+   errx(EX_USAGE, %s and %s/%s are the same 
file, *argv, to_name, basename(*argv));
install(*argv, to_name, fset, iflags | DIRECTORY);
+   }
exit(EX_OK);
/* NOTREACHED */
}

Re: nc -U -u (Unix datagram socket support)

[Jeremy Evans]

jmc@ and I discussed these man page changes. He's OK with this patch,
but would like another network developer to approve.  So, looking for
OKs.

Jeremy

Index: nc.1
===
RCS file: /cvs/src/usr.bin/nc/nc.1,v
retrieving revision 1.56
diff -u -p -r1.56 nc.1
--- nc.18 Jan 2011 00:44:19 -   1.56
+++ nc.19 Jan 2011 21:10:44 -
@@ -40,7 +40,7 @@
 .Op Fl O Ar length
 .Op Fl P Ar proxy_username
 .Op Fl p Ar source_port
-.Op Fl s Ar source_ip_address
+.Op Fl s Ar source
 .Op Fl T Ar ToS
 .Op Fl V Ar rtable
 .Op Fl w Ar timeout
@@ -49,7 +49,7 @@
 .Fl x Ar proxy_address Ns Oo : Ns
 .Ar port Oc
 .Xc Oc
-.Op Ar hostname
+.Op Ar destination
 .Op Ar port
 .Ek
 .Sh DESCRIPTION
@@ -57,8 +57,10 @@ The
 .Nm
 (or
 .Nm netcat )
-utility is used for just about anything under the sun involving TCP
-or UDP.
+utility is used for just about anything under the sun involving TCP,
+UDP, or
+.Ux Ns -domain
+sockets.
 It can open TCP connections, send UDP packets, listen on arbitrary
 TCP and UDP ports, do port scanning, and deal with both IPv4 and
 IPv6.
@@ -153,7 +155,7 @@ instead of sequentially within a range o
 assigns them.
 .It Fl S
 Enables the RFC 2385 TCP MD5 signature option.
-.It Fl s Ar source_ip_address
+.It Fl s Ar source
 Specifies the IP of the interface which is used to send the packets.
 For
 .Ux Ns -domain
@@ -234,7 +236,7 @@ If the protocol is not specified, SOCKS 
 Requests that
 .Nm
 should connect to
-.Ar hostname
+.Ar destination
 using a proxy at
 .Ar proxy_address
 and
@@ -252,16 +254,22 @@ It is an error to use this option in con
 option.
 .El
 .Pp
-.Ar hostname
+.Ar destination
 can be a numerical IP address or a symbolic hostname
 (unless the
 .Fl n
 option is given).
-In general, a hostname must be specified,
+In general, a destination must be specified,
 unless the
 .Fl l
 option is given
 (in which case the local host is used).
+For
+.Ux Ns -domain
+sockets, a destination is required and is the socket path to connect to
+(or listen on if the
+.Fl l
+option is given).
 .Pp
 .Ar port
 can be a single integer or a range of ports.
@@ -270,8 +278,7 @@ In general,
 a destination port must be specified,
 unless the
 .Fl U
-option is given
-(in which case a socket must be specified).
+option is given.
 .Sh CLIENT/SERVER MODEL
 It is quite simple to build a very basic client/server model using
 .Nm .
@@ -404,7 +411,7 @@ IP for the local end of the connection:
 .Pp
 Create and listen on a
 .Ux Ns -domain
-socket:
+stream socket:
 .Pp
 .Dl $ nc -lU /var/tmp/dsocket
 .Pp
Index: netcat.c
===
RCS file: /cvs/src/usr.bin/nc/netcat.c,v
retrieving revision 1.99
diff -u -p -r1.99 netcat.c
--- netcat.c8 Jan 2011 00:44:19 -   1.99
+++ netcat.c9 Jan 2011 21:10:45 -
@@ -930,9 +930,9 @@ usage(int ret)
 {
fprintf(stderr,
usage: nc [-46DdhklnrStUuvz] [-I length] [-i interval] [-O 
length]\n
-   \t  [-P proxy_username] [-p source_port] [-s source_ip_address] 
[-T ToS]\n
+   \t  [-P proxy_username] [-p source_port] [-s source] [-T ToS]\n
\t  [-V rtable] [-w timeout] [-X proxy_protocol]\n
-   \t  [-x proxy_address[:port]] [hostname] [port]\n);
+   \t  [-x proxy_address[:port]] [destination] [port]\n);
if (ret)
exit(1);
 }

Re: nc -U -u (Unix datagram socket support)

[Jeremy Evans]

On 01/07 09:31, Nicholas Marriott wrote:
 On Thu, Jan 06, 2011 at 03:32:17PM -0800, Jeremy Evans wrote:
  This patch adds unix datagram socket support to nc(1).  It's basically
  the same patch I sent last June (see
  http://marc.info/?l=openbsd-techm=127627296925965w=2), but updated
  for -current.
  
  Tested on amd64.  Doesn't appear to cause any regressions to existing
  support, tested with unix stream and IP stream and datagram sockets.
  Looking for OKs.
  
  Jeremy
 
 Hmm, ISTR I meant to look at this ages ago but it got lost, sorry.
 
 So you are overloading -u to mean UDP without -l and datagram with -l?
 I guess this makes sense, but we need man page changes?
 
If you mean -U instead of -l, yes.  -u without -U is IP UDP, -u with -U
is unix datagram.  The man page doesn't say you can't use -u and -U
together, and it doesn't say that -U means unix stream sockets, though
that is currently all that -U supports.  However, I think that making
some clarifications to the man page would helpful.

Responses inline and new diff at the end.

  Index: netcat.c
  ===
  RCS file: /cvs/src/usr.bin/nc/netcat.c,v
  retrieving revision 1.98
  diff -u -p -r1.98 netcat.c
  --- netcat.c3 Jul 2010 04:44:51 -   1.98
  +++ netcat.c6 Jan 2011 21:48:04 -
  @@ -89,6 +89,7 @@ u_int rtableid;
   int timeout = -1;
   int family = AF_UNSPEC;
   char *portlist[PORT_MAX+1];
  +char *unix_dg_tmp_socket;
   
   void   atelnet(int, unsigned char *, unsigned int);
   void   build_ports(char *);
  @@ -99,6 +100,7 @@ int  remote_connect(const char *, const c
   intsocks_connect(const char *, const char *, struct addrinfo,
  const char *, const char *, struct addrinfo, int, const char *);
   intudptest(int);
  +intunix_bind(char *);
   intunix_connect(char *);
   intunix_listen(char *);
   void   set_common_sockopts(int);
  @@ -241,8 +243,6 @@ main(int argc, char *argv[])
   
  /* Cruft to make sure options are clean, and used properly. */
  if (argv[0]  !argv[1]  family == AF_UNIX) {
  -   if (uflag)
  -   errx(1, cannot use -u and -U);
  host = argv[0];
  uport = NULL;
  } else if (argv[0]  !argv[1]) {
  @@ -265,6 +265,18 @@ main(int argc, char *argv[])
  if (!lflag  kflag)
  errx(1, must use -l with -k);
   
  +   /* Get name of temporary socket for unix datagram client */
  +   if ((family == AF_UNIX)  uflag  !lflag) {
  +   if(pflag) {
  +   unix_dg_tmp_socket = pflag;
  +   } else {
  +   if((unix_dg_tmp_socket = (char *)malloc(19)) == NULL)
  +   errx(1, not enough memory);
 
 Style nit: space between if and (.
 
OK. My diff was bad about that, so I fixed the other cases as well.

  +   strlcpy(unix_dg_tmp_socket, /tmp/nc.XX, 19);
  +   mktemp(unix_dg_tmp_socket);
 
 What if this fails?

You're right, a failure of mktemp should definitely be checked.
 
  +   }
  +   }
  +
  /* Initialize addrinfo structure. */
  if (family != AF_UNIX) {
  memset(hints, 0, sizeof(struct addrinfo));
  @@ -307,8 +319,12 @@ main(int argc, char *argv[])
  int connfd;
  ret = 0;
   
  -   if (family == AF_UNIX)
  -   s = unix_listen(host);
  +   if (family == AF_UNIX) {
  +   if(uflag)
  +   s = unix_bind(host);
  +   else
  +   s = unix_listen(host);
  +   }
   
  /* Allow only one connection at a time, but stay alive. */
  for (;;) {
  @@ -337,17 +353,19 @@ main(int argc, char *argv[])
  if (rv  0)
  err(1, connect);
   
  -   connfd = s;
  +   readwrite(s);
  } else {
  len = sizeof(cliaddr);
  connfd = accept(s, (struct sockaddr *)cliaddr,
  len);
  +   readwrite(connfd);
  +   close(connfd);
  }
   
  -   readwrite(connfd);
  -   close(connfd);
  if (family != AF_UNIX)
  close(s);
  +   else if (uflag)
  +   connect(s, NULL, 0);
 
 Likewise.
 
Correct, this should be checked as well.

   
  if (!kflag)
  break;
  @@ -361,6 +379,8 @@ main(int argc, char *argv[])
  } else
  ret = 1;
   
  +   if(uflag)
  +   unlink(unix_dg_tmp_socket);
 
 Shouldn't this have the same condition as above?
 
The condition when

Re: nc -U -u (Unix datagram socket support)

[Jeremy Evans]

On 01/07 06:21, Nicholas Marriott wrote:
 On Fri, Jan 07, 2011 at 08:48:20AM -0800, Jeremy Evans wrote:
  On 01/07 09:31, Nicholas Marriott wrote:
   On Thu, Jan 06, 2011 at 03:32:17PM -0800, Jeremy Evans wrote:
This patch adds unix datagram socket support to nc(1).  It's basically
the same patch I sent last June (see
http://marc.info/?l=openbsd-techm=127627296925965w=2), but updated
for -current.

Tested on amd64.  Doesn't appear to cause any regressions to existing
support, tested with unix stream and IP stream and datagram sockets.
Looking for OKs.

Jeremy
   
   Hmm, ISTR I meant to look at this ages ago but it got lost, sorry.
   
   So you are overloading -u to mean UDP without -l and datagram with -l?
   I guess this makes sense, but we need man page changes?
   
  If you mean -U instead of -l, yes.  -u without -U is IP UDP, -u with -U
  is unix datagram.  The man page doesn't say you can't use -u and -U
  together, and it doesn't say that -U means unix stream sockets, though
  that is currently all that -U supports.  However, I think that making
  some clarifications to the man page would helpful.
 
 Yes, sorry, I meant -U.
 
 This mostly looks fine to me, a few comments:
 
 I don't much like using mktemp at all. How about just plain requiring -s
 with -Uu?

It's annoying to the user to have to specify it when they won't usually
care.  If you are worried about security, guenther@ thought this usage
was secure: http://marc.info/?l=openbsd-techm=120299257422367w=2

I'd prefer we always use a random socket over forcing the user to
specify one, but I thinking giving the user choice is best, just like
we give them choice of sending address in the IP case.

 Do you actually hit the ENOBUFS condition in atomicio.c?

Yes.  That's the only reason I knew to add it.

Jeremy

Re: nc -U -u (Unix datagram socket support)

[Jeremy Evans]

On 01/07 07:31, Nicholas Marriott wrote:
 On Fri, Jan 07, 2011 at 10:52:18AM -0800, Jeremy Evans wrote:
  On 01/07 06:21, Nicholas Marriott wrote:
 Two further minor comments:
 
 - Can the mktemp buffer be on the stack rather than malloc()d?
 
Sure.

 - I think the man page should mention it creates a file in /tmp (or
   mktemp).
 
Makes sense.

OK to commit this diff, which contains the above changes?:

Index: atomicio.c
===
RCS file: /cvs/src/usr.bin/nc/atomicio.c,v
retrieving revision 1.9
diff -u -p -r1.9 atomicio.c
--- atomicio.c  7 Sep 2007 14:50:44 -   1.9
+++ atomicio.c  6 Jan 2011 21:48:04 -
@@ -53,7 +53,7 @@ atomicio(ssize_t (*f) (int, void *, size
case -1:
if (errno == EINTR)
continue;
-   if (errno == EAGAIN) {
+   if ((errno == EAGAIN) || (errno == ENOBUFS)) {
(void)poll(pfd, 1, -1);
continue;
}
Index: nc.1
===
RCS file: /cvs/src/usr.bin/nc/nc.1,v
retrieving revision 1.55
diff -u -p -r1.55 nc.1
--- nc.125 Jul 2010 07:51:39 -  1.55
+++ nc.17 Jan 2011 20:08:35 -
@@ -155,6 +155,10 @@ assigns them.
 Enables the RFC 2385 TCP MD5 signature option.
 .It Fl s Ar source_ip_address
 Specifies the IP of the interface which is used to send the packets.
+For 
+.Ux Ns -domain
+datagram sockets, specifies the local temporary socket file
+to create and use so that datagrams can be received.
 It is an error to use this option in conjunction with the
 .Fl l
 option.
@@ -179,6 +183,15 @@ Specifies to use
 sockets.
 .It Fl u
 Use UDP instead of the default option of TCP.
+For
+.Ux Ns -domain
+sockets, use a datagram socket instead of a stream socket.
+If a 
+.Ux Ns -domain
+socket is used, a temporary receiving socket is created in /tmp unless
+you specify one with the
+.Fl s
+flag.
 .It Fl V Ar rtable
 Set the routing table to be used.
 The default is 0.
Index: netcat.c
===
RCS file: /cvs/src/usr.bin/nc/netcat.c,v
retrieving revision 1.98
diff -u -p -r1.98 netcat.c
--- netcat.c3 Jul 2010 04:44:51 -   1.98
+++ netcat.c7 Jan 2011 20:03:37 -
@@ -62,6 +62,7 @@
 
 #define PORT_MAX   65535
 #define PORT_MAX_LEN   6
+#define UNIX_DG_TMP_SOCKET_SIZE19
 
 /* Command Line Options */
 intdflag;  /* detached, no stdin */
@@ -89,6 +90,7 @@ u_int rtableid;
 int timeout = -1;
 int family = AF_UNSPEC;
 char *portlist[PORT_MAX+1];
+char *unix_dg_tmp_socket;
 
 void   atelnet(int, unsigned char *, unsigned int);
 void   build_ports(char *);
@@ -99,6 +101,7 @@ int  remote_connect(const char *, const c
 intsocks_connect(const char *, const char *, struct addrinfo,
const char *, const char *, struct addrinfo, int, const char *);
 intudptest(int);
+intunix_bind(char *);
 intunix_connect(char *);
 intunix_listen(char *);
 void   set_common_sockopts(int);
@@ -117,6 +120,7 @@ main(int argc, char *argv[])
char *proxy;
const char *errstr, *proxyhost = , *proxyport = NULL;
struct addrinfo proxyhints;
+   char unix_dg_tmp_socket_buf[UNIX_DG_TMP_SOCKET_SIZE];
 
ret = 1;
s = 0;
@@ -241,8 +245,6 @@ main(int argc, char *argv[])
 
/* Cruft to make sure options are clean, and used properly. */
if (argv[0]  !argv[1]  family == AF_UNIX) {
-   if (uflag)
-   errx(1, cannot use -u and -U);
host = argv[0];
uport = NULL;
} else if (argv[0]  !argv[1]) {
@@ -265,6 +267,19 @@ main(int argc, char *argv[])
if (!lflag  kflag)
errx(1, must use -l with -k);
 
+   /* Get name of temporary socket for unix datagram client */
+   if ((family == AF_UNIX)  uflag  !lflag) {
+   if (sflag) {
+   unix_dg_tmp_socket = sflag;
+   } else {
+   strlcpy(unix_dg_tmp_socket_buf, /tmp/nc.XX,
+   UNIX_DG_TMP_SOCKET_SIZE);
+   if (mktemp(unix_dg_tmp_socket_buf) == NULL)
+   err(1, mktemp);
+   unix_dg_tmp_socket = unix_dg_tmp_socket_buf;
+   }
+   }
+
/* Initialize addrinfo structure. */
if (family != AF_UNIX) {
memset(hints, 0, sizeof(struct addrinfo));
@@ -307,8 +322,12 @@ main(int argc, char *argv[])
int connfd;
ret = 0;
 
-   if (family == AF_UNIX)
-   s = unix_listen(host);
+   if (family == AF_UNIX) {
+   if (uflag)
+   s = unix_bind(host

nc -U -u (Unix datagram socket support)

[Jeremy Evans]

This patch adds unix datagram socket support to nc(1).  It's basically
the same patch I sent last June (see
http://marc.info/?l=openbsd-techm=127627296925965w=2), but updated
for -current.

Tested on amd64.  Doesn't appear to cause any regressions to existing
support, tested with unix stream and IP stream and datagram sockets.
Looking for OKs.

Jeremy

Index: atomicio.c
===
RCS file: /cvs/src/usr.bin/nc/atomicio.c,v
retrieving revision 1.9
diff -u -p -r1.9 atomicio.c
--- atomicio.c  7 Sep 2007 14:50:44 -   1.9
+++ atomicio.c  6 Jan 2011 21:48:04 -
@@ -53,7 +53,7 @@ atomicio(ssize_t (*f) (int, void *, size
case -1:
if (errno == EINTR)
continue;
-   if (errno == EAGAIN) {
+   if ((errno == EAGAIN) || (errno == ENOBUFS)) {
(void)poll(pfd, 1, -1);
continue;
}
Index: netcat.c
===
RCS file: /cvs/src/usr.bin/nc/netcat.c,v
retrieving revision 1.98
diff -u -p -r1.98 netcat.c
--- netcat.c3 Jul 2010 04:44:51 -   1.98
+++ netcat.c6 Jan 2011 21:48:04 -
@@ -89,6 +89,7 @@ u_int rtableid;
 int timeout = -1;
 int family = AF_UNSPEC;
 char *portlist[PORT_MAX+1];
+char *unix_dg_tmp_socket;
 
 void   atelnet(int, unsigned char *, unsigned int);
 void   build_ports(char *);
@@ -99,6 +100,7 @@ int  remote_connect(const char *, const c
 intsocks_connect(const char *, const char *, struct addrinfo,
const char *, const char *, struct addrinfo, int, const char *);
 intudptest(int);
+intunix_bind(char *);
 intunix_connect(char *);
 intunix_listen(char *);
 void   set_common_sockopts(int);
@@ -241,8 +243,6 @@ main(int argc, char *argv[])
 
/* Cruft to make sure options are clean, and used properly. */
if (argv[0]  !argv[1]  family == AF_UNIX) {
-   if (uflag)
-   errx(1, cannot use -u and -U);
host = argv[0];
uport = NULL;
} else if (argv[0]  !argv[1]) {
@@ -265,6 +265,18 @@ main(int argc, char *argv[])
if (!lflag  kflag)
errx(1, must use -l with -k);
 
+   /* Get name of temporary socket for unix datagram client */
+   if ((family == AF_UNIX)  uflag  !lflag) {
+   if(pflag) {
+   unix_dg_tmp_socket = pflag;
+   } else {
+   if((unix_dg_tmp_socket = (char *)malloc(19)) == NULL)
+   errx(1, not enough memory);
+   strlcpy(unix_dg_tmp_socket, /tmp/nc.XX, 19);
+   mktemp(unix_dg_tmp_socket);
+   }
+   }
+
/* Initialize addrinfo structure. */
if (family != AF_UNIX) {
memset(hints, 0, sizeof(struct addrinfo));
@@ -307,8 +319,12 @@ main(int argc, char *argv[])
int connfd;
ret = 0;
 
-   if (family == AF_UNIX)
-   s = unix_listen(host);
+   if (family == AF_UNIX) {
+   if(uflag)
+   s = unix_bind(host);
+   else
+   s = unix_listen(host);
+   }
 
/* Allow only one connection at a time, but stay alive. */
for (;;) {
@@ -337,17 +353,19 @@ main(int argc, char *argv[])
if (rv  0)
err(1, connect);
 
-   connfd = s;
+   readwrite(s);
} else {
len = sizeof(cliaddr);
connfd = accept(s, (struct sockaddr *)cliaddr,
len);
+   readwrite(connfd);
+   close(connfd);
}
 
-   readwrite(connfd);
-   close(connfd);
if (family != AF_UNIX)
close(s);
+   else if (uflag)
+   connect(s, NULL, 0);
 
if (!kflag)
break;
@@ -361,6 +379,8 @@ main(int argc, char *argv[])
} else
ret = 1;
 
+   if(uflag)
+   unlink(unix_dg_tmp_socket);
exit(ret);
 
} else {
@@ -421,18 +441,19 @@ main(int argc, char *argv[])
 }
 
 /*
- * unix_connect()
- * Returns a socket connected to a local unix socket. Returns -1 on failure.
+ * unix_bind()
+ * Returns a unix socket bound to the given path
  */
 int
-unix_connect(char *path)
+unix_bind(char *path)
 {
struct 

Re: nc -U -u (Unix datagram socket support)

[Jeremy Evans]

On 01/06 07:07, Ted Unangst wrote:
 On Thu, Jan 6, 2011 at 6:32 PM, Jeremy Evans jer...@openbsd.org wrote:
  This patch adds unix datagram socket support to nc(1). ?It's basically
  the same patch I sent last June (see
  http://marc.info/?l=openbsd-techm=127627296925965w=2), but updated
  for -current.
 
  Tested on amd64. ?Doesn't appear to cause any regressions to existing
  support, tested with unix stream and IP stream and datagram sockets.
  Looking for OKs.
 
 Why is the socket name specified via -p for datagrams or possibly
 random?  Shouldn't this be just like the stream unix code?

I believe that for unix stream sockets, you don't need to have a sending
socket file created, while you do for datagram sockets, as otherwise you
can't have a bidirectional connection.

I have no problem with always using a random sending socket file, and
ignoring -p in the unix datagram case, if you think that is best.
Thinking more about it, -s would be more appropriate than -p anyway if
you did want to specify the source socket.

Jeremy

 
  + ? ? ? /* Get name of temporary socket for unix datagram client */
  + ? ? ? if ((family == AF_UNIX)  uflag  !lflag) {
  + ? ? ? ? ? ? ? if(pflag) {
  + ? ? ? ? ? ? ? ? ? ? ? unix_dg_tmp_socket = pflag;
  + ? ? ? ? ? ? ? } else {
  + ? ? ? ? ? ? ? ? ? ? ? if((unix_dg_tmp_socket = (char *)malloc(19)) == 
  NULL)
  + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? errx(1, not enough memory);
  + ? ? ? ? ? ? ? ? ? ? ? strlcpy(unix_dg_tmp_socket, /tmp/nc.XX, 
  19);
  + ? ? ? ? ? ? ? ? ? ? ? mktemp(unix_dg_tmp_socket);
  + ? ? ? ? ? ? ? }
  + ? ? ? }
  +

Re: nc -U -u (Unix datagram socket support)

[Jeremy Evans]

On 01/06 08:56, Ted Unangst wrote:
 On Thu, Jan 6, 2011 at 7:19 PM, Jeremy Evans jer...@openbsd.org wrote:
  I believe that for unix stream sockets, you don't need to have a sending
  socket file created, while you do for datagram sockets, as otherwise you
  can't have a bidirectional connection.
 
  I have no problem with always using a random sending socket file, and
  ignoring -p in the unix datagram case, if you think that is best.
  Thinking more about it, -s would be more appropriate than -p anyway if
  you did want to specify the source socket.
 
 The part that's confusing me is, who is at the other end of this tmp
 socket?  How do they know about it?

With a stream socket where you are acting as a client, you don't need a
temporary socket created, because a stream socket is bidirectional. If
you send a datagram to a server listening on a unix datagram socket,
there is no way for the server program to respond to you unless the
sending program is also bound to a socket.

So with this patch, if you are operating nc in client mode and
sending a datagram to a unix datagram socket, nc binds a temporary
unix datagram socket so that you can receive responses.

guenther@ agreed back in 2008 that it was necessary to create a
temporary socket if you want bidirectional communication, see
http://marc.info/?l=openbsd-techm=120299257422367w=2.  He also
mentioned that it might be beneficial to implement two modes, one
providing bidirectional and one providing unidirectional, but I have
not implemented that (yet).

Jeremy

[PATCH] nc -U -u

[Jeremy Evans]

It's been about two years since I last sent a version of this
patch, which allows you to use unix datagram sockets with nc(1).
Currently nc(1) supports IP datagram sockets and unix stream
sockets, but not unix datagram sockets, and this limitation is
is not mentioned in the man page.

One main improvement with this patch over previous versions is
that when listing on a unix datagram socket with the -k flag, it
handles connections from separate sending sockets correctly.

I currently have to use the net/socat port for something that
nc(1) with this patch can now handle (controlling the
audio/aqualung port, which listens on a unix datagram socket).

For emails with previous versions of this patch, see:
http://marc.info/?t=11950906644r=1w=2

Is there any interest in modifying nc(1) to support unix datagram
sockets?  Other than a single response from guenther@ a couple
years ago, which had good recommendations that are now included in
this patch, I haven't had any feedback on this.

Thanks,
Jeremy

Index: atomicio.c
===
RCS file: /cvs/src/usr.bin/nc/atomicio.c,v
retrieving revision 1.9
diff -N -u atomicio.c
--- atomicio.c  7 Sep 2007 14:50:44 -   1.9
+++ atomicio.c  11 Jun 2010 16:04:45 -
@@ -53,7 +53,7 @@
case -1:
if (errno == EINTR)
continue;
-   if (errno == EAGAIN) {
+   if ((errno == EAGAIN) || (errno == ENOBUFS)) {
(void)poll(pfd, 1, -1);
continue;
}
Index: netcat.c
===
RCS file: /cvs/src/usr.bin/nc/netcat.c,v
retrieving revision 1.97
diff -N -u netcat.c
--- netcat.c20 Apr 2010 07:28:28 -  1.97
+++ netcat.c11 Jun 2010 16:04:45 -
@@ -89,6 +89,7 @@
 int timeout = -1;
 int family = AF_UNSPEC;
 char *portlist[PORT_MAX+1];
+char *unix_dg_tmp_socket;
 
 void   atelnet(int, unsigned char *, unsigned int);
 void   build_ports(char *);
@@ -99,6 +100,7 @@
 intsocks_connect(const char *, const char *, struct addrinfo,
const char *, const char *, struct addrinfo, int, const char *);
 intudptest(int);
+intunix_bind(char *);
 intunix_connect(char *);
 intunix_listen(char *);
 void   set_common_sockopts(int);
@@ -241,8 +243,6 @@
 
/* Cruft to make sure options are clean, and used properly. */
if (argv[0]  !argv[1]  family == AF_UNIX) {
-   if (uflag)
-   errx(1, cannot use -u and -U);
host = argv[0];
uport = NULL;
} else if (argv[0]  !argv[1]) {
@@ -265,6 +265,18 @@
if (!lflag  kflag)
errx(1, must use -l with -k);
 
+   /* Get name of temporary socket for unix datagram client */
+   if ((family == AF_UNIX)  uflag  !lflag) {
+   if(pflag) {
+   unix_dg_tmp_socket = pflag;
+   } else {
+   if((unix_dg_tmp_socket = (char *)malloc(19)) == NULL)
+   errx(1, not enough memory);
+   strlcpy(unix_dg_tmp_socket, /tmp/nc.XX, 19);
+   mktemp(unix_dg_tmp_socket);
+   }
+   }
+
/* Initialize addrinfo structure. */
if (family != AF_UNIX) {
memset(hints, 0, sizeof(struct addrinfo));
@@ -307,8 +319,12 @@
int connfd;
ret = 0;
 
-   if (family == AF_UNIX)
-   s = unix_listen(host);
+   if (family == AF_UNIX) {
+   if(uflag)
+   s = unix_bind(host);
+   else
+   s = unix_listen(host);
+   }
 
/* Allow only one connection at a time, but stay alive. */
for (;;) {
@@ -337,17 +353,19 @@
if (rv  0)
err(1, connect);
 
-   connfd = s;
+   readwrite(s);
} else {
len = sizeof(cliaddr);
connfd = accept(s, (struct sockaddr *)cliaddr,
len);
+   readwrite(connfd);
+   close(connfd);
}
 
-   readwrite(connfd);
-   close(connfd);
if (family != AF_UNIX)
close(s);
+   else if (uflag)
+   connect(s, NULL, 0);
 
if (!kflag)
break;
@@ -361,6 +379,8 @@
} else
ret = 1;
 
+