Bug#935650: netcat-openbsd: valid arguments disallowed

2019-08-26 Thread astian
astian:
> astian:
>>> AFAICT it's a no-op for stream sockets; might make sense to error
>>> out unless ‘-u’ is set.
>>
>> Right, so it could be something like:
>>
>> -} else if (argc == 1 && !pflag && !sflag) {
>> +} else if (argc == 1 && !pflag && (!sflag || (family == AF_UNIX && 
>> uflag))) {
>>
>> Or using the previously suggested diff and adding a dedicated check in
>> the if group below:
>>
>>  if (family == AF_UNIX) {
>>  ...
>> +if (sflag && !uflag)
>> +errx(1, "for unix sockets -s is only valid together "
>> +"with -u");
> 
> Actually there are still issues here.  I'll write tomorrow.

Issue is that this still leaves '-s' as a no-op if '-l' is also given.
Upstream always disallows combining those two, but here an added !lflag
is needed, for example:

-   } else if (argc == 1 && !pflag && !sflag) {
+   } else if (argc == 1 && !pflag && (!sflag || (family == AF_UNIX && 
uflag && !lflag))) {

Cheers.



Bug#935650: netcat-openbsd: valid arguments disallowed

2019-08-25 Thread astian
astian:
>> AFAICT it's a no-op for stream sockets; might make sense to error
>> out unless ‘-u’ is set.
> 
> Right, so it could be something like:
> 
> - } else if (argc == 1 && !pflag && !sflag) {
> + } else if (argc == 1 && !pflag && (!sflag || (family == AF_UNIX && 
> uflag))) {
> 
> Or using the previously suggested diff and adding a dedicated check in
> the if group below:
> 
>   if (family == AF_UNIX) {
>  ...
> + if (sflag && !uflag)
> + errx(1, "for unix sockets -s is only valid together "
> + "with -u");

Actually there are still issues here.  I'll write tomorrow.

Cheers.



Bug#935650: netcat-openbsd: valid arguments disallowed

2019-08-25 Thread astian
Guilhem Moulin:
> On Sat, 24 Aug 2019 at 20:25:00 +, astian wrote:
>> Looking at the patch I don't trust this is the only behaviour change.  I
>> don't understand why this divergence from upstream was introduced and I
>> wish it was reverted altogether.
>
> The patch was added to support the following calls for consistency with
> netcat-traditional:
>
> nc -l -s 127.0.0.1 -p 12345 ## listen on AF_INET socket 
> INADDR_LOOPBACK:12345
> nc -l -p 12345  ## listen on AF_INET socket ADDR_ANY:12345
>
> It'd be a regression to stop supporting these forms, or any of the
> others from https://bugs.debian.org/897020#17 .

Ah, blessed compatibility.

>> But if that's not possible below is a patch that you can fold into the
>> existing one.
>> […]
>> -} else if (argc == 1 && !pflag && !sflag) {
>> +} else if (argc == 1 && !pflag && (!sflag || family == AF_UNIX)) {
>
> In combination with ‘-U’ the ‘-s’ option only makes sense for datagrams,
> no?

Yes, sorry.  (Now I remember that I was looking into making stream
UNIX-domain clients also bind() and accept an optional '-s' so that the
peer address is reported correctly in verbose mode.  This is an upstream
quirk/bug.)

> AFAICT it's a no-op for stream sockets; might make sense to error
> out unless ‘-u’ is set.

Right, so it could be something like:

-   } else if (argc == 1 && !pflag && !sflag) {
+   } else if (argc == 1 && !pflag && (!sflag || (family == AF_UNIX && 
uflag))) {

Or using the previously suggested diff and adding a dedicated check in
the if group below:

if (family == AF_UNIX) {
 ...
+   if (sflag && !uflag)
+   errx(1, "for unix sockets -s is only valid together "
+   "with -u");

> As for your other patches, I'm personally not keen to further diverge
> with upstream (especially when it comes to adding new flags/options) —

We concur.

Cheers.



Bug#935650: netcat-openbsd: valid arguments disallowed

2019-08-24 Thread Guilhem Moulin
Control: retitle -1 netcat-openbsd: Unable to specify client socket for 
UNIX-domain datagram sockets
Control: found -1 1.187-1
Control: found -1 1.195-2

Hi,

On Sat, 24 Aug 2019 at 20:25:00 +, astian wrote:
> Looking at the patch I don't trust this is the only behaviour change.  I
> don't understand why this divergence from upstream was introduced and I
> wish it was reverted altogether.

The patch was added to support the following calls for consistency with
netcat-traditional:

nc -l -s 127.0.0.1 -p 12345 ## listen on AF_INET socket 
INADDR_LOOPBACK:12345
nc -l -p 12345  ## listen on AF_INET socket ADDR_ANY:12345

It'd be a regression to stop supporting these forms, or any of the
others from https://bugs.debian.org/897020#17 .

> But if that's not possible below is a patch that you can fold into the
> existing one.
> […]
> - } else if (argc == 1 && !pflag && !sflag) {
> + } else if (argc == 1 && !pflag && (!sflag || family == AF_UNIX)) {

In combination with ‘-U’ the ‘-s’ option only makes sense for datagrams,
no?  AFAICT it's a no-op for stream sockets; might make sense to error
out unless ‘-u’ is set.

As for your other patches, I'm personally not keen to further diverge
with upstream (especially when it comes to adding new flags/options) —
but hopefully they'll eventually pick up your fixes or address your
concerns ;-)

Thanks for the reports and patches!
-- 
Guilhem.


signature.asc
Description: PGP signature


Bug#935650: netcat-openbsd: valid arguments disallowed

2019-08-24 Thread astian
Package: netcat-openbsd
Version: 1.203-1
Severity: normal
Control: tags -1 + patch

Dear Maintainer,

The debian-specific patch "use-flags-to-specify-listen-address.patch"
disallows the usage of "-s" in some valid cases.  For example, the
following should connect to the unix socket "target" and use "foo" as
its client socket:

  $ nc -U -s foo target
  usage: nc ...

Looking at the patch I don't trust this is the only behaviour change.  I
don't understand why this divergence from upstream was introduced and I
wish it was reverted altogether.  But if that's not possible below is a
patch that you can fold into the existing one.

By the way, when I was looking at netcat 2 weeks ago I found several
other issues.  I wrote to the upstream ML and was planning to report the
bugs to the Debian BTS afterwards but it seems upstream is not very
interested because there was no reply in 10+ days.  Anyway, in case you
want to fix any of these, of the upstream issues the ones that are most
relevant for the Debian "fork" are the following:
  - https://marc.info/?l=openbsd-bugs=156551733219405=2 (the
non-kernel part, obviously)
  - https://marc.info/?l=openbsd-bugs=156568806424551=2
  - https://marc.info/?l=openbsd-bugs=156551644519192=2
  - https://marc.info/?l=openbsd-bugs=156551645219193=2
  - https://marc.info/?l=openbsd-bugs=156551644219191=2

There's also this one, but here Debian unfortunately diverged so there's
less of a compelling reason to fix it:
  - https://marc.info/?l=openbsd-bugs=156551640819187=2

The remaining one is OpenBSD-specific.

In my Debian box I've been running a custom netcat package with all of
the above patched (some "provisionally", some with patches that differ
slightly for Linux), so feel free to ask if you want the patches.


Thanks.

-- System Information:
Debian Release: bullseye/sid
  APT prefers unstable-debug
  APT policy: (500, 'unstable-debug'), (500, 'unstable'), (500, 'testing'), 
(500, 'stable')
Architecture: amd64 (x86_64)

Kernel: Linux 4.19.0-5-amd64 (SMP w/4 CPU cores)
Locale: LANG=en_GB.UTF-8, LC_CTYPE=en_GB.UTF-8 (charmap=UTF-8), 
LANGUAGE=en_GB:en (charmap=UTF-8)
Shell: /bin/sh linked to /bin/dash
Init: systemd (via /run/systemd/system)
LSM: AppArmor: enabled

Versions of packages netcat-openbsd depends on:
ii  libbsd0  0.9.1-2
ii  libc62.28-10

netcat-openbsd recommends no packages.

netcat-openbsd suggests no packages.

-- no debconf information

--- a/netcat.c
+++ b/netcat.c
@@ -511,7 +511,7 @@ main(int argc, char *argv[])
if (argc == 0 && lflag) {
uport = 
host = sflag;
-   } else if (argc == 1 && !pflag && !sflag) {
+   } else if (argc == 1 && !pflag && (!sflag || family == AF_UNIX)) {
if (family == AF_UNIX) {
host = argv[0];
uport = NULL;