Re: Questions about syspatch(8) mtree(8) behaviour

2019-09-02 Thread Antoine Jacoutot
On Mon, Sep 02, 2019 at 08:58:01PM +0200, Hiltjo Posthuma wrote:
> On Mon, Sep 02, 2019 at 12:07:59PM -0600, Theo de Raadt wrote:
> > Hiltjo Posthuma  wrote:
> > 
> > > Hi,
> > > 
> > > I have three questions regarding a behaviour of syspatch(8) with mtree(8).
> > > 
> > > 1. I noticed when applying patches it resets some permissions of new, but 
> > > also of
> > > existing directories on the system using mtree(8).
> > > 
> > > In the shellscript syspatch(8) there is a function:
> > > 
> > > trap_handler():
> > > # in case a patch added a new directory (install -D)
> > > if [[ -n ${_PATCHES} ]]; then
> > > mtree -qdef /etc/mtree/4.4BSD.dist -p / -U >/dev/null
> > > [[ -f /var/sysmerge/xetc.tgz ]] &&
> > > mtree -qdef /etc/mtree/BSD.x11.dist -p / -U 
> > > >/dev/null
> > > fi
> > > 
> > > Here the comment says: "in case a patch added a new directory (install 
> > > -D)".
> > > This is true, but it also applies to existing directories and resets
> > > permissions, ownership, etc.
> > > 
> > > A real-world example: on my system after applying syspatch this changed
> > > permissions of an existing directory and a daemon (mysqld) failed to 
> > > start,
> > > because it could not access a UNIX domain socket file in the www chroot.
> > 
> > A very long mail without being 100% PRECISE.
> > 
> > > Is this intended? If so should this behaviour perhaps get documented in 
> > > the man
> > > page? I can write a patch if so.
> > 
> > Intentional.  As a general rule if you change a system component, you own
> > all the pieces.
> > 
> > But I guess you did chmod a+wrxt / or something, right?  I have to assume
> > so, because your mail is not PRECISE.
> > 
> 
> In this particular case it was the directory /var/www/run. The default
> permissions are as specified in /etc/mtree/4.4BSD.dist:
> 
>   run type=dir uname=root gname=daemon mode=755
> 
> I changed it from 755 to 775 (still root:daemon) so both mysqld and PHP could
> access the UNIX domain socket in the www chroot (/var/www).

Why don't you do what the mariadb readme advises?

chrooted daemons and MariaDB socket
===

For external program running under a chroot(8) to be able to access the
MariaDB server without using a network connection, the socket must be
placed inside the chroot.

e.g. httpd(8) or nginx(8): connecting to MariaDB from PHP
-
Create a directory for the MariaDB socket:

# install -d -m 0711 -o _mysql -g _mysql /var/www/var/run/mysql

Adjust ${SYSCONFDIR}/my.cnf to use the socket in the chroot - this
applies to both client and server processes:

[client-server]
socket = /var/www/var/run/mysql/mysql.sock


-- 
Antoine



Re: vmd(8): remove unused error code

2019-09-02 Thread Mike Larkin
On Mon, Sep 02, 2019 at 12:43:18AM +0200, Tobias Heider wrote:
> The VMD_DISK_INVALID error code is no longer used since
> https://marc.info/?l=openbsd-cvs&m=153147762830175 and
> can be removed.
> 
> Ok?
> 
> Index: vmctl/vmctl.c
> ===
> RCS file: /cvs/src/usr.sbin/vmctl/vmctl.c,v
> retrieving revision 1.69
> diff -u -p -u -r1.69 vmctl.c
> --- vmctl/vmctl.c 22 May 2019 16:19:21 -  1.69
> +++ vmctl/vmctl.c 1 Sep 2019 22:06:32 -
> @@ -235,11 +235,6 @@ vm_start_complete(struct imsg *imsg, int
>   warnx("could not open disk image(s)");
>   *ret = ENOENT;
>   break;
> - case VMD_DISK_INVALID:
> - warnx("specified disk image(s) are "
> - "not regular files");
> - *ret = ENOENT;
> - break;
>   case VMD_CDROM_MISSING:
>   warnx("could not find specified iso image");
>   *ret = ENOENT;
> Index: vmd/vmd.h
> ===
> RCS file: /cvs/src/usr.sbin/vmd/vmd.h,v
> retrieving revision 1.95
> diff -u -p -u -r1.95 vmd.h
> --- vmd/vmd.h 17 Jul 2019 05:51:07 -  1.95
> +++ vmd/vmd.h 1 Sep 2019 22:06:32 -
> @@ -68,7 +68,6 @@
>  /* vmd -> vmctl error codes */
>  #define VMD_BIOS_MISSING 1001
>  #define VMD_DISK_MISSING 1002
> -#define VMD_DISK_INVALID 1003
>  #define VMD_VM_STOP_INVALID  1004
>  #define VMD_CDROM_MISSING1005
>  #define VMD_CDROM_INVALID1006
> 

ok mlarkin

You might want to comment that 1003 was VMD_DISK_INVALID in case someone
later is wondering why we skipped 1002 to 1004. Like we do with syscalls.

-ml



libedit: Does not run input command in vi mode

2019-09-02 Thread Masato Asou
Does not run input command by vi editor with vi mode.

I do the following:

1. set vi mode.
   $ echo "bind -v" > ~/.editrc

2. launch /usr/bin/ftp command.
   $ ftp

3. launch vi editor with ESC + v.
   ftp> ESC + v

4. input "help" in vi editor.
   i + help + ESC + :wq

5. then 'help' command does not run.

I fix this problem with following patch. This fix is come from NetBSD
lib/libedit/vi.c 1.46 and 1.47.

ok?

Index: vi.c
===
RCS file: /cvs/src/lib/libedit/vi.c,v
retrieving revision 1.27
diff -u -p -r1.27 vi.c
--- vi.c3 Sep 2019 02:28:25 -   1.27
+++ vi.c3 Sep 2019 05:34:31 -
@@ -1058,12 +1058,12 @@ vi_histedit(EditLine *el, wint_t c __att
while (waitpid(pid, &status, 0) != pid)
continue;
lseek(fd, (off_t)0, SEEK_SET);
-   st = read(fd, cp, TMP_BUFSIZ);
+   st = read(fd, cp, TMP_BUFSIZ - 1);
if (st > 0) {
-   len = (size_t)(el->el_line.lastchar -
-   el->el_line.buffer);
+   cp[st] = '\0';
+   len = (size_t)(el->el_line.limit - el->el_line.buffer);
len = mbstowcs(el->el_line.buffer, cp, len);
-   if (len > 0 && el->el_line.buffer[len -1] == '\n')
+   if (len > 0 && el->el_line.buffer[len - 1] == '\n')
--len;
}
else

--
ASOU Masato



Re: bnxt: Support MSI-X

2019-09-02 Thread Jonathan Matthew
On Mon, Sep 02, 2019 at 04:47:32PM +0200, Stefan Fritsch wrote:
> Tested with a BCM57412
> 
> OK?

ok jmatthew@

> 
> diff --git a/sys/dev/pci/if_bnxt.c b/sys/dev/pci/if_bnxt.c
> --- a/sys/dev/pci/if_bnxt.c
> +++ b/sys/dev/pci/if_bnxt.c
> @@ -102,6 +102,7 @@
>  #define BNXT_FLAG_NPAR  0x0002
>  #define BNXT_FLAG_WOL_CAP   0x0004
>  #define BNXT_FLAG_SHORT_CMD 0x0008
> +#define BNXT_FLAG_MSIX  0x0010
>  
>  /* NVRam stuff has a five minute timeout */
>  #define BNXT_NVM_TIMEO   (5 * 60 * 1000)
> @@ -507,7 +508,9 @@ bnxt_attach(struct device *parent, struct device *self, 
> void *aux)
>* devices advertise msi support, but there's no way to tell a
>* completion queue to use msi mode, only legacy or msi-x.
>*/
> - if (/*pci_intr_map_msi(pa, &ih) != 0 && */ pci_intr_map(pa, &ih) != 0) {
> + if (pci_intr_map_msix(pa, 0, &ih) == 0) {
> + sc->sc_flags |= BNXT_FLAG_MSIX;
> + } else if (pci_intr_map(pa, &ih) != 0) {
>   printf(": unable to map interrupt\n");
>   goto free_resp;
>   }
> @@ -2658,7 +2661,9 @@ bnxt_hwrm_ring_alloc(struct bnxt_softc *softc, uint8_t 
> type,
>   req.logical_id = htole16(ring->id);
>   req.cmpl_ring_id = htole16(cmpl_ring_id);
>   req.queue_id = htole16(softc->sc_q_info[0].id);
> - req.int_mode = 0;
> + req.int_mode = (softc->sc_flags & BNXT_FLAG_MSIX) ?
> + HWRM_RING_ALLOC_INPUT_INT_MODE_MSIX :
> + HWRM_RING_ALLOC_INPUT_INT_MODE_LEGACY;
>   BNXT_HWRM_LOCK(softc);
>   rc = _hwrm_send_message(softc, &req, sizeof(req));
>   if (rc)
> 



em: Fix potential endless loop in attach

2019-09-02 Thread Stefan Fritsch
If the NIC is in some error state during device attach (seen on a i219LM 
when em_read_phy_reg_ex() returns at "MDI Error"), it can happen that we 
loop endlessly because the loop variable is modified again down in the 
call stack:

em_match_gig_phy() -> em_read_phy_reg() -> em_access_phy_reg_hv() -> sets 
hw->phy_addr

Fixing the underlying issue needs some more debugging. But we can use a 
separate variable to avoid the hang. The attach will then error out with 
"Hardware Initialization Failed".

OK?


diff --git a/sys/dev/pci/if_em_hw.c b/sys/dev/pci/if_em_hw.c
index 77adfe5707b..ccbc620739d 100644
--- a/sys/dev/pci/if_em_hw.c
+++ b/sys/dev/pci/if_em_hw.c
@@ -5810,7 +5810,12 @@ em_detect_gig_phy(struct em_hw *hw)
}
 
/* Read the PHY ID Registers to identify which PHY is onboard. */
-   for (hw->phy_addr = 1; (hw->phy_addr < 4); hw->phy_addr++) {
+   for (int i = 1; i < 4; i++) {
+   /*
+* hw->phy_addr may be modified down in the call stack,
+* we can't use it as loop variable.
+*/
+   hw->phy_addr = i;
ret_val = em_match_gig_phy(hw);
if (ret_val == E1000_SUCCESS)
return E1000_SUCCESS;



smtpd junk filtering action

2019-09-02 Thread gilles
The following diff introduces the junk action which allows builtin
filters to junk a session or transaction.

The following would junk a session if it doesn't have rdns when it
reaches the helo filtering hook:

match "check-rdns" phase helo match !rdns junk


Currently this is not doable so builtin filters may only accept or
throw away a session or transaction, whereas with this diff you'll
be able to automatically classify in Junk folder with the junk mda
option:

action "local_users" maildir junk


This also allows simplifying proc filters that want to junk as the
current method requires registering for data-line and injecting an
X-Spam: Yes header, while this can now be handled by returning the
junk filter response at any phase and letting smtpd prefill header




Index: lka_filter.c
===
RCS file: /cvs/src/usr.sbin/smtpd/lka_filter.c,v
retrieving revision 1.44
diff -u -p -r1.44 lka_filter.c
--- lka_filter.c29 Aug 2019 08:49:55 -  1.44
+++ lka_filter.c2 Sep 2019 20:04:37 -
@@ -56,6 +56,7 @@ static intfilter_builtins_mail_from(str
 static int filter_builtins_rcpt_to(struct filter_session *, struct filter 
*, uint64_t, const char *);
 
 static voidfilter_result_proceed(uint64_t);
+static voidfilter_result_junk(uint64_t);
 static voidfilter_result_rewrite(uint64_t, const char *);
 static voidfilter_result_reject(uint64_t, const char *);
 static voidfilter_result_disconnect(uint64_t, const char *);
@@ -479,6 +480,11 @@ lka_filter_process_response(const char *
fatalx("Unexpected parameter after proceed: %s", line);
filter_protocol_next(token, reqid, 0);
return;
+   } else if (strcmp(response, "junk") == 0) {
+   if (parameter != NULL)
+   fatalx("Unexpected parameter after junk: %s", line);
+   filter_result_junk(reqid);
+   return;
} else {
if (parameter == NULL)
fatalx("Missing parameter: %s", line);
@@ -574,6 +580,15 @@ filter_protocol_internal(struct filter_s
filter_result_disconnect(reqid, 
filter->config->disconnect);
return;
}
+   else if (filter->config->junk) {
+   log_trace(TRACE_FILTERS, "%016"PRIx64" filters protocol 
phase=%s, "
+   "resume=%s, action=junk, filter=%s, query=%s",
+   fs->id, phase_name, resume ? "y" : "n",
+   filter->name,
+   param);
+   filter_result_junk(reqid);
+   return;
+   }
else {
log_trace(TRACE_FILTERS, "%016"PRIx64" filters protocol 
phase=%s, "
"resume=%s, action=reject, filter=%s, query=%s, 
response=%s",
@@ -759,6 +774,15 @@ filter_result_proceed(uint64_t reqid)
m_create(p_pony, IMSG_FILTER_SMTP_PROTOCOL, 0, 0, -1);
m_add_id(p_pony, reqid);
m_add_int(p_pony, FILTER_PROCEED);
+   m_close(p_pony);
+}
+
+static void
+filter_result_junk(uint64_t reqid)
+{
+   m_create(p_pony, IMSG_FILTER_SMTP_PROTOCOL, 0, 0, -1);
+   m_add_id(p_pony, reqid);
+   m_add_int(p_pony, FILTER_JUNK);
m_close(p_pony);
 }
 
Index: lka_report.c
===
RCS file: /cvs/src/usr.sbin/smtpd/lka_report.c,v
retrieving revision 1.27
diff -u -p -r1.27 lka_report.c
--- lka_report.c29 Aug 2019 07:23:18 -  1.27
+++ lka_report.c2 Sep 2019 20:04:38 -
@@ -428,6 +428,9 @@ lka_report_smtp_filter_response(const ch
case FILTER_PROCEED:
response_name = "proceed";
break;
+   case FILTER_JUNK:
+   response_name = "junk";
+   break;
case FILTER_REWRITE:
response_name = "rewrite";
break;
Index: parse.y
===
RCS file: /cvs/src/usr.sbin/smtpd/parse.y,v
retrieving revision 1.259
diff -u -p -r1.259 parse.y
--- parse.y 25 Aug 2019 03:40:45 -  1.259
+++ parse.y 2 Sep 2019 20:04:38 -
@@ -1297,6 +1297,9 @@ REJECT STRING {
 | REWRITE STRING {
filter_config->rewrite = $2;
 }
+| JUNK {
+   filter_config->junk = 1;
+}
 ;
 
 filter_phase_check_fcrdns:
Index: smtp_session.c
===
RCS file: /cvs/src/usr.sbin/smtpd/smtp_session.c,v
retrieving revision 1.408
diff -u -p -r1.408 smtp_session.c
--- smtp_session.c  28 Aug 2019 15:50:36 -  1.408
+++ smtp_session.c  2 Sep 2019 20:04:39 -
@@ -126,6 +126,8 @@ struct smtp_tx {
int  rcvcount;
int  has_date;
int 

Re: [Patch] smtp(1) with proto "smtps" should default to port smtps/465

2019-09-02 Thread gilles
committed thanks !

August 31, 2019 1:57 PM, "Ross L Richardson"  wrote:

> ...unless I'm very mistaken!
> 
> Ross
> 
> 
> Index: smtpc.c
> ===
> RCS file: /cvs/src/usr.sbin/smtpd/smtpc.c,v
> retrieving revision 1.6
> diff -u -p -r1.6 smtpc.c
> --- smtpc.c 2 Jul 2019 09:36:20 - 1.6
> +++ smtpc.c 31 Aug 2019 11:48:17 -
> @@ -229,7 +229,7 @@ parse_server(char *server)
> else if (!strcmp(scheme, "smtps")) {
> params.tls_req = TLS_SMTPS;
> if (port == NULL)
> - port = "submission";
> + port = "smtps";
> }
> else if (!strcmp(scheme, "smtp")) {
> }



Re: smtpd: change PATH for filters

2019-09-02 Thread gilles
September 2, 2019 9:15 PM, "Martijn van Duren" 
 wrote:

> On 9/2/19 9:10 PM, gil...@poolp.org wrote:
> 
>> September 2, 2019 7:29 PM, "Martijn van Duren" 
>>  wrote:
>> 
>>> On 9/2/19 6:00 PM, gil...@poolp.org wrote:
>> 
>> September 2, 2019 5:55 PM, gil...@poolp.org wrote:
>> 
>> September 2, 2019 5:23 PM, "Martijn van Duren" 
>>  wrote:
>> 
>> Gilles should probably elaborate, but the way things are now is that
>> system(3) is used to start the filters, allowing us to run any arbitrary
>> (set of) command(s) as a filter.
>> 
>> Since the filters now in ports are non-interactive commands I proposed
>> to move them to /usr/local/libexec/smtpd, which gilles@ is a proponent
>> of. This however means that all filters need to be specified by a full
>> path, which is not something I would promote.
>> 
>> Hence the proposition of this diff.
>> I don't feel comfortable adding that path to PATH, even if we're going
>> to call system() right behind.
>> 
>> Why not detect if the command starts with '/' and prepend libexec directory
>> if that's not the case ?
>> 
>> I also want to rework the command line before it's passed to system() so we
>> exec and save some unnecessary processes which are only waiting for a child
>> to exit its infinite loop.
>> 
>> To do that, we are going to copy the command anyways so checking if command
>> starts with a / and prepending the absolute path is going to be easy.
>>> Something like this?
>> 
>> can't test right now but comments inlined:
>> 
>>> Index: smtpd.c
>>> ===
>>> RCS file: /cvs/src/usr.sbin/smtpd/smtpd.c,v
>>> retrieving revision 1.324
>>> diff -u -p -r1.324 smtpd.c
>>> --- smtpd.c 26 Jul 2019 07:08:34 - 1.324
>>> +++ smtpd.c 2 Sep 2019 17:27:31 -
>>> @@ -1277,6 +1277,8 @@ fork_processor(const char *name, const c
>>> int sp[2], errfd[2];
>>> struct passwd *pw;
>>> struct group *gr;
>>> + char exec[_POSIX_ARG_MAX];
>> 
>> I don't know if _POSIX_ARG_MAX is the proper define to use,
>> I genuinely don't know so someone more knowledgeable should jump in.
> 
> https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/limits.h.html
> {_POSIX_ARG_MAX}
> Maximum length of argument to the exec functions including environment data.
> Value: 4 096
> 

looks good to me unless someone objects


>>> + int execr;
>>> 
>>> if (user == NULL)
>>> user = SMTPD_USER;
>>> @@ -1340,6 +1342,14 @@ fork_processor(const char *name, const c
>>> signal(SIGHUP, SIG_DFL) == SIG_ERR)
>>> err(1, "signal");
>>> 
>>> + if (command[0] == '/')
>>> + execr = snprintf(exec, sizeof(exec), "exec %s", command);
>>> + else
>>> + execr = snprintf(exec, sizeof(exec), "exec %s/filter-%s",
>>> + PATH_LIBEXEC, command);
>> 
>> I don't really like that 'filter-' is implicit.
>> 
>> So if I specify full path it's filter-rspamd, when i don't it's rspamd,
>> which is confusing because that's also the name of a software on my
>> system.
>> 
>> I think people can live with typing 'filter-' and we keep it explicit.
> 
> Sure, no problem. I choose this approach to be more in line with
> queue-*, scheduler-*, and table-*.
> I'll change it before commit.
> 

okie dokie

I dislike that we did this for queue-*, scheduler-* and table-* too,
I'd rather see if we can change that in the future.

I have a plan for 2020 to switch queue / scheduler / table to an API
like filters' so we might want to revisit then :-)


>>> + if (execr >= (int) sizeof(exec))
>>> + errx(1, "%s: exec path too long", name);
>>> +
>>> /*
>>> * Wait for lka to acknowledge that it received the fd.
>>> * This prevents a race condition between the filter sending an error
>>> @@ -1350,7 +1360,7 @@ fork_processor(const char *name, const c
>>> */
>>> if (read(STDERR_FILENO, &buf, 1) != 0)
>>> errx(1, "lka didn't properly close write end of error socket");
>>> - if (system(command) == -1)
>>> + if (system(exec) == -1)
>>> err(1, NULL);
>>> 
>>> /* there's no successful exit from a processor */
>>> Index: smtpd.conf.5
>>> ===
>>> RCS file: /cvs/src/usr.sbin/smtpd/smtpd.conf.5,v
>>> retrieving revision 1.221
>>> diff -u -p -r1.221 smtpd.conf.5
>>> --- smtpd.conf.5 17 Aug 2019 14:43:21 - 1.221
>>> +++ smtpd.conf.5 2 Sep 2019 17:27:31 -
>>> @@ -383,6 +383,11 @@ filter
>>> .Ar filter-name
>>> from
>>> .Ar command .
>>> +If
>>> +.Ar command
>>> +starts with a slash it is executed with an absolute path,
>>> +else it will be prepended with
>>> +.Dq /usr/local/libexec/smtpd/filter- .
>>> .It Ic include Qq Ar pathname
>>> Replace this directive with the content of the additional configuration
>>> file at the absolute
>>> @@ -757,6 +762,11 @@ Register an external process named
>>> from
>>> .Ar command .
>>> Such processes may be used to share the same instance between multiple 
>>> filters.
>>> +If
>>> +.Ar command
>>> +starts with a slash it is executed with an absolute path,
>>> +else it will be prepended with
>>> +.Dq /usr/local/li

Re: smtpd: change PATH for filters

2019-09-02 Thread Martijn van Duren
On 9/2/19 9:10 PM, gil...@poolp.org wrote:
> September 2, 2019 7:29 PM, "Martijn van Duren" 
>  wrote:
> 
>> On 9/2/19 6:00 PM, gil...@poolp.org wrote:
>>
>>> September 2, 2019 5:55 PM, gil...@poolp.org wrote:
>>>
 September 2, 2019 5:23 PM, "Martijn van Duren" 
  wrote:
>>>
>>> Gilles should probably elaborate, but the way things are now is that
>>> system(3) is used to start the filters, allowing us to run any arbitrary
>>> (set of) command(s) as a filter.
>>>
>>> Since the filters now in ports are non-interactive commands I proposed
>>> to move them to /usr/local/libexec/smtpd, which gilles@ is a proponent
>>> of. This however means that all filters need to be specified by a full
>>> path, which is not something I would promote.
>>>
>>> Hence the proposition of this diff.
 I don't feel comfortable adding that path to PATH, even if we're going
 to call system() right behind.

 Why not detect if the command starts with '/' and prepend libexec directory
 if that's not the case ?
>>>
>>> I also want to rework the command line before it's passed to system() so we
>>> exec and save some unnecessary processes which are only waiting for a child
>>> to exit its infinite loop.
>>>
>>> To do that, we are going to copy the command anyways so checking if command
>>> starts with a / and prepending the absolute path is going to be easy.
>>
>> Something like this?
>>
> 
> can't test right now but comments inlined:
> 
> 
>> Index: smtpd.c
>> ===
>> RCS file: /cvs/src/usr.sbin/smtpd/smtpd.c,v
>> retrieving revision 1.324
>> diff -u -p -r1.324 smtpd.c
>> --- smtpd.c 26 Jul 2019 07:08:34 - 1.324
>> +++ smtpd.c 2 Sep 2019 17:27:31 -
>> @@ -1277,6 +1277,8 @@ fork_processor(const char *name, const c
>> int sp[2], errfd[2];
>> struct passwd *pw;
>> struct group *gr;
>> + char exec[_POSIX_ARG_MAX];
> 
> I don't know if _POSIX_ARG_MAX is the proper define to use,
> I genuinely don't know so someone more knowledgeable should jump in.

https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/limits.h.html
{_POSIX_ARG_MAX}
Maximum length of argument to the exec functions including environment data.
Value: 4 096
> 
> 
>> + int execr;
>>
>> if (user == NULL)
>> user = SMTPD_USER;
>> @@ -1340,6 +1342,14 @@ fork_processor(const char *name, const c
>> signal(SIGHUP, SIG_DFL) == SIG_ERR)
>> err(1, "signal");
>>
>> + if (command[0] == '/')
>> + execr = snprintf(exec, sizeof(exec), "exec %s", command);
>> + else
>> + execr = snprintf(exec, sizeof(exec), "exec %s/filter-%s",
>> + PATH_LIBEXEC, command);
> 
> I don't really like that 'filter-' is implicit.
> 
> So if I specify full path it's filter-rspamd, when i don't it's rspamd,
> which is confusing because that's also the name of a software on my
> system.
> 
> I think people can live with typing 'filter-' and we keep it explicit.

Sure, no problem. I choose this approach to be more in line with
queue-*, scheduler-*, and table-*.
I'll change it before commit.
> 
> 
> 
>> + if (execr >= (int) sizeof(exec))
>> + errx(1, "%s: exec path too long", name);
>> +
>> /*
>> * Wait for lka to acknowledge that it received the fd.
>> * This prevents a race condition between the filter sending an error
>> @@ -1350,7 +1360,7 @@ fork_processor(const char *name, const c
>> */
>> if (read(STDERR_FILENO, &buf, 1) != 0)
>> errx(1, "lka didn't properly close write end of error socket");
>> - if (system(command) == -1)
>> + if (system(exec) == -1)
>> err(1, NULL);
>>
>> /* there's no successful exit from a processor */
>> Index: smtpd.conf.5
>> ===
>> RCS file: /cvs/src/usr.sbin/smtpd/smtpd.conf.5,v
>> retrieving revision 1.221
>> diff -u -p -r1.221 smtpd.conf.5
>> --- smtpd.conf.5 17 Aug 2019 14:43:21 - 1.221
>> +++ smtpd.conf.5 2 Sep 2019 17:27:31 -
>> @@ -383,6 +383,11 @@ filter
>> .Ar filter-name
>> from
>> .Ar command .
>> +If
>> +.Ar command
>> +starts with a slash it is executed with an absolute path,
>> +else it will be prepended with
>> +.Dq /usr/local/libexec/smtpd/filter- .
>> .It Ic include Qq Ar pathname
>> Replace this directive with the content of the additional configuration
>> file at the absolute
>> @@ -757,6 +762,11 @@ Register an external process named
>> from
>> .Ar command .
>> Such processes may be used to share the same instance between multiple 
>> filters.
>> +If
>> +.Ar command
>> +starts with a slash it is executed with an absolute path,
>> +else it will be prepended with
>> +.Dq /usr/local/libexec/smtpd/filter- .
>> .It Ic queue Cm compression
>> Store queue files in a compressed format.
>> This may be useful to save disk space.



Re: smtpd: change PATH for filters

2019-09-02 Thread gilles
September 2, 2019 7:29 PM, "Martijn van Duren" 
 wrote:

> On 9/2/19 6:00 PM, gil...@poolp.org wrote:
> 
>> September 2, 2019 5:55 PM, gil...@poolp.org wrote:
>> 
>>> September 2, 2019 5:23 PM, "Martijn van Duren" 
>>>  wrote:
>> 
>> Gilles should probably elaborate, but the way things are now is that
>> system(3) is used to start the filters, allowing us to run any arbitrary
>> (set of) command(s) as a filter.
>> 
>> Since the filters now in ports are non-interactive commands I proposed
>> to move them to /usr/local/libexec/smtpd, which gilles@ is a proponent
>> of. This however means that all filters need to be specified by a full
>> path, which is not something I would promote.
>> 
>> Hence the proposition of this diff.
>>> I don't feel comfortable adding that path to PATH, even if we're going
>>> to call system() right behind.
>>> 
>>> Why not detect if the command starts with '/' and prepend libexec directory
>>> if that's not the case ?
>> 
>> I also want to rework the command line before it's passed to system() so we
>> exec and save some unnecessary processes which are only waiting for a child
>> to exit its infinite loop.
>> 
>> To do that, we are going to copy the command anyways so checking if command
>> starts with a / and prepending the absolute path is going to be easy.
> 
> Something like this?
> 

can't test right now but comments inlined:


> Index: smtpd.c
> ===
> RCS file: /cvs/src/usr.sbin/smtpd/smtpd.c,v
> retrieving revision 1.324
> diff -u -p -r1.324 smtpd.c
> --- smtpd.c 26 Jul 2019 07:08:34 - 1.324
> +++ smtpd.c 2 Sep 2019 17:27:31 -
> @@ -1277,6 +1277,8 @@ fork_processor(const char *name, const c
> int sp[2], errfd[2];
> struct passwd *pw;
> struct group *gr;
> + char exec[_POSIX_ARG_MAX];

I don't know if _POSIX_ARG_MAX is the proper define to use,
I genuinely don't know so someone more knowledgeable should jump in.


> + int execr;
> 
> if (user == NULL)
> user = SMTPD_USER;
> @@ -1340,6 +1342,14 @@ fork_processor(const char *name, const c
> signal(SIGHUP, SIG_DFL) == SIG_ERR)
> err(1, "signal");
> 
> + if (command[0] == '/')
> + execr = snprintf(exec, sizeof(exec), "exec %s", command);
> + else
> + execr = snprintf(exec, sizeof(exec), "exec %s/filter-%s",
> + PATH_LIBEXEC, command);

I don't really like that 'filter-' is implicit.

So if I specify full path it's filter-rspamd, when i don't it's rspamd,
which is confusing because that's also the name of a software on my
system.

I think people can live with typing 'filter-' and we keep it explicit.



> + if (execr >= (int) sizeof(exec))
> + errx(1, "%s: exec path too long", name);
> +
> /*
> * Wait for lka to acknowledge that it received the fd.
> * This prevents a race condition between the filter sending an error
> @@ -1350,7 +1360,7 @@ fork_processor(const char *name, const c
> */
> if (read(STDERR_FILENO, &buf, 1) != 0)
> errx(1, "lka didn't properly close write end of error socket");
> - if (system(command) == -1)
> + if (system(exec) == -1)
> err(1, NULL);
> 
> /* there's no successful exit from a processor */
> Index: smtpd.conf.5
> ===
> RCS file: /cvs/src/usr.sbin/smtpd/smtpd.conf.5,v
> retrieving revision 1.221
> diff -u -p -r1.221 smtpd.conf.5
> --- smtpd.conf.5 17 Aug 2019 14:43:21 - 1.221
> +++ smtpd.conf.5 2 Sep 2019 17:27:31 -
> @@ -383,6 +383,11 @@ filter
> .Ar filter-name
> from
> .Ar command .
> +If
> +.Ar command
> +starts with a slash it is executed with an absolute path,
> +else it will be prepended with
> +.Dq /usr/local/libexec/smtpd/filter- .
> .It Ic include Qq Ar pathname
> Replace this directive with the content of the additional configuration
> file at the absolute
> @@ -757,6 +762,11 @@ Register an external process named
> from
> .Ar command .
> Such processes may be used to share the same instance between multiple 
> filters.
> +If
> +.Ar command
> +starts with a slash it is executed with an absolute path,
> +else it will be prepended with
> +.Dq /usr/local/libexec/smtpd/filter- .
> .It Ic queue Cm compression
> Store queue files in a compressed format.
> This may be useful to save disk space.



Re: [PATCH] Avoid leftover temporary mount points when using -P (mfs)

2019-09-02 Thread Rafael Neves
On Mon, Sep 02, 2019 at 07:26:27AM +0200, Otto Moerbeek wrote:
> On Sun, Sep 01, 2019 at 05:01:55PM -0300, Rafael Neves wrote:
> 
> > On Wed, Aug 28, 2019 at 03:39:17PM +0200, Otto Moerbeek wrote:
> > > On Sat, Aug 17, 2019 at 12:13:50PM -0300, Rafael Neves wrote:
> > > 
> > > > Hi, 
> > > > 
> > > > Submitting to tech@ to broader audience.
> > > > 
> > > > When using -P option in mfs with a directory or a block device that
> > > > doen't exist, for example when the device roams, newfs(2) leaves
> > > > leftovers of temporary mount points.
> > > > 
> > > > With my /etc/fstab:
> > > > ca7552589896b01e.b none swap sw
> > > > ca7552589896b01e.a / ffs rw 1 1
> > > > ca7552589896b01e.k /home ffs rw,nodev,nosuid 1 2
> > > > #ca7552589896b01e.d /tmp ffs rw,nodev,nosuid 1 2
> > > > swap /tmp mfs rw,nodev,nosuid,-s=300M 0 0
> > > > ca7552589896b01e.f /usr ffs rw,nodev 1 2
> > > > ca7552589896b01e.g /usr/X11R6 ffs rw,nodev 1 2
> > > > ca7552589896b01e.h /usr/local ffs rw,nodev,wxallowed 1 2
> > > > ca7552589896b01e.j /usr/build ffs rw,noperm,noauto 1 2
> > > > swap /usr/obj mfs rw,nodev,nosuid,noauto,-s=4G,-P=/foo/bar  0 0
> > > > ca7552589896b01e.i /usr/src ffs rw,nodev,nosuid 1 2
> > > > ca7552589896b01e.e /var ffs rw,nodev,nosuid 1 2
> > > > 
> > > > Result when trying to mount /usr/obj:
> > > > root@orus [rneves]# mount /usr/obj
> > > > mount_mfs: cannot stat /foo/bar: No such file or directory
> > > > root@orus [rneves]# mount
> > > > /dev/sd2a on / type ffs (local)
> > > > /dev/sd2k on /home type ffs (local, nodev, nosuid)
> > > > mfs:28249 on /tmp type mfs (asynchronous, local, nodev, nosuid, 
> > > > size=614400 512-blocks)
> > > > /dev/sd2f on /usr type ffs (local, nodev)
> > > > /dev/sd2g on /usr/X11R6 type ffs (local, nodev)
> > > > /dev/sd2h on /usr/local type ffs (local, nodev, wxallowed)
> > > > /dev/sd2i on /usr/src type ffs (local, nodev, nosuid)
> > > > /dev/sd2e on /var type ffs (local, nodev, nosuid)
> > > > mfs:44634 on /tmp/mntoMG6WmZTT7 type mfs (asynchronous, local, 
> > > > nodev, nosuid, size=8388608 512-blocks)
> > > > 
> > > > 
> > > > Tracking down the issue I found that:
> > > > + When -P is specified, pop != NULL.
> > > > + After fork, waitformount() is called. It creates the temporary
> > > >   places to store the data.
> > > > + copy() is called, and it it fails the following umount() and 
> > > >   rmdir() is not called, leaving the temporary mounts.
> > > > 
> > > > As copy() can fail in a couple of ways, I thought about the following 
> > > > change:
> > > > + Make all errors a warning, but after then return to the first
> > > >   caller indicating an error. Getting the erro the clean up is
> > > >   done, and exit(1).
> > > > 
> > > > + Make copy() return an int: -1 in fail, and 0 otherwise.
> > > > + isdir(), gettmpmnt(): Convert errors to warnings + return(-1).
> > > > 
> > > > There is a change of messages printing if you don't have a /tmp
> > > > is read-only, first the error of cannot fstat, and after
> > > > "Cannot create tmp mountpoint for -P".
> > > > 
> > > > There still a chance to get a inconsistent state: if there is no 
> > > > /bin/pax, or errors in do_exec(). But erros in do_exec seem very
> > > > critical, the same with a missing /bin/pax. So I didn't change them.
> > > > 
> > > > Otto@ said that another alternative is using atexit(3), but we
> > > > pointed out what it is very difficult to get it right, and almost
> > > > always has race conditions. Given that and what manpage says I have no
> > > > hope that I can get it right.
> > > > 
> > > > What do you think?
> > > > 
> > > > Note that the check in line 519 (newfs.c) was changed to add the new 
> > > > possible return value. Actually, currently it is not allowed -P with a
> > > > read-only /tmp, because in this case gettmpmnt() returns 0, and by this
> > > > early check newfs(2) throws an error. Actually, gettmpmnt() must changed
> > > > to it work properly. The `created` if uses a <= by symmetry. 
> > > > 
> > > > But this is a differente issue, that I think could be changed in a
> > > > separated diff.
> > > > 
> > > > Regards,
> > > > Rafael Neves
> > > > 
> > > > 
> > > > Patch:
> > > > 
> > > > 
> > > > Index: newfs.c
> > > > ===
> > > > RCS file: /cvs/src/sbin/newfs/newfs.c,v
> > > > retrieving revision 1.112
> > > > diff -u -p -r1.112 newfs.c
> > > > --- newfs.c 28 Jun 2019 13:32:45 -  1.112
> > > > +++ newfs.c 17 Aug 2019 14:27:46 -
> > > > @@ -147,7 +147,7 @@ struct disklabel *getdisklabel(char *, i
> > > >  static void waitformount(char *, pid_t);
> > > >  static int do_exec(const char *, const char *, char *const[]);
> > > >  static int isdir(const

Re: Questions about syspatch(8) mtree(8) behaviour

2019-09-02 Thread Hiltjo Posthuma
On Mon, Sep 02, 2019 at 12:07:59PM -0600, Theo de Raadt wrote:
> Hiltjo Posthuma  wrote:
> 
> > Hi,
> > 
> > I have three questions regarding a behaviour of syspatch(8) with mtree(8).
> > 
> > 1. I noticed when applying patches it resets some permissions of new, but 
> > also of
> > existing directories on the system using mtree(8).
> > 
> > In the shellscript syspatch(8) there is a function:
> > 
> > trap_handler():
> > # in case a patch added a new directory (install -D)
> > if [[ -n ${_PATCHES} ]]; then
> > mtree -qdef /etc/mtree/4.4BSD.dist -p / -U >/dev/null
> > [[ -f /var/sysmerge/xetc.tgz ]] &&
> > mtree -qdef /etc/mtree/BSD.x11.dist -p / -U 
> > >/dev/null
> > fi
> > 
> > Here the comment says: "in case a patch added a new directory (install -D)".
> > This is true, but it also applies to existing directories and resets
> > permissions, ownership, etc.
> > 
> > A real-world example: on my system after applying syspatch this changed
> > permissions of an existing directory and a daemon (mysqld) failed to start,
> > because it could not access a UNIX domain socket file in the www chroot.
> 
> A very long mail without being 100% PRECISE.
> 
> > Is this intended? If so should this behaviour perhaps get documented in the 
> > man
> > page? I can write a patch if so.
> 
> Intentional.  As a general rule if you change a system component, you own
> all the pieces.
> 
> But I guess you did chmod a+wrxt / or something, right?  I have to assume
> so, because your mail is not PRECISE.
> 

In this particular case it was the directory /var/www/run. The default
permissions are as specified in /etc/mtree/4.4BSD.dist:

run type=dir uname=root gname=daemon mode=755

I changed it from 755 to 775 (still root:daemon) so both mysqld and PHP could
access the UNIX domain socket in the www chroot (/var/www).

I was a bit surprised syspatch silently changed it back to 755 and thus broke
my setup.  Maybe having mtree be more verbose or have a sentence documenting it
in the syspatch(8) man page would help. I would be happy to write a patch for
this.


> > 
> > 2. This code-path is called when $_PATCHES is set, thus when patches are
> > available and are being applied, but on patch rollback (syspatch -r or -R) 
> > it
> > does not run mtree. Wouldn't it be more consistent to also run mtree after
> > patch rollback?
> > 
> > 3. With an other case on another machine with low disk-space the following
> > occurred: syspatch is run and ran out of disk-space while applying patches: 
> > "No
> > space left on sd0f, aborting", but it still ran mtree and reset the 
> > permissions
> > on "SIGEXIT". Wouldn't it make more sense to not change anything if no patch
> > could be applied?
> > 
> > Thanks for your time,
> > 

-- 
Kind regards,
Hiltjo



Re: Questions about syspatch(8) mtree(8) behaviour

2019-09-02 Thread Theo de Raadt
Hiltjo Posthuma  wrote:

> Hi,
> 
> I have three questions regarding a behaviour of syspatch(8) with mtree(8).
> 
> 1. I noticed when applying patches it resets some permissions of new, but 
> also of
> existing directories on the system using mtree(8).
> 
> In the shellscript syspatch(8) there is a function:
> 
> trap_handler():
> # in case a patch added a new directory (install -D)
> if [[ -n ${_PATCHES} ]]; then
> mtree -qdef /etc/mtree/4.4BSD.dist -p / -U >/dev/null
> [[ -f /var/sysmerge/xetc.tgz ]] &&
> mtree -qdef /etc/mtree/BSD.x11.dist -p / -U >/dev/null
> fi
> 
> Here the comment says: "in case a patch added a new directory (install -D)".
> This is true, but it also applies to existing directories and resets
> permissions, ownership, etc.
> 
> A real-world example: on my system after applying syspatch this changed
> permissions of an existing directory and a daemon (mysqld) failed to start,
> because it could not access a UNIX domain socket file in the www chroot.

A very long mail without being 100% PRECISE.

> Is this intended? If so should this behaviour perhaps get documented in the 
> man
> page? I can write a patch if so.

Intentional.  As a general rule if you change a system component, you own
all the pieces.

But I guess you did chmod a+wrxt / or something, right?  I have to assume
so, because your mail is not PRECISE.


> 
> 2. This code-path is called when $_PATCHES is set, thus when patches are
> available and are being applied, but on patch rollback (syspatch -r or -R) it
> does not run mtree. Wouldn't it be more consistent to also run mtree after
> patch rollback?
> 
> 3. With an other case on another machine with low disk-space the following
> occurred: syspatch is run and ran out of disk-space while applying patches: 
> "No
> space left on sd0f, aborting", but it still ran mtree and reset the 
> permissions
> on "SIGEXIT". Wouldn't it make more sense to not change anything if no patch
> could be applied?
> 
> Thanks for your time,
> 
> -- 
> Kind regards,
> Hiltjo
> 



Questions about syspatch(8) mtree(8) behaviour

2019-09-02 Thread Hiltjo Posthuma
Hi,

I have three questions regarding a behaviour of syspatch(8) with mtree(8).

1. I noticed when applying patches it resets some permissions of new, but also 
of
existing directories on the system using mtree(8).

In the shellscript syspatch(8) there is a function:

trap_handler():
# in case a patch added a new directory (install -D)
if [[ -n ${_PATCHES} ]]; then
mtree -qdef /etc/mtree/4.4BSD.dist -p / -U >/dev/null
[[ -f /var/sysmerge/xetc.tgz ]] &&
mtree -qdef /etc/mtree/BSD.x11.dist -p / -U >/dev/null
fi

Here the comment says: "in case a patch added a new directory (install -D)".
This is true, but it also applies to existing directories and resets
permissions, ownership, etc.

A real-world example: on my system after applying syspatch this changed
permissions of an existing directory and a daemon (mysqld) failed to start,
because it could not access a UNIX domain socket file in the www chroot.

Is this intended? If so should this behaviour perhaps get documented in the man
page? I can write a patch if so.

2. This code-path is called when $_PATCHES is set, thus when patches are
available and are being applied, but on patch rollback (syspatch -r or -R) it
does not run mtree. Wouldn't it be more consistent to also run mtree after
patch rollback?

3. With an other case on another machine with low disk-space the following
occurred: syspatch is run and ran out of disk-space while applying patches: "No
space left on sd0f, aborting", but it still ran mtree and reset the permissions
on "SIGEXIT". Wouldn't it make more sense to not change anything if no patch
could be applied?

Thanks for your time,

-- 
Kind regards,
Hiltjo



Re: smtpd: change PATH for filters

2019-09-02 Thread Martijn van Duren
On 9/2/19 6:00 PM, gil...@poolp.org wrote:
> September 2, 2019 5:55 PM, gil...@poolp.org wrote:
> 
>> September 2, 2019 5:23 PM, "Martijn van Duren" 
>>  wrote:
>>
>>> Gilles should probably elaborate, but the way things are now is that
>>> system(3) is used to start the filters, allowing us to run any arbitrary
>>> (set of) command(s) as a filter.
>>>
>>> Since the filters now in ports are non-interactive commands I proposed
>>> to move them to /usr/local/libexec/smtpd, which gilles@ is a proponent
>>> of. This however means that all filters need to be specified by a full
>>> path, which is not something I would promote.
>>>
>>> Hence the proposition of this diff.
>>
>> I don't feel comfortable adding that path to PATH, even if we're going
>> to call system() right behind.
>>
>> Why not detect if the command starts with '/' and prepend libexec directory
>> if that's not the case ?
>>
> 
> I also want to rework the command line before it's passed to system() so we
> exec and save some unnecessary processes which are only waiting for a child
> to exit its infinite loop.
> 
> To do that, we are going to copy the command anyways so checking if command
> starts with a / and prepending the absolute path is going to be easy.
> 
Something like this?

Index: smtpd.c
===
RCS file: /cvs/src/usr.sbin/smtpd/smtpd.c,v
retrieving revision 1.324
diff -u -p -r1.324 smtpd.c
--- smtpd.c 26 Jul 2019 07:08:34 -  1.324
+++ smtpd.c 2 Sep 2019 17:27:31 -
@@ -1277,6 +1277,8 @@ fork_processor(const char *name, const c
int  sp[2], errfd[2];
struct passwd   *pw;
struct group*gr;
+   char exec[_POSIX_ARG_MAX];
+   int  execr;
 
if (user == NULL)
user = SMTPD_USER;
@@ -1340,6 +1342,14 @@ fork_processor(const char *name, const c
signal(SIGHUP, SIG_DFL) == SIG_ERR)
err(1, "signal");
 
+   if (command[0] == '/')
+   execr = snprintf(exec, sizeof(exec), "exec %s", command);
+   else
+   execr = snprintf(exec, sizeof(exec), "exec %s/filter-%s", 
+   PATH_LIBEXEC, command);
+   if (execr >= (int) sizeof(exec))
+   errx(1, "%s: exec path too long", name);
+
/*
 * Wait for lka to acknowledge that it received the fd.
 * This prevents a race condition between the filter sending an error
@@ -1350,7 +1360,7 @@ fork_processor(const char *name, const c
 */
if (read(STDERR_FILENO, &buf, 1) != 0)
errx(1, "lka didn't properly close write end of error socket");
-   if (system(command) == -1)
+   if (system(exec) == -1)
err(1, NULL);
 
/* there's no successful exit from a processor */
Index: smtpd.conf.5
===
RCS file: /cvs/src/usr.sbin/smtpd/smtpd.conf.5,v
retrieving revision 1.221
diff -u -p -r1.221 smtpd.conf.5
--- smtpd.conf.517 Aug 2019 14:43:21 -  1.221
+++ smtpd.conf.52 Sep 2019 17:27:31 -
@@ -383,6 +383,11 @@ filter
 .Ar filter-name
 from
 .Ar command .
+If
+.Ar command
+starts with a slash it is executed with an absolute path,
+else it will be prepended with
+.Dq /usr/local/libexec/smtpd/filter- .
 .It Ic include Qq Ar pathname
 Replace this directive with the content of the additional configuration
 file at the absolute
@@ -757,6 +762,11 @@ Register an external process named
 from
 .Ar command .
 Such processes may be used to share the same instance between multiple filters.
+If
+.Ar command
+starts with a slash it is executed with an absolute path,
+else it will be prepended with
+.Dq /usr/local/libexec/smtpd/filter- .
 .It Ic queue Cm compression
 Store queue files in a compressed format.
 This may be useful to save disk space.



less(1): replace the last step_char(-1)

2019-09-02 Thread Ingo Schwarze
Hi,

The command line handling code in less/cmdbuf.c is very complicated.
>From the top level function cmd_char(), the stack can go down nine levels
before finally reaching the bottom level function cmd_step_common().
One of the functions traversed during that descent is the recursive
function cmd_repaint(), and the call tree has many different branches.
UTF-8 handling occurs both in the top level function and in the
bottom level function, and also at some places in between.

So when cleaning up the UTF-8 handling in here, i'm trying to proceed
very slowly and in minimal steps.  Given that i have little hope of
finding a system for testing each and every possible code path, my
plan is to make sure that individual changes do not change behaviour
locally (or only in intended ways).

So, lets get to the first, small step.  Right now, only one place
remains where the function step_char() is called with dir = -1,
i.e. moving the cursor leftward: from cmd_step_left() near the
bottom of the call stack.  That function is used when moving the
cursor left by one character (with or without erase), by one word,
or all the way back to the prompt.  It is also used to delete
leftover characters after repainting the line when the new text is
shorter than the old text.  And it is also used when scrolling
forward.  Given the complexity of the code, there may be even more
situations.

The obvious replacement for step_char(-1) is the new function
mbtowc_left() that i recently introduced.  Using that allows dropping
the "dir" argument from step_char().  Of course, the plan still is
to ultimately get rid fo step_char() altogether.

Let's make sure the new behaviour makes sense:

 * When at the beginning of the buffer, do not move, and use L'\0'
   as a replacement for the character that isn't there.
   Same behaviour as before.
 * When there is no valid UTF-8 character to the left (return value -1),
   back up one byte and use L'\0'.
   This _is_ a change of behaviour.  Current behaviour is to back up
   across all UTF-8 continuation bytes (no matter how many) until
   finding a byte that is not a continuation byte, then take the
   length given in that byte at face value (even if invalid) and
   construct a codepoint using that number of bytes (even if that
   implies ignoring additional garbage bytes in between, and even
   it it means extending the character that is supposedly to the
   left of the cursor to bytes that are to the right of the current
   cursor position).
   I believe making multibyte steps only for valid characters,
   and stepping byte-by-byte across invalid bytes, is safer than 
   the reckless jumping done right now.
   Then again, the practical difference may be small (if any)
   because i'm not quite sure how to get invalid bytes into the
   command buffer in the first place.
 * When there is a NUL byte to the left (return value 0),
   back up by this one byte, and use it as L'\0'.
   Same behaviour as before.

Tested by typing UTF-8 into the command buffer, moving the cursor
left and right across it (with and without deleting), inserting
UTF-8 before it, and scrolling the UTF-8 off the screen both to the
right and to the left.

OK?
  Ingo


Index: charset.c
===
RCS file: /cvs/src/usr.bin/less/charset.c,v
retrieving revision 1.26
diff -u -p -r1.26 charset.c
--- charset.c   2 Sep 2019 14:07:45 -   1.26
+++ charset.c   2 Sep 2019 15:00:50 -
@@ -353,7 +353,7 @@ get_wchar(const char *p)
  * Step forward or backward one character in a string.
  */
 LWCHAR
-step_char(char **pp, int dir, char *limit)
+step_char(char **pp, char *limit)
 {
LWCHAR ch;
int len;
@@ -361,11 +361,8 @@ step_char(char **pp, int dir, char *limi
 
if (!utf_mode) {
/* It's easy if chars are one byte. */
-   if (dir > 0)
-   ch = (LWCHAR) ((p < limit) ? *p++ : 0);
-   else
-   ch = (LWCHAR) ((p > limit) ? *--p : 0);
-   } else if (dir > 0) {
+   ch = (LWCHAR) ((p < limit) ? *p++ : 0);
+   } else {
len = utf_len(*p);
if (p + len > limit) {
ch = 0;
@@ -374,13 +371,6 @@ step_char(char **pp, int dir, char *limi
ch = get_wchar(p);
p += len;
}
-   } else {
-   while (p > limit && IS_UTF8_TRAIL(p[-1]))
-   p--;
-   if (p > limit)
-   ch = get_wchar(--p);
-   else
-   ch = 0;
}
*pp = p;
return (ch);
Index: cmdbuf.c
===
RCS file: /cvs/src/usr.bin/less/cmdbuf.c,v
retrieving revision 1.20
diff -u -p -r1.20 cmdbuf.c
--- cmdbuf.c2 Sep 2019 14:07:45 -   1.20
+++ cmdbuf.c2 Sep 2019 15:00:51 -
@@ -141,7 +141,7 @@ len_cmdbuf(void)
   

Re: smtpd: change PATH for filters

2019-09-02 Thread Theo de Raadt
gil...@poolp.org wrote:

> September 2, 2019 5:23 PM, "Martijn van Duren" 
>  wrote:
> 
> > Gilles should probably elaborate, but the way things are now is that
> > system(3) is used to start the filters, allowing us to run any arbitrary
> > (set of) command(s) as a filter.
> > 
> > Since the filters now in ports are non-interactive commands I proposed
> > to move them to /usr/local/libexec/smtpd, which gilles@ is a proponent
> > of. This however means that all filters need to be specified by a full
> > path, which is not something I would promote.
> > 
> > Hence the proposition of this diff.
> > 
> 
> I don't feel comfortable adding that path to PATH, even if we're going
> to call system() right behind.

The main problem with adding it to PATH, is the transitive nature of the
environment.  If the first command being run does a system of something
else, it will be seen again.  If it runs a shell script, there it is.
Easily reachable code fragments becomes available by virtue of being at
the head of the path.  That is improper.



Re: smtpd: change PATH for filters

2019-09-02 Thread gilles
September 2, 2019 5:55 PM, gil...@poolp.org wrote:

> September 2, 2019 5:23 PM, "Martijn van Duren" 
>  wrote:
> 
>> Gilles should probably elaborate, but the way things are now is that
>> system(3) is used to start the filters, allowing us to run any arbitrary
>> (set of) command(s) as a filter.
>> 
>> Since the filters now in ports are non-interactive commands I proposed
>> to move them to /usr/local/libexec/smtpd, which gilles@ is a proponent
>> of. This however means that all filters need to be specified by a full
>> path, which is not something I would promote.
>> 
>> Hence the proposition of this diff.
> 
> I don't feel comfortable adding that path to PATH, even if we're going
> to call system() right behind.
> 
> Why not detect if the command starts with '/' and prepend libexec directory
> if that's not the case ?
> 

I also want to rework the command line before it's passed to system() so we
exec and save some unnecessary processes which are only waiting for a child
to exit its infinite loop.

To do that, we are going to copy the command anyways so checking if command
starts with a / and prepending the absolute path is going to be easy.



Re: smtpd: change PATH for filters

2019-09-02 Thread gilles
September 2, 2019 5:23 PM, "Martijn van Duren" 
 wrote:

> Gilles should probably elaborate, but the way things are now is that
> system(3) is used to start the filters, allowing us to run any arbitrary
> (set of) command(s) as a filter.
> 
> Since the filters now in ports are non-interactive commands I proposed
> to move them to /usr/local/libexec/smtpd, which gilles@ is a proponent
> of. This however means that all filters need to be specified by a full
> path, which is not something I would promote.
> 
> Hence the proposition of this diff.
> 

I don't feel comfortable adding that path to PATH, even if we're going
to call system() right behind.

Why not detect if the command starts with '/' and prepend libexec directory
if that's not the case ?



> On 9/2/19 5:13 PM, Theo de Raadt wrote:
> 
>> This seems really unconvenitional.
>> 
>> PATH is normally used by interactive commands, or scripts which want
>> easily accessible programs. libexec has traditionally been excluded.
>> The idea is that programs which need things in libexec, must hardcode
>> the path. Intentionally. This is a subdirectory of libexec, probably
>> trying to follow the same pattern. So why is it not excluded in the same
>> way?
>> 
>> Second questoin: are filter programs going to be in /bin or /usr/sbin?
>> If not, why is /bin on this PATH you are defining?
>> 
>> It smell execvp abuse.
>> 
>> Martijn van Duren  wrote:
>> 
>>> With filters most likely defaulting to /usr/local/libexec/smtpd in the
>>> near future I would like to add this as the default PATH, followed by
>>> the usual suspects. I left the usual suspects in place because people
>>> have shown they already implement filters in awk and probably will do
>>> so in /bin/sh in the future, which will need all the other paths.
>>> 
>>> I put it in PATH_FILTER, so it can easily be altered for portable
>>> where sysadmins may decide to change the path for filters or for
>>> us if we ever want anything in base under /usr/libexec/smtpd.
>>> 
>>> OK?
>>> 
>>> martijn@
>>> 
>>> Index: smtpd.c
>>> ===
>>> RCS file: /cvs/src/usr.sbin/smtpd/smtpd.c,v
>>> retrieving revision 1.324
>>> diff -u -p -r1.324 smtpd.c
>>> --- smtpd.c 26 Jul 2019 07:08:34 - 1.324
>>> +++ smtpd.c 2 Sep 2019 15:00:47 -
>>> @@ -1350,6 +1350,7 @@ fork_processor(const char *name, const c
>>> */
>>> if (read(STDERR_FILENO, &buf, 1) != 0)
>>> errx(1, "lka didn't properly close write end of error socket");
>>> + setenv("PATH", PATH_FILTER, 1);
>>> if (system(command) == -1)
>>> err(1, NULL);
>>> 
>>> Index: smtpd.h
>>> ===
>>> RCS file: /cvs/src/usr.sbin/smtpd/smtpd.h,v
>>> retrieving revision 1.633
>>> diff -u -p -r1.633 smtpd.h
>>> --- smtpd.h 28 Aug 2019 15:50:36 - 1.633
>>> +++ smtpd.h 2 Sep 2019 15:00:47 -
>>> @@ -56,6 +56,7 @@
>>> #define SMTPD_BACKLOG 5
>>> 
>>> #define PATH_SMTPCTL "/usr/sbin/smtpctl"
>>> +#define PATH_FILTER "/usr/local/libexec/smtpd:" _PATH_DEFPATH
>>> 
>>> #define PATH_OFFLINE "/offline"
>>> #define PATH_PURGE "/purge"



bnxt: Support MSI-X

2019-09-02 Thread Stefan Fritsch
Tested with a BCM57412

OK?

diff --git a/sys/dev/pci/if_bnxt.c b/sys/dev/pci/if_bnxt.c
--- a/sys/dev/pci/if_bnxt.c
+++ b/sys/dev/pci/if_bnxt.c
@@ -102,6 +102,7 @@
 #define BNXT_FLAG_NPAR  0x0002
 #define BNXT_FLAG_WOL_CAP   0x0004
 #define BNXT_FLAG_SHORT_CMD 0x0008
+#define BNXT_FLAG_MSIX  0x0010
 
 /* NVRam stuff has a five minute timeout */
 #define BNXT_NVM_TIMEO (5 * 60 * 1000)
@@ -507,7 +508,9 @@ bnxt_attach(struct device *parent, struct device *self, 
void *aux)
 * devices advertise msi support, but there's no way to tell a
 * completion queue to use msi mode, only legacy or msi-x.
 */
-   if (/*pci_intr_map_msi(pa, &ih) != 0 && */ pci_intr_map(pa, &ih) != 0) {
+   if (pci_intr_map_msix(pa, 0, &ih) == 0) {
+   sc->sc_flags |= BNXT_FLAG_MSIX;
+   } else if (pci_intr_map(pa, &ih) != 0) {
printf(": unable to map interrupt\n");
goto free_resp;
}
@@ -2658,7 +2661,9 @@ bnxt_hwrm_ring_alloc(struct bnxt_softc *softc, uint8_t 
type,
req.logical_id = htole16(ring->id);
req.cmpl_ring_id = htole16(cmpl_ring_id);
req.queue_id = htole16(softc->sc_q_info[0].id);
-   req.int_mode = 0;
+   req.int_mode = (softc->sc_flags & BNXT_FLAG_MSIX) ?
+   HWRM_RING_ALLOC_INPUT_INT_MODE_MSIX :
+   HWRM_RING_ALLOC_INPUT_INT_MODE_LEGACY;
BNXT_HWRM_LOCK(softc);
rc = _hwrm_send_message(softc, &req, sizeof(req));
if (rc)



Re: armv7: remove gcc support from kernel Makefile

2019-09-02 Thread Theo de Raadt
> OK?  Or it can be rolled into somebody else's larger "remove gcc
> from armv7" diff.

This is fine with me as-is.



armv7: remove gcc support from kernel Makefile

2019-09-02 Thread Christian Weisgerber
This drops support for building the armv7 kernel with gcc and reduces
the difference to arm64.

I can't test this.

OK?  Or it can be rolled into somebody else's larger "remove gcc
from armv7" diff.

Index: arch/armv7/conf/Makefile.armv7
===
RCS file: /cvs/src/sys/arch/armv7/conf/Makefile.armv7,v
retrieving revision 1.45
diff -u -p -r1.45 Makefile.armv7
--- arch/armv7/conf/Makefile.armv7  21 Jun 2019 15:34:07 -  1.45
+++ arch/armv7/conf/Makefile.armv7  2 Sep 2019 15:26:17 -
@@ -25,14 +25,10 @@ INCLUDES=   -nostdinc -I$S -I. -I$S/arch
 CPPFLAGS=  ${INCLUDES} ${IDENT} ${PARAM} -D_KERNEL -D__${_mach}__ -MD -MP
 CWARNFLAGS=-Werror -Wall -Wimplicit-function-declaration \
-Wno-uninitialized -Wno-pointer-sign \
+   -Wno-constant-conversion -Wno-address-of-packed-member \
-Wframe-larger-than=2047
 
-CMACHFLAGS=-msoft-float
-.if ${COMPILER_VERSION:Mgcc4}
-CMACHFLAGS+=   -march=armv6 -Wa,-march=armv7a
-.else
-CMACHFLAGS+=   -march=armv7a
-.endif
+CMACHFLAGS=-msoft-float -march=armv7a
 CMACHFLAGS+=   -ffreestanding ${NOPIE_FLAGS}
 SORTR= sort -R
 .if ${IDENT:M-DNO_PROPOLICE}
@@ -42,10 +38,6 @@ CMACHFLAGS+= -fno-stack-protector
 SORTR= cat
 COPTS?=-Oz
 .endif
-.if ${COMPILER_VERSION:Mclang}
-NO_INTEGR_AS=  -no-integrated-as
-CWARNFLAGS+=   -Wno-address-of-packed-member -Wno-constant-conversion
-.endif
 
 DEBUG?=-g
 COPTS?=-O2
@@ -103,7 +95,7 @@ LINKFLAGS+=  -S
 assym.h: $S/kern/genassym.sh Makefile \
 ${_archdir}/${_arch}/genassym.cf ${_machdir}/${_mach}/genassym.cf
cat ${_archdir}/${_arch}/genassym.cf ${_machdir}/${_mach}/genassym.cf | 
\
-   sh $S/kern/genassym.sh ${CC} ${NO_INTEGR_AS} ${CFLAGS} ${CPPFLAGS} 
-MF assym.P > assym.h.tmp
+   sh $S/kern/genassym.sh ${CC} ${CFLAGS} ${CPPFLAGS} 
-no-integrated-as -MF assym.P > assym.h.tmp
sed '1s/.*/assym.h: \\/' assym.P > assym.d
sort -u assym.h.tmp > assym.h
 
-- 
Christian "naddy" Weisgerber  na...@mips.inka.de



Re: smtpd: change PATH for filters

2019-09-02 Thread Theo de Raadt
Martijn van Duren  wrote:

> Gilles should probably elaborate, but the way things are now is that
> system(3) is used to start the filters, allowing us to run any arbitrary
> (set of) command(s) as a filter.
> 
> Since the filters now in ports are non-interactive commands I proposed
> to move them to /usr/local/libexec/smtpd, which gilles@ is a proponent
> of. This however means that all filters need to be specified by a full
> path, which is not something I would promote.
> 
> Hence the proposition of this diff.

None of that prevents using an absolute path.

absolute path execution is "the rule" when it comes to libexec.

For instance, inetd.conf



Re: smtpd: change PATH for filters

2019-09-02 Thread Martijn van Duren
Gilles should probably elaborate, but the way things are now is that
system(3) is used to start the filters, allowing us to run any arbitrary
(set of) command(s) as a filter.

Since the filters now in ports are non-interactive commands I proposed
to move them to /usr/local/libexec/smtpd, which gilles@ is a proponent
of. This however means that all filters need to be specified by a full
path, which is not something I would promote.

Hence the proposition of this diff.

On 9/2/19 5:13 PM, Theo de Raadt wrote:
> This seems really unconvenitional.
> 
> PATH is normally used by interactive commands, or scripts which want
> easily accessible programs.  libexec has traditionally been excluded.
> The idea is that programs which need things in libexec, must hardcode
> the path.  Intentionally.   This is a subdirectory of libexec, probably
> trying to follow the same pattern.  So why is it not excluded in the same
> way?
> 
> Second questoin: are filter programs going to be in /bin or /usr/sbin?
> If not, why is /bin on this PATH you are defining?
> 
> It smell execvp abuse.
> 
> Martijn van Duren  wrote:
> 
>> With filters most likely defaulting to /usr/local/libexec/smtpd in the  
>> near future I would like to add this as the default PATH, followed by
>> the usual suspects. I left the usual suspects in place because people
>> have shown they already implement filters in awk and probably will do
>> so in /bin/sh in the future, which will need all the other paths.
>>
>> I put it in PATH_FILTER, so it can easily be altered for portable
>> where sysadmins may decide to change the path for filters or for
>> us if we ever want anything in base under /usr/libexec/smtpd.
>>
>> OK?
>>
>> martijn@
>>
>> Index: smtpd.c
>> ===
>> RCS file: /cvs/src/usr.sbin/smtpd/smtpd.c,v
>> retrieving revision 1.324
>> diff -u -p -r1.324 smtpd.c
>> --- smtpd.c  26 Jul 2019 07:08:34 -  1.324
>> +++ smtpd.c  2 Sep 2019 15:00:47 -
>> @@ -1350,6 +1350,7 @@ fork_processor(const char *name, const c
>>   */
>>  if (read(STDERR_FILENO, &buf, 1) != 0)
>>  errx(1, "lka didn't properly close write end of error socket");
>> +setenv("PATH", PATH_FILTER, 1);
>>  if (system(command) == -1)
>>  err(1, NULL);
>>  
>> Index: smtpd.h
>> ===
>> RCS file: /cvs/src/usr.sbin/smtpd/smtpd.h,v
>> retrieving revision 1.633
>> diff -u -p -r1.633 smtpd.h
>> --- smtpd.h  28 Aug 2019 15:50:36 -  1.633
>> +++ smtpd.h  2 Sep 2019 15:00:47 -
>> @@ -56,6 +56,7 @@
>>  #define SMTPD_BACKLOG5
>>  
>>  #define PATH_SMTPCTL"/usr/sbin/smtpctl"
>> +#define PATH_FILTER "/usr/local/libexec/smtpd:" _PATH_DEFPATH
>>  
>>  #define PATH_OFFLINE"/offline"
>>  #define PATH_PURGE  "/purge"
>>



Re: smtpd: change PATH for filters

2019-09-02 Thread Theo de Raadt
This seems really unconvenitional.

PATH is normally used by interactive commands, or scripts which want
easily accessible programs.  libexec has traditionally been excluded.
The idea is that programs which need things in libexec, must hardcode
the path.  Intentionally.   This is a subdirectory of libexec, probably
trying to follow the same pattern.  So why is it not excluded in the same
way?

Second questoin: are filter programs going to be in /bin or /usr/sbin?
If not, why is /bin on this PATH you are defining?

It smell execvp abuse.

Martijn van Duren  wrote:

> With filters most likely defaulting to /usr/local/libexec/smtpd in the  
> near future I would like to add this as the default PATH, followed by
> the usual suspects. I left the usual suspects in place because people
> have shown they already implement filters in awk and probably will do
> so in /bin/sh in the future, which will need all the other paths.
> 
> I put it in PATH_FILTER, so it can easily be altered for portable
> where sysadmins may decide to change the path for filters or for
> us if we ever want anything in base under /usr/libexec/smtpd.
> 
> OK?
> 
> martijn@
> 
> Index: smtpd.c
> ===
> RCS file: /cvs/src/usr.sbin/smtpd/smtpd.c,v
> retrieving revision 1.324
> diff -u -p -r1.324 smtpd.c
> --- smtpd.c   26 Jul 2019 07:08:34 -  1.324
> +++ smtpd.c   2 Sep 2019 15:00:47 -
> @@ -1350,6 +1350,7 @@ fork_processor(const char *name, const c
>*/
>   if (read(STDERR_FILENO, &buf, 1) != 0)
>   errx(1, "lka didn't properly close write end of error socket");
> + setenv("PATH", PATH_FILTER, 1);
>   if (system(command) == -1)
>   err(1, NULL);
>  
> Index: smtpd.h
> ===
> RCS file: /cvs/src/usr.sbin/smtpd/smtpd.h,v
> retrieving revision 1.633
> diff -u -p -r1.633 smtpd.h
> --- smtpd.h   28 Aug 2019 15:50:36 -  1.633
> +++ smtpd.h   2 Sep 2019 15:00:47 -
> @@ -56,6 +56,7 @@
>  #define SMTPD_BACKLOG 5
>  
>  #define  PATH_SMTPCTL"/usr/sbin/smtpctl"
> +#define PATH_FILTER  "/usr/local/libexec/smtpd:" _PATH_DEFPATH
>  
>  #define PATH_OFFLINE "/offline"
>  #define PATH_PURGE   "/purge"
> 



smtpd: change PATH for filters

2019-09-02 Thread Martijn van Duren
With filters most likely defaulting to /usr/local/libexec/smtpd in the  
near future I would like to add this as the default PATH, followed by
the usual suspects. I left the usual suspects in place because people
have shown they already implement filters in awk and probably will do
so in /bin/sh in the future, which will need all the other paths.

I put it in PATH_FILTER, so it can easily be altered for portable
where sysadmins may decide to change the path for filters or for
us if we ever want anything in base under /usr/libexec/smtpd.

OK?

martijn@

Index: smtpd.c
===
RCS file: /cvs/src/usr.sbin/smtpd/smtpd.c,v
retrieving revision 1.324
diff -u -p -r1.324 smtpd.c
--- smtpd.c 26 Jul 2019 07:08:34 -  1.324
+++ smtpd.c 2 Sep 2019 15:00:47 -
@@ -1350,6 +1350,7 @@ fork_processor(const char *name, const c
 */
if (read(STDERR_FILENO, &buf, 1) != 0)
errx(1, "lka didn't properly close write end of error socket");
+   setenv("PATH", PATH_FILTER, 1);
if (system(command) == -1)
err(1, NULL);
 
Index: smtpd.h
===
RCS file: /cvs/src/usr.sbin/smtpd/smtpd.h,v
retrieving revision 1.633
diff -u -p -r1.633 smtpd.h
--- smtpd.h 28 Aug 2019 15:50:36 -  1.633
+++ smtpd.h 2 Sep 2019 15:00:47 -
@@ -56,6 +56,7 @@
 #define SMTPD_BACKLOG   5
 
 #definePATH_SMTPCTL"/usr/sbin/smtpctl"
+#define PATH_FILTER"/usr/local/libexec/smtpd:" _PATH_DEFPATH
 
 #define PATH_OFFLINE   "/offline"
 #define PATH_PURGE "/purge"



Re: sxiccmu - A64 support

2019-09-02 Thread Krystian Lewandowski
I just checked 100us on Pinebook one more time,
the boot process is stuck at "root on sd0a [...]" - I think it's the first
time when CPU clock is set. Lowest value (round to 100) that was working
for me was 200us.

But if you think it should be smaller (even 1us like on H3/H5 in this driver
already) then it's fine with me. I can use different value for this specific
machine (I didn't have any problems with 1us on A64+ if I can recall). It's
not a problem to modify one value if the code is already there, which would
be nice.

Thank you for looking into the patch,
-- 
Krystian



Re: sxiccmu - A64 support

2019-09-02 Thread Mark Kettenis
> Date: Sat, 31 Aug 2019 23:51:06 +0200
> From: Krystian Lewandowski 
> 
> I just checked 100us on Pinebook one more time,
> the boot process is stuck at "root on sd0a [...]" - I think it's the first
> time when CPU clock is set. Lowest value (round to 100) that was working
> for me was 200us.
> 
> But if you think it should be smaller (even 1us like on H3/H5 in this driver
> already) then it's fine with me. I can use different value for this specific
> machine (I didn't have any problems with 1us on A64+ if I can recall). It's
> not a problem to modify one value if the code is already there, which would
> be nice.

No worries. If 200 us is the smallest value that works, that's the
value we need to use.  I've committed the patch.  Thanks!



Re: amd64, i386: remove gcc from sys Makefiles

2019-09-02 Thread Christian Weisgerber
On 2019-09-01, Christian Weisgerber  wrote:

> This removes the support for building kernels and boot loaders with
> gcc on amd64 and i386, simplifying the corresponding Makefiles.

daniel@ has objected directly to me that he'd like to continue to
be able to build the tree with ports gcc for testing purposes and
that coverity analysis for the kernel depends on gcc support.

-- 
Christian "naddy" Weisgerber  na...@mips.inka.de



Re: snmp(1): Add SNMPv3/USM support [5/5]

2019-09-02 Thread Martijn van Duren
This diff adds support for the HP laserjet. The problem with this device
is that it only returns the engineid on probing, but only returns boots 
and time after a packet has been send with full auth/enc.

It's not an intrusive diff and allows us to easily walk this device
without resorting to tcpdump for the information and setting it via -e
and -Z, so I think it's worth having it.

diff --git a/usm.c b/usm.c
index ba2020c..f34a6c9 100644
--- a/usm.c
+++ b/usm.c
@@ -142,6 +142,12 @@ usm_doinit(struct snmp_agent *agent)
agent->v3->level = level;
usm->userlen = userlen;
 
+   /* Ugly hack for HP Laserjet */
+   if (!usm->engineidset || !usm->bootsset || !usm->timeset) {
+   if ((ber = snmp_get(agent, NULL, 0)) == NULL)
+   return -1;
+   ber_free_element(ber);
+   }
return 0;
 }
 
@@ -395,6 +401,14 @@ usm_parseparams(struct snmp_agent *agent, char *packet, 
size_t packetlen,
goto fail;
}
}
+   /*
+* Don't assume these are set if both are zero.
+* Ugly hack for HP Laserjet
+*/
+   if (usm->boots == 0 && usm->time == 0) {
+   usm->bootsset = 0;
+   usm->timeset = 0;
+   }
 
if (userlen != usm->userlen ||
memcmp(user, usm->user, userlen) != 0)



Re: snmp(1): Add SNMPv3/USM support [4/5]

2019-09-02 Thread Martijn van Duren
This diff adds support for authPriv.

diff --git a/snmp.1 b/snmp.1
index e810560..fe283a5 100644
--- a/snmp.1
+++ b/snmp.1
@@ -28,6 +28,7 @@
 .Op Fl c Ar community
 .Op Fl E Ar ctxengineid
 .Op Fl e Ar secengineid
+.Op Fl K Ar localpriv
 .Op Fl k Ar localauth
 .Op Fl l Ar seclevel
 .Op Fl n Ar ctxname
@@ -36,6 +37,8 @@
 .Op Fl t Ar timeout
 .Op Fl u Ar user
 .Op Fl v Ar version
+.Op Fl X Ar privpass
+.Op Fl x Ar cipher
 .Op Fl Z Ar boots , Ns Ar time
 .Ar agent
 .Ar oid ...
@@ -46,6 +49,7 @@
 .Op Fl c Ar community
 .Op Fl E Ar ctxengineid
 .Op Fl e Ar secengineid
+.Op Fl K Ar localpriv
 .Op Fl k Ar localauth
 .Op Fl l Ar seclevel
 .Op Fl n Ar ctxname
@@ -54,6 +58,8 @@
 .Op Fl t Ar timeout
 .Op Fl u Ar user
 .Op Fl v Ar version
+.Op Fl X Ar privpass
+.Op Fl x Ar cipher
 .Op Fl Z Ar boots , Ns Ar time
 .Op Fl C Cm cIipt
 .Op Fl C Cm E Ar endoid
@@ -66,6 +72,7 @@
 .Op Fl c Ar community
 .Op Fl E Ar ctxengineid
 .Op Fl e Ar secengineid
+.Op Fl K Ar localpriv
 .Op Fl k Ar localauth
 .Op Fl l Ar seclevel
 .Op Fl n Ar ctxname
@@ -74,6 +81,8 @@
 .Op Fl t Ar timeout
 .Op Fl u Ar user
 .Op Fl v Ar version
+.Op Fl X Ar privpass
+.Op Fl x Ar cipher
 .Op Fl Z Ar boots , Ns Ar time
 .Op Fl C Cm n Ns Ar nonrep Ns Cm r Ns Ar maxrep
 .Ar agent
@@ -85,6 +94,7 @@
 .Op Fl c Ar community
 .Op Fl E Ar ctxengineid
 .Op Fl e Ar secengineid
+.Op Fl K Ar localpriv
 .Op Fl k Ar localauth
 .Op Fl l Ar seclevel
 .Op Fl n Ar ctxname
@@ -93,6 +103,8 @@
 .Op Fl t Ar timeout
 .Op Fl u Ar user
 .Op Fl v Ar version
+.Op Fl X Ar privpass
+.Op Fl x Ar cipher
 .Op Fl Z Ar boots , Ns Ar time
 .Op Fl C Cm cipn Ns Ar nonrep Ns Cm r Ns Ar maxrep
 .Ar agent
@@ -104,6 +116,7 @@
 .Op Fl c Ar community
 .Op Fl E Ar ctxengineid
 .Op Fl e Ar secengineid
+.Op Fl K Ar localpriv
 .Op Fl k Ar localauth
 .Op Fl l Ar seclevel
 .Op Fl n Ar ctxname
@@ -111,6 +124,8 @@
 .Op Fl t Ar timeout
 .Op Fl u Ar user
 .Op Fl v Ar version
+.Op Fl X Ar privpass
+.Op Fl x Ar cipher
 .Op Fl Z Ar boots , Ns Ar time
 .Ar agent uptime trapoid
 .Oo Ar varoid type value Oc ...
@@ -302,6 +317,14 @@ The snmpv3 context engine id.
 Most of the time this value can be safely ignored.
 This option is only used by
 .Fl v Cm 3 .
+.It Fl K Ar localpriv
+The localized privacy password for the user in hexadecimal format
+.Po
+optionally prefixed with a
+.Cm 0x
+.Pc .
+This option is only used by
+.Fl v Cm 3 .
 .It Fl k Ar localauth
 The localized authentication password for the user in hexadecimal format
 .Po
@@ -313,14 +336,24 @@ This option is only used by
 .It Fl l Ar seclevel
 The security level.
 Values can be
-.Cm noAuthNoPriv Pq default
-or
+.Cm noAuthNoPriv Pq default ,
 .Cm authNoPriv
 .Po
 requires either
 .Fl A
 or
 .Fl k
+.Pc
+or
+.Cm authPriv
+.Po
+requires either
+.Fl X
+or
+.Fl K
+in addition to the
+.Cm authNoPriv
+requirements
 .Pc .
 This option is only used by
 .Fl v Cm 3 .
@@ -382,6 +415,22 @@ or
 .Cm 3 .
 Currently defaults to
 .Cm 2c .
+.It Fl X Ar privpass
+The privacy password for the user.
+This will be tansformed to
+.Ar localpriv .
+This option is only used by
+.Fl v Cm 3 .
+.It Fl x Ar cipher
+Sets the cipher
+.Pq privacy
+protocol.
+Options are
+.Cm DES
+and
+.Cm AES .
+This option is only used by
+.Fl v Cm 3 .
 .It Fl Z Ar boots , Ns Ar time
 Set the engine boots and engine time.
 Under normal circumstances this value is discovered via snmpv3 discovery and
diff --git a/snmp.c b/snmp.c
index 57b40e3..ba4dc3c 100644
--- a/snmp.c
+++ b/snmp.c
@@ -357,7 +357,7 @@ static char *
 snmp_package(struct snmp_agent *agent, struct ber_element *pdu, size_t *len)
 {
struct ber ber;
-   struct ber_element *message, *scopedpdu = NULL, *secparams;
+   struct ber_element *message, *scopedpdu = NULL, *secparams, *encpdu;
ssize_t securitysize, ret;
size_t secparamsoffset;
char *securityparams = NULL, *buf, *packet = NULL;
@@ -400,6 +400,13 @@ snmp_package(struct snmp_agent *agent, struct ber_element 
*pdu, size_t *len)
ber_free_elements(scopedpdu);
goto fail;
}
+   if (agent->v3->level & SNMP_MSGFLAG_PRIV) {
+   if ((encpdu = agent->v3->sec->encpdu(agent, scopedpdu,
+   cookie)) == NULL)
+   goto fail;
+   ber_free_elements(scopedpdu);
+   scopedpdu = encpdu;
+   }
if (ber_printf_elements(message, "d{idxd}xe",
agent->version, msgid, 1472, &(agent->v3->level),
(size_t) 1, agent->v3->sec->model, securityparams,
@@ -449,8 +456,9 @@ snmp_unpackage(struct snmp_agent *agent, char *buf, size_t 
buflen)
size_t msgflagslen, secparamslen;
struct ber_element *message = NULL, *payload, *scopedpdu, *ctxname;
off_t secparamsoffset;
-   char *engineid;
-   size_t engineidlen;
+   char *encpdu, *engineid;
+   size_t encpdulen, engineidlen;
+   void *cookie = NULL;

Re: snmp(1): Add SNMPv3/USM support [3/5]

2019-09-02 Thread Martijn van Duren
This diff adds support for authNoPriv.

diff --git a/Makefile b/Makefile
index 9eb684b..102582b 100644
--- a/Makefile
+++ b/Makefile
@@ -2,8 +2,8 @@
 
 PROG=  snmp
 SRCS=  mib.c smi.c snmp.c snmpc.c usm.c
-LDADD+=-lutil
-DPADD+=${LIBUTIL}
+LDADD+=-lcrypto -lutil
+DPADD+=${LIBCRYPTO} ${LIBUTIL}
 
 MAN=   snmp.1
 
diff --git a/snmp.1 b/snmp.1
index 523fd16..e810560 100644
--- a/snmp.1
+++ b/snmp.1
@@ -23,9 +23,13 @@
 .Sh SYNOPSIS
 .Nm
 .Cm get | getnext
+.Op Fl A Ar authpass
+.Op Fl a Ar digest
 .Op Fl c Ar community
 .Op Fl E Ar ctxengineid
 .Op Fl e Ar secengineid
+.Op Fl k Ar localauth
+.Op Fl l Ar seclevel
 .Op Fl n Ar ctxname
 .Op Fl O Cm afnQqSvx
 .Op Fl r Ar retries
@@ -37,9 +41,13 @@
 .Ar oid ...
 .Nm
 .Cm walk
+.Op Fl A Ar authpass
+.Op Fl a Ar digest
 .Op Fl c Ar community
 .Op Fl E Ar ctxengineid
 .Op Fl e Ar secengineid
+.Op Fl k Ar localauth
+.Op Fl l Ar seclevel
 .Op Fl n Ar ctxname
 .Op Fl O Cm afnQqSvx
 .Op Fl r Ar retries
@@ -53,9 +61,13 @@
 .Op Ar oid
 .Nm
 .Cm bulkget
+.Op Fl A Ar authpass
+.Op Fl a Ar digest
 .Op Fl c Ar community
 .Op Fl E Ar ctxengineid
 .Op Fl e Ar secengineid
+.Op Fl k Ar localauth
+.Op Fl l Ar seclevel
 .Op Fl n Ar ctxname
 .Op Fl O Cm afnQqSvx
 .Op Fl r Ar retries
@@ -68,9 +80,13 @@
 .Ar oid ...
 .Nm
 .Cm bulkwalk
+.Op Fl A Ar authpass
+.Op Fl a Ar digest
 .Op Fl c Ar community
 .Op Fl E Ar ctxengineid
 .Op Fl e Ar secengineid
+.Op Fl k Ar localauth
+.Op Fl l Ar seclevel
 .Op Fl n Ar ctxname
 .Op Fl O Cm afnQqSvx
 .Op Fl r Ar retries
@@ -83,9 +99,13 @@
 .Op Ar oid
 .Nm
 .Cm trap
+.Op Fl A Ar authpass
+.Op Fl a Ar digest
 .Op Fl c Ar community
 .Op Fl E Ar ctxengineid
 .Op Fl e Ar secengineid
+.Op Fl k Ar localauth
+.Op Fl l Ar seclevel
 .Op Fl n Ar ctxname
 .Op Fl r Ar retries
 .Op Fl t Ar timeout
@@ -170,6 +190,28 @@ Dump the tree of compiled-in MIB objects.
 .Pp
 The options are as follows:
 .Bl -tag -width Ds
+.It Fl A Ar authpass
+The authentication password for the user.
+This will be transformed to
+.Ar localauth .
+This option is only used by
+.Fl v Cm 3 .
+.It Fl a Ar digest
+Set the digest
+.Pq authentication
+protocol.
+Options are
+.Cm MD5 ,
+.Cm SHA ,
+.Cm SHA-224 ,
+.Cm SHA-256 ,
+.Cm SHA-384
+or
+.Cm SHA-512 .
+This option defaults to
+.Cm MD5 .
+This option is only used by
+.Fl v Cm 3 .
 .It Fl C Ar appopt
 Set the application specific
 .Ar appopt
@@ -260,6 +302,28 @@ The snmpv3 context engine id.
 Most of the time this value can be safely ignored.
 This option is only used by
 .Fl v Cm 3 .
+.It Fl k Ar localauth
+The localized authentication password for the user in hexadecimal format
+.Po
+optionally prefixed with a
+.Cm 0x
+.Pc .
+This option is only used by
+.Fl v Cm 3 .
+.It Fl l Ar seclevel
+The security level.
+Values can be
+.Cm noAuthNoPriv Pq default
+or
+.Cm authNoPriv
+.Po
+requires either
+.Fl A
+or
+.Fl k
+.Pc .
+This option is only used by
+.Fl v Cm 3 .
 .It Fl n Ar ctxname
 Sets the context name.
 Defaults to an empty string.
diff --git a/snmp.c b/snmp.c
index 7165976..57b40e3 100644
--- a/snmp.c
+++ b/snmp.c
@@ -37,6 +37,7 @@ static char *
 static struct ber_element *
 snmp_unpackage(struct snmp_agent *, char *, size_t);
 static void snmp_v3_free(struct snmp_v3 *);
+static void snmp_v3_secparamsoffset(void *, size_t);
 
 struct snmp_v3 *
 snmp_v3_init(int level, const char *ctxname, size_t ctxnamelen,
@@ -356,10 +357,12 @@ static char *
 snmp_package(struct snmp_agent *agent, struct ber_element *pdu, size_t *len)
 {
struct ber ber;
-   struct ber_element *message, *scopedpdu = NULL;
+   struct ber_element *message, *scopedpdu = NULL, *secparams;
ssize_t securitysize, ret;
+   size_t secparamsoffset;
char *securityparams = NULL, *buf, *packet = NULL;
long long msgid;
+   void *cookie = NULL;
 
bzero(&ber, sizeof(ber));
ber_set_application(&ber, smi_application);
@@ -393,7 +396,7 @@ snmp_package(struct snmp_agent *agent, struct ber_element 
*pdu, size_t *len)
}
pdu = NULL;
if ((securityparams = agent->v3->sec->genparams(agent,
-   &securitysize)) == NULL) {
+   &securitysize, &cookie)) == NULL) {
ber_free_elements(scopedpdu);
goto fail;
}
@@ -402,6 +405,10 @@ snmp_package(struct snmp_agent *agent, struct ber_element 
*pdu, size_t *len)
(size_t) 1, agent->v3->sec->model, securityparams,
securitysize, scopedpdu) == NULL)
goto fail;
+   if (ber_scanf_elements(message, "{SSe", &secparams) == -1)
+   goto fail;
+   ber_set_writecallback(secparams, snmp_v3_secparamsoffset,
+   &secparamsoffset);
break;
}
 
@@ -413,7 +420,17 @@ snmp_package(struct snmp_agent *agent, struct ber_element 
*pdu, size_t *len)

Re: snmp(1): Add SNMPv3/USM support [2/5]

2019-09-02 Thread Martijn van Duren
Here's the diff that adds initial SNMPv3 support with USM noAuthNoPriv
support.

diff --git a/Makefile b/Makefile
index 62bb556..9eb684b 100644
--- a/Makefile
+++ b/Makefile
@@ -1,7 +1,7 @@
 #  $OpenBSD: Makefile,v 1.1 2019/08/09 06:17:59 martijn Exp $
 
 PROG=  snmp
-SRCS=  mib.c smi.c snmp.c snmpc.c
+SRCS=  mib.c smi.c snmp.c snmpc.c usm.c
 LDADD+=-lutil
 DPADD+=${LIBUTIL}
 
diff --git a/snmp.1 b/snmp.1
index e158ba0..523fd16 100644
--- a/snmp.1
+++ b/snmp.1
@@ -24,19 +24,29 @@
 .Nm
 .Cm get | getnext
 .Op Fl c Ar community
+.Op Fl E Ar ctxengineid
+.Op Fl e Ar secengineid
+.Op Fl n Ar ctxname
+.Op Fl O Cm afnQqSvx
 .Op Fl r Ar retries
 .Op Fl t Ar timeout
+.Op Fl u Ar user
 .Op Fl v Ar version
-.Op Fl O Cm afnQqSvx
+.Op Fl Z Ar boots , Ns Ar time
 .Ar agent
 .Ar oid ...
 .Nm
 .Cm walk
 .Op Fl c Ar community
+.Op Fl E Ar ctxengineid
+.Op Fl e Ar secengineid
+.Op Fl n Ar ctxname
+.Op Fl O Cm afnQqSvx
 .Op Fl r Ar retries
 .Op Fl t Ar timeout
+.Op Fl u Ar user
 .Op Fl v Ar version
-.Op Fl O Cm afnQqSvx
+.Op Fl Z Ar boots , Ns Ar time
 .Op Fl C Cm cIipt
 .Op Fl C Cm E Ar endoid
 .Ar agent
@@ -44,29 +54,44 @@
 .Nm
 .Cm bulkget
 .Op Fl c Ar community
+.Op Fl E Ar ctxengineid
+.Op Fl e Ar secengineid
+.Op Fl n Ar ctxname
+.Op Fl O Cm afnQqSvx
 .Op Fl r Ar retries
 .Op Fl t Ar timeout
+.Op Fl u Ar user
 .Op Fl v Ar version
-.Op Fl O Cm afnQqSvx
+.Op Fl Z Ar boots , Ns Ar time
 .Op Fl C Cm n Ns Ar nonrep Ns Cm r Ns Ar maxrep
 .Ar agent
 .Ar oid ...
 .Nm
 .Cm bulkwalk
 .Op Fl c Ar community
+.Op Fl E Ar ctxengineid
+.Op Fl e Ar secengineid
+.Op Fl n Ar ctxname
+.Op Fl O Cm afnQqSvx
 .Op Fl r Ar retries
 .Op Fl t Ar timeout
+.Op Fl u Ar user
 .Op Fl v Ar version
-.Op Fl O Cm afnQqSvx
+.Op Fl Z Ar boots , Ns Ar time
 .Op Fl C Cm cipn Ns Ar nonrep Ns Cm r Ns Ar maxrep
 .Ar agent
 .Op Ar oid
 .Nm
 .Cm trap
 .Op Fl c Ar community
+.Op Fl E Ar ctxengineid
+.Op Fl e Ar secengineid
+.Op Fl n Ar ctxname
 .Op Fl r Ar retries
 .Op Fl t Ar timeout
+.Op Fl u Ar user
 .Op Fl v Ar version
+.Op Fl Z Ar boots , Ns Ar time
 .Ar agent uptime trapoid
 .Oo Ar varoid type value Oc ...
 .Nm
@@ -220,6 +245,26 @@ Set the
 string.
 Defaults to
 .Cm public .
+This option is only used by
+.Fl v Cm 1
+and
+.Fl v Cm 2c .
+.It Fl e Ar secengineid
+The USM security engine id.
+Under normal circumstances this value is discovered via snmpv3 discovery and
+does not need to be specified.
+This option is only used by
+.Fl v Cm 3 .
+.It Fl E Ar ctxengineid
+The snmpv3 context engine id.
+Most of the time this value can be safely ignored.
+This option is only used by
+.Fl v Cm 3 .
+.It Fl n Ar ctxname
+Sets the context name.
+Defaults to an empty string.
+This option is only used by
+.Fl v Cm 3 .
 .It Fl O Ar output
 Set the
 .Ar output
@@ -256,15 +301,29 @@ Set the
 .Ar timeout
 to wait for a reply, in seconds.
 Defaults to 1.
+.It Fl u Ar user
+Sets the username.
+If
+.Fl v Cm 3
+is used this option is required.
+This option is only used by
+.Fl v Cm 3 .
 .It Fl v Ar version
 Set the snmp protocol
 .Ar version
 to either
-.Cm 1
+.Cm 1 ,
+.Cm 2c
 or
-.Cm 2c .
+.Cm 3 .
 Currently defaults to
 .Cm 2c .
+.It Fl Z Ar boots , Ns Ar time
+Set the engine boots and engine time.
+Under normal circumstances this value is discovered via snmpv3 discovery and
+does not need to be specified.
+This option is only used by
+.Fl v Cm 3 .
 .El
 .Pp
 The syntax for the
diff --git a/snmp.c b/snmp.c
index b2d5cfc..7165976 100644
--- a/snmp.c
+++ b/snmp.c
@@ -36,6 +36,47 @@ static char *
 snmp_package(struct snmp_agent *, struct ber_element *, size_t *);
 static struct ber_element *
 snmp_unpackage(struct snmp_agent *, char *, size_t);
+static void snmp_v3_free(struct snmp_v3 *);
+
+struct snmp_v3 *
+snmp_v3_init(int level, const char *ctxname, size_t ctxnamelen,
+struct snmp_sec *sec)
+{
+   struct snmp_v3 *v3;
+
+   if ((level & (SNMP_MSGFLAG_SECMASK | SNMP_MSGFLAG_REPORT)) != level ||
+   sec == NULL) {
+   errno = EINVAL;
+   return NULL;
+   }
+   if ((v3 = calloc(1, sizeof(*v3))) == NULL)
+   return NULL;
+
+   v3->level = level | SNMP_MSGFLAG_REPORT;
+   v3->ctxnamelen = ctxnamelen;
+   if (ctxnamelen != 0) {
+   if ((v3->ctxname = malloc(ctxnamelen)) == NULL) {
+   free(v3);
+   return NULL;
+   }
+   memcpy(v3->ctxname, ctxname, ctxnamelen);
+   }
+   v3->sec = sec;
+   return v3;
+}
+
+int
+snmp_v3_setengineid(struct snmp_v3 *v3, char *engineid, size_t engineidlen)
+{
+   if (v3->engineidset)
+   free(v3->engineid);
+   if ((v3->engineid = malloc(engineidlen)) == NULL)
+   return -1;
+   memcpy(v3->engineid, engineid, engineidlen);
+   v3->engineidlen = engineidlen;
+   v3->engineidset = 1;
+   return 0;
+}
 
 struct snmp_agent *
 snmp_connect_v12(int fd, enum snmp_version version, const 

Re: snmp(1): Add SNMPv3/USM support [1/5]

2019-09-02 Thread Martijn van Duren
Here's a diff that restructures packet handling to allow easier addition
of SNMPv3.

diff --git a/snmp.c b/snmp.c
index 7fac777..b2d5cfc 100644
--- a/snmp.c
+++ b/snmp.c
@@ -32,6 +32,10 @@
 
 static struct ber_element *
 snmp_resolve(struct snmp_agent *, struct ber_element *, int);
+static char *
+snmp_package(struct snmp_agent *, struct ber_element *, size_t *);
+static struct ber_element *
+snmp_unpackage(struct snmp_agent *, char *, size_t);
 
 struct snmp_agent *
 snmp_connect_v12(int fd, enum snmp_version version, const char *community)
@@ -171,19 +175,16 @@ fail:
 static struct ber_element *
 snmp_resolve(struct snmp_agent *agent, struct ber_element *pdu, int reply)
 {
-   struct ber_element *message, *varbind;
+   struct ber_element *varbind;
struct ber_oid oid;
struct timespec start, now;
struct pollfd pfd;
-   struct ber ber;
+   char *message;
ssize_t len;
long long reqid, rreqid;
-   long long version;
-   char *community;
short direction;
int to, nfds, ret;
int tries;
-   void *ptr;
char buf[READ_BUF_SIZE];
 
if (ber_scanf_elements(pdu, "{i", &reqid) != 0) {
@@ -192,23 +193,8 @@ snmp_resolve(struct snmp_agent *agent, struct ber_element 
*pdu, int reply)
return NULL;
}
 
-   if ((message = ber_add_sequence(NULL)) == NULL) {
-   ber_free_elements(pdu);
-   return NULL;
-   }
-   if (ber_printf_elements(message, "dse", agent->version,
-   agent->community, pdu) == NULL) {
-   ber_free_elements(pdu);
-   ber_free_elements(message);
+   if ((message = snmp_package(agent, pdu, &len)) == NULL)
return NULL;
-   }
-   memset(&ber, 0, sizeof(ber));
-   ber_set_application(&ber, smi_application);
-   len = ber_write_elements(&ber, message);
-   ber_free_elements(message);
-   message = NULL;
-   if (ber_get_writebuf(&ber, &ptr) < 1)
-   goto fail;
 
clock_gettime(CLOCK_MONOTONIC, &start);
memcpy(&now, &start, sizeof(now));
@@ -236,7 +222,7 @@ snmp_resolve(struct snmp_agent *agent, struct ber_element 
*pdu, int reply)
goto fail;
}
if (direction == POLLOUT) {
-   ret = send(agent->fd, ptr, len, MSG_DONTWAIT);
+   ret = send(agent->fd, message, len, MSG_DONTWAIT);
if (ret == -1)
goto fail;
if (ret < len) {
@@ -253,25 +239,10 @@ snmp_resolve(struct snmp_agent *agent, struct ber_element 
*pdu, int reply)
errno = ECONNRESET;
if (ret <= 0)
goto fail;
-   ber_set_readbuf(&ber, buf, ret);
-   if ((message = ber_read_elements(&ber, NULL)) == NULL) {
-   direction = POLLOUT;
+   if ((pdu = snmp_unpackage(agent, buf, ret)) == NULL) {
tries--;
-   continue;
-   }
-   if (ber_scanf_elements(message, "{ise", &version, &community,
-   &pdu) != 0) {
-   errno = EPROTO;
direction = POLLOUT;
-   tries--;
-   continue;
-   }
-   /* Skip invalid packets; should not happen */
-   if (version != agent->version ||
-   strcmp(community, agent->community) != 0) {
errno = EPROTO;
-   direction = POLLOUT;
-   tries--;
continue;
}
/* Validate pdu format and check request id */
@@ -297,17 +268,96 @@ snmp_resolve(struct snmp_agent *agent, struct ber_element 
*pdu, int reply)
break;
}
}
-   if (varbind != NULL)
-   continue;
 
-   ber_unlink_elements(message->be_sub->be_next);
-   ber_free_elements(message);
-   ber_free(&ber);
+   free(message);
return pdu;
}
 
+fail:
+   free(message);
+   return NULL;
+}
+
+static char *
+snmp_package(struct snmp_agent *agent, struct ber_element *pdu, size_t *len)
+{
+   struct ber ber;
+   struct ber_element *message;
+   ssize_t ret;
+   char *buf, *packet = NULL;
+
+   bzero(&ber, sizeof(ber));
+   ber_set_application(&ber, smi_application);
+
+   if ((message = ber_add_sequence(NULL)) == NULL) {
+   ber_free_elements(pdu);
+   goto fail;
+   }
+
+   switch (agent->version) {
+   case SNMP_V1:
+   case SNMP_V2C:
+   if (ber_printf_elements(message, "dse", agent->version,
+   agent->community, pdu) == NULL) {

snmp(1): Add SNMPv3/USM support [0/5]

2019-09-02 Thread Martijn van Duren
Hello tech@,

I've worked hard to get SNMPv3 support for snmp(1). Here's the end
result. This mail contains the full diff for people just wanting to
test, the follow up mails will contain the incremental diffs (still
massive beasts) so they're easier to review.

I've implemented Net-SNMP's -A, -a, -E, -e, -3K (as -K), -3k (as -k),
-l, -n, -u, -X, -x and -Z. I choose to leave out the -3 because the
way to handle this is really ugly and -K and -k are unused by
Net-SNMP.
This is also the reason there's no support for master keys, because -m
and -M are used by Net-SNMP and I don't know if I want to implement that
at some point and I haven't seen a device that needs the master key. The
scaffolding for adding master key support is there and if someone has
an actual usecase for it and comes up with a good suggestion for a flag
I'll be happy to implement it.

Tested with snmpd(8), netsnmpd and HP Laserjet 4730mfp by me and
netgear GS724Tv4 ProSafe by semarie@ on previous iteration of diff.

Tests, feedback, OKs welcome.

martijn@

diff --git a/Makefile b/Makefile
index 62bb556..102582b 100644
--- a/Makefile
+++ b/Makefile
@@ -1,9 +1,9 @@
 #  $OpenBSD: Makefile,v 1.1 2019/08/09 06:17:59 martijn Exp $
 
 PROG=  snmp
-SRCS=  mib.c smi.c snmp.c snmpc.c
-LDADD+=-lutil
-DPADD+=${LIBUTIL}
+SRCS=  mib.c smi.c snmp.c snmpc.c usm.c
+LDADD+=-lcrypto -lutil
+DPADD+=${LIBCRYPTO} ${LIBUTIL}
 
 MAN=   snmp.1
 
diff --git a/snmp.1 b/snmp.1
index e158ba0..fe283a5 100644
--- a/snmp.1
+++ b/snmp.1
@@ -23,50 +23,110 @@
 .Sh SYNOPSIS
 .Nm
 .Cm get | getnext
+.Op Fl A Ar authpass
+.Op Fl a Ar digest
 .Op Fl c Ar community
+.Op Fl E Ar ctxengineid
+.Op Fl e Ar secengineid
+.Op Fl K Ar localpriv
+.Op Fl k Ar localauth
+.Op Fl l Ar seclevel
+.Op Fl n Ar ctxname
+.Op Fl O Cm afnQqSvx
 .Op Fl r Ar retries
 .Op Fl t Ar timeout
+.Op Fl u Ar user
 .Op Fl v Ar version
-.Op Fl O Cm afnQqSvx
+.Op Fl X Ar privpass
+.Op Fl x Ar cipher
+.Op Fl Z Ar boots , Ns Ar time
 .Ar agent
 .Ar oid ...
 .Nm
 .Cm walk
+.Op Fl A Ar authpass
+.Op Fl a Ar digest
 .Op Fl c Ar community
+.Op Fl E Ar ctxengineid
+.Op Fl e Ar secengineid
+.Op Fl K Ar localpriv
+.Op Fl k Ar localauth
+.Op Fl l Ar seclevel
+.Op Fl n Ar ctxname
+.Op Fl O Cm afnQqSvx
 .Op Fl r Ar retries
 .Op Fl t Ar timeout
+.Op Fl u Ar user
 .Op Fl v Ar version
-.Op Fl O Cm afnQqSvx
+.Op Fl X Ar privpass
+.Op Fl x Ar cipher
+.Op Fl Z Ar boots , Ns Ar time
 .Op Fl C Cm cIipt
 .Op Fl C Cm E Ar endoid
 .Ar agent
 .Op Ar oid
 .Nm
 .Cm bulkget
+.Op Fl A Ar authpass
+.Op Fl a Ar digest
 .Op Fl c Ar community
+.Op Fl E Ar ctxengineid
+.Op Fl e Ar secengineid
+.Op Fl K Ar localpriv
+.Op Fl k Ar localauth
+.Op Fl l Ar seclevel
+.Op Fl n Ar ctxname
+.Op Fl O Cm afnQqSvx
 .Op Fl r Ar retries
 .Op Fl t Ar timeout
+.Op Fl u Ar user
 .Op Fl v Ar version
-.Op Fl O Cm afnQqSvx
+.Op Fl X Ar privpass
+.Op Fl x Ar cipher
+.Op Fl Z Ar boots , Ns Ar time
 .Op Fl C Cm n Ns Ar nonrep Ns Cm r Ns Ar maxrep
 .Ar agent
 .Ar oid ...
 .Nm
 .Cm bulkwalk
+.Op Fl A Ar authpass
+.Op Fl a Ar digest
 .Op Fl c Ar community
+.Op Fl E Ar ctxengineid
+.Op Fl e Ar secengineid
+.Op Fl K Ar localpriv
+.Op Fl k Ar localauth
+.Op Fl l Ar seclevel
+.Op Fl n Ar ctxname
+.Op Fl O Cm afnQqSvx
 .Op Fl r Ar retries
 .Op Fl t Ar timeout
+.Op Fl u Ar user
 .Op Fl v Ar version
-.Op Fl O Cm afnQqSvx
+.Op Fl X Ar privpass
+.Op Fl x Ar cipher
+.Op Fl Z Ar boots , Ns Ar time
 .Op Fl C Cm cipn Ns Ar nonrep Ns Cm r Ns Ar maxrep
 .Ar agent
 .Op Ar oid
 .Nm
 .Cm trap
+.Op Fl A Ar authpass
+.Op Fl a Ar digest
 .Op Fl c Ar community
+.Op Fl E Ar ctxengineid
+.Op Fl e Ar secengineid
+.Op Fl K Ar localpriv
+.Op Fl k Ar localauth
+.Op Fl l Ar seclevel
+.Op Fl n Ar ctxname
 .Op Fl r Ar retries
 .Op Fl t Ar timeout
+.Op Fl u Ar user
 .Op Fl v Ar version
+.Op Fl X Ar privpass
+.Op Fl x Ar cipher
+.Op Fl Z Ar boots , Ns Ar time
 .Ar agent uptime trapoid
 .Oo Ar varoid type value Oc ...
 .Nm
@@ -145,6 +205,28 @@ Dump the tree of compiled-in MIB objects.
 .Pp
 The options are as follows:
 .Bl -tag -width Ds
+.It Fl A Ar authpass
+The authentication password for the user.
+This will be transformed to
+.Ar localauth .
+This option is only used by
+.Fl v Cm 3 .
+.It Fl a Ar digest
+Set the digest
+.Pq authentication
+protocol.
+Options are
+.Cm MD5 ,
+.Cm SHA ,
+.Cm SHA-224 ,
+.Cm SHA-256 ,
+.Cm SHA-384
+or
+.Cm SHA-512 .
+This option defaults to
+.Cm MD5 .
+This option is only used by
+.Fl v Cm 3 .
 .It Fl C Ar appopt
 Set the application specific
 .Ar appopt
@@ -220,6 +302,66 @@ Set the
 string.
 Defaults to
 .Cm public .
+This option is only used by
+.Fl v Cm 1
+and
+.Fl v Cm 2c .
+.It Fl e Ar secengineid
+The USM security engine id.
+Under normal circumstances this value is discovered via snmpv3 discovery and
+does not need to be specified.
+This option is only used by
+.Fl v Cm 3 .
+.It Fl E Ar ctxengineid
+The snmpv3 context engine id.
+Most of the time this value can be sa