Re: Questions about syspatch(8) mtree(8) behaviour
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
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
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
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
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
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
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
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
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
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)
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
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
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
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
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)
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
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
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
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
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
> 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
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
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
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
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
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
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
> 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
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]
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]
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]
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]
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]
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]
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