Re: bug: fifo kqfilter & FREAD|FWRITE
On 07/04/20(Tue) 15:00, Todd C. Miller wrote: > On Tue, 07 Apr 2020 12:42:06 +0200, Martin Pieuchot wrote: > > > fifo_poll() honors FREAD and FWRITE. It doesn't return events > > incompatible with the open flags. Diff below makes the kqfilters > > behave like that as well. > > Looks good to me. Is there any reason you put "int a_fflag" in the > middle of struct vop_kqfilter_args in between two pointers? I just did like all other vop_*_args do, many of them have an int in between two pointers as second argument. I'm welcoming you to clean they all at once! Thanks for falling into my trap :O)
[PATCH] some NULL checks before free(3)
Ran a grep to find unneccessary NULL checks before free(3) and found
some in dhclient(8) and makefs(8).
Geoff Hill
Index: sbin/dhclient/dhclient.c
===
RCS file: /cvs/src/sbin/dhclient/dhclient.c,v
retrieving revision 1.659
diff -u -p -r1.659 dhclient.c
--- sbin/dhclient/dhclient.c26 Jan 2020 10:31:03 - 1.659
+++ sbin/dhclient/dhclient.c7 Apr 2020 23:38:50 -
@@ -2315,8 +2315,7 @@ fork_privchld(struct interface_info *ifi
go_daemon();
- if (log_procname != NULL)
- free(log_procname);
+ free(log_procname);
rslt = asprintf(&log_procname, "%s [priv]", ifi->name);
if (rslt == -1)
fatal("log_procname");
Index: usr.sbin/makefs/ffs.c
===
RCS file: /cvs/src/usr.sbin/makefs/ffs.c,v
retrieving revision 1.31
diff -u -p -r1.31 ffs.c
--- usr.sbin/makefs/ffs.c 21 Jan 2017 21:58:32 - 1.31
+++ usr.sbin/makefs/ffs.c 7 Apr 2020 23:38:52 -
@@ -448,8 +448,7 @@ ffs_create_image(const char *image, fsin
}
bufrem -= i;
}
- if (buf)
- free(buf);
+ free(buf);
/* make the file system */
if (Tflag) {
@@ -706,8 +705,7 @@ ffs_populate_dir(const char *dir, fsnode
}
/* cleanup */
- if (dirbuf.buf != NULL)
- free(dirbuf.buf);
+ free(dirbuf.buf);
return (1);
}
@@ -803,8 +801,7 @@ ffs_write_file(union dinode *din, uint32
ffs_write_inode(&in.i_din, in.i_number, fsopts);
leave_ffs_write_file:
- if (fbuf)
- free(fbuf);
+ free(fbuf);
if (ffd != -1)
close(ffd);
}
Re: bug: fifo kqfilter & FREAD|FWRITE
On Tue, 07 Apr 2020 12:42:06 +0200, Martin Pieuchot wrote: > fifo_poll() honors FREAD and FWRITE. It doesn't return events > incompatible with the open flags. Diff below makes the kqfilters > behave like that as well. Looks good to me. Is there any reason you put "int a_fflag" in the middle of struct vop_kqfilter_args in between two pointers? I suppose it doesn't really matter, you are going to lose 32 bits either way on 64-bit platforms due to struct alignment but it looks a bit odd. - todd
Re: fifo kqueue bug
On Tue, 07 Apr 2020 12:08:35 +0200, Martin Pieuchot wrote: > The write socket should be passed to the write filter otherwise checks > are performed against the wrong socket. OK millert@ - todd
Re: pppx(4): prevent concurency with pppx_if_destroy()
> On 7 Apr 2020, at 17:43, Martin Pieuchot wrote:
>
> On 07/04/20(Tue) 17:14, Vitaliy Makkoveev wrote:
>> As Claudio Jeker noticed, NET_LOCK() can release KERNEL_LOCK(). pppx(4)
>> code has some NET_LOCK() dances which make it unsafe. [...]
>
> The easiest way to fix that is to move if_detach() out of pppx_if_destroy().
pppxioctl() is under KERNEL_LOCK(). pppxioctl() holds NET_LOCK() it’s whole
runtime. So your suggestion is (pseudo code):
KERNEL_LOCK();
pppx_ioctl()
{
NET_LOCK();
switch() {
case PIPEXDSESSION:
NET_UNLOCK();
/*
* KERNEL_LOCK() can be released here and concurrent
* thread with PIPEXDSESSION case will enter here
*/
if_detach();
NET_LOCK();
pppx_if_destroy();
break;
}
NET_UNLOCK();
}
KERNEL_UNLOCK();
I understood well?
>
> It generally makes sense to call if_detach() first then free/close the
> descriptor of a driver. However some drivers have callbacks and in that
> case you might want to teardown those first then call if_detach().
>
> if_detach() will require the NET_LOCK() for some time. However
> pseudo-driver should start protecting their own data structure with
> different locks.
>
Re: FW: Add mprotect_ept ioctl to vmm(4)
* Adam Steen [2020-04-07 08:18:19 +]:
On Fri, Feb 07, 2020 at 01:25:38PM -0800, Mike Larkin wrote:
> On Fri, Feb 07, 2020 at 04:20:16AM +, Adam Steen wrote:
> > Hi
> >
> > Please see the attached patch to add an 'IOCTL handler to sets the access
> > protections of the ept'
> >
> > vmd(8) does not make use of this change, but solo5, which uses vmm(4) as
> > a backend hypervisor. The code calling 'VMM_IOC_MPROTECT_EPT' is
> > available here
https://github.com/Solo5/solo5/compare/master...adamsteen:wnox
> >
> > there are changes to vmd too, but this is just to ensure completeness,
> > if mprotect ept is called in the future, we would want the vm to be
> > stopped if we get a protection fault.
> >
> > I was unsure what todo if called with execute only permissions on a cpu that
> > does not support it. I went with add read permissions and logging the
> > fact, instead of returning EINVAL.
> >
> > Cheers
> > Adam
> >
>
> I have been giving Adam feedback on this diff for a while. There are a few
> minor comments below, but I think this is ok if someone wants to commit it
after
> the fixes below are incorporated.
>
> -ml
>
See updated comment below.
-ml
> > + /* No W^X permissions */
> > + if ((prot & PROT_MASK) != prot &&
> > + (prot & (PROT_WRITE | PROT_EXEC)) == (PROT_WRITE | PROT_EXEC)) {
> > + DPRINTF("%s: No W^X permissions\n", __func__);
> > + return (EINVAL);
> > + }
>
> I would probably reword this to "W+X permission requested".
>
> > +/*
> > + * vmx_mprotect_ept
> > + *
> > + * apply the ept protections to the requested pages, faulting the page if
>
> "faulting in"
>
> > + * required.
> > + */
> > +int
> > +vmx_mprotect_ept(vm_map_t vm_map, paddr_t sgpa, paddr_t egpa, int prot)
> > +{
> > + struct vmx_invept_descriptor vid;
> > + pmap_t pmap;
> > + pt_entry_t *pte;
> > + paddr_t addr;
> > + int ret = 0;
> > > +
> > + pmap = vm_map->pmap;
> > +
> > + for (addr = sgpa; addr < egpa; addr += PAGE_SIZE) {
> > + pte = vmx_pmap_find_pte_ept(pmap, addr);
> > + if (pte == NULL) {
>
> if (pte & PG_V) == 0
>
After reading a reply from Adam, I think the original suggested way is fine.
My idea of checking PG_V is fine for RWX EPT entries, but as soon as anyone
uses XO entries, this check wouldn't work.
-ml
Hi
I have incorporated the fixes ml requested above and a fixed a few nits
from pd@, but with the crazyness of life these days, i haven't been able to it
commited, so i attaching it again below.
tested ok last week. I can commit it (this and cr0 one) tonight and
watch for any fallout
Thanks and apologies for delay!
--
Pratik
Re: Include /var/www/tmp into base install
On Tue, Apr 07, 2020 at 06:13:12PM +0200, Stefan Sperling wrote: > For temp stuff we really need a separate space that can just be wiped > without consequences when it has run full. The way Got internally provides access to files in /tmp for every helper process is to pass one or more open file descriptors for files in /tmp from the main process to the helper process. The helper doesn't have any direct filesystem access. Of course it could fill up /tmp with this one file, but not /var. It would be nice if e.g. slowcgi could provide something like that. At present gotweb uses /var/www/got/tmp instead of /tmp because its main process runs in /var/www chroot via slowcgi.
Re: Include /var/www/tmp into base install
For me, the "/var is full" problem can be adequately mitigated by mounting a separate partition as /var/tmp. More of an issue, although obviously not major - if there are a large number of tmp directories, is making sure that they are all routinely purged. Yes, I know this is down to careless admin practice, but it happened to me earlier this year. I can't give them all their own partitions. On Tue, 7 Apr 2020 at 16:58, Theo de Raadt wrote: > Stefan Sperling wrote: > > > On Tue, Apr 07, 2020 at 09:37:02AM -0600, Theo de Raadt wrote: > > > > The idea was to have /var/www/tmp created by default, but with > > > > www:www ownership. > > > > > Create the directory. Now as a user, completely fill it. > > > > The proposal is to create tmp with www:www ownership, writable only for > > that user, not like the old /var/tmp which was writable by anyone. > > That's not true; the diff created it mode 1777. > > A smaller secondary concern is if you can convince software using this > space, > from remote, to hog the space too much, and/or lose track of files in > there. > Which would also create the fallout problems of "/var is full". > > It's a matter of how other /var-using software misbehaves or fails in > those circumstances. These concerns have been ignored too long. > >
Re: Include /var/www/tmp into base install
On Tue, Apr 07, 2020 at 05:05:08PM +0100, Stuart Henderson wrote: > On 2020/04/07 18:01, Stefan Sperling wrote: > > Yes, absolutely correct. Logs or tempfiles filling up /var are a problem, > > and in the gotweb application Tracey and I created it is indeed possible > > for requests to trigger large tempfiles. We need to look at that and come > > up with a better solution. > > We could check whether httpd/slowcgi could help with this somehow and try > > to come up with something that works for any application and not just ours. > > > > fwiw my usual approach is to put /var/www on a separate filesystem .. I had that in mind, too. But then you can still fill up that partition, and in gotweb's case it has Git repositories which also freak out in various ways if you run them out of disk. So it doesn't solve the problem from the application's point of view. Like regular /var, these repositories always need some breathing room. For temp stuff we really need a separate space that can just be wiped without consequences when it has run full.
Re: Include /var/www/tmp into base install
Stuart Henderson wrote: > On 2020/04/07 18:01, Stefan Sperling wrote: > > On Tue, Apr 07, 2020 at 09:51:15AM -0600, Theo de Raadt wrote: > > > Stefan Sperling wrote: > > > > > > > On Tue, Apr 07, 2020 at 09:37:02AM -0600, Theo de Raadt wrote: > > > > > > The idea was to have /var/www/tmp created by default, but with > > > > > > www:www ownership. > > > > > > > > > Create the directory. Now as a user, completely fill it. > > > > > > > > The proposal is to create tmp with www:www ownership, writable only for > > > > that user, not like the old /var/tmp which was writable by anyone. > > > > > > That's not true; the diff created it mode 1777. > > > > Ah, I missed that. Yes that would be a problem in the diff. > > > > > A smaller secondary concern is if you can convince software using this > > > space, > > > from remote, to hog the space too much, and/or lose track of files in > > > there. > > > Which would also create the fallout problems of "/var is full". > > > > > > It's a matter of how other /var-using software misbehaves or fails in > > > those circumstances. These concerns have been ignored too long. > > > > Yes, absolutely correct. Logs or tempfiles filling up /var are a problem, > > and in the gotweb application Tracey and I created it is indeed possible > > for requests to trigger large tempfiles. We need to look at that and come > > up with a better solution. > > We could check whether httpd/slowcgi could help with this somehow and try > > to come up with something that works for any application and not just ours. > > > > fwiw my usual approach is to put /var/www on a separate filesystem .. That's my approach also. But now /var/www can get filled. The problem is how will /tmp files be handled? Sloppily, I predict.
Re: Include /var/www/tmp into base install
Stefan Sperling wrote: > > A smaller secondary concern is if you can convince software using this > > space, > > from remote, to hog the space too much, and/or lose track of files in there. > > Which would also create the fallout problems of "/var is full". > > > > It's a matter of how other /var-using software misbehaves or fails in > > those circumstances. These concerns have been ignored too long. > > Yes, absolutely correct. Logs or tempfiles filling up /var are a problem, > and in the gotweb application Tracey and I created it is indeed possible > for requests to trigger large tempfiles. We need to look at that and come > up with a better solution. > We could check whether httpd/slowcgi could help with this somehow and try > to come up with something that works for any application and not just ours. I'm worried anyone using a tmp directory is going to write software which "loses files" when it crashes. That's yet another reason why a filesystem can get full. And when /var gets full, there are big consequences. We've done pretty much NOTHING to the other /var consuming-software to make sure they work (or fail kindly) when /var is full. It is an ignored problem. It only takes a little experiment to know the problem is real.
Re: pppx(4): prevent concurency with pppx_if_destroy()
On Tue, Apr 07, 2020 at 06:38:11PM +0300, Vitaliy Makkoveev wrote: > On Tue, Apr 07, 2020 at 04:43:55PM +0200, Martin Pieuchot wrote: > > On 07/04/20(Tue) 17:14, Vitaliy Makkoveev wrote: > > > As Claudio Jeker noticed, NET_LOCK() can release KERNEL_LOCK(). pppx(4) > > > code has some NET_LOCK() dances which make it unsafe. [...] > > > > The easiest way to fix that is to move if_detach() out of pppx_if_destroy(). > > Yes, but we are going to move pppx(4) code out from locking. My diff > prevents concurent access to pxi_if by pppx_del_session(). Yes > pppx_del_session() can grab pxi_if owned by other threads (in the > future, after KERNEL_LOCK() will gone), but after refcounters this will > be fine and we can push NET_LOCK() deeper. > In other words my diff kills the unlocked gap between pppx_if_find() and pppx_if_destroy(). Concurent pppx_del_session() can grab pppx_if (not for now) in this gap. Also I wish to avoid dances with twice bumped refcounter here in future. > > It generally makes sense to call if_detach() first then free/close the > > descriptor of a driver. However some drivers have callbacks and in that > > case you might want to teardown those first then call if_detach(). > > > > All the code from pppx_if_destroy() and pppx_add_session() has the wrong > ordered netif usage. > > > if_detach() will require the NET_LOCK() for some time. However > > pseudo-driver should start protecting their own data structure with > > different locks. > >
Re: Include /var/www/tmp into base install
On 2020/04/07 18:01, Stefan Sperling wrote: > On Tue, Apr 07, 2020 at 09:51:15AM -0600, Theo de Raadt wrote: > > Stefan Sperling wrote: > > > > > On Tue, Apr 07, 2020 at 09:37:02AM -0600, Theo de Raadt wrote: > > > > > The idea was to have /var/www/tmp created by default, but with > > > > > www:www ownership. > > > > > > > Create the directory. Now as a user, completely fill it. > > > > > > The proposal is to create tmp with www:www ownership, writable only for > > > that user, not like the old /var/tmp which was writable by anyone. > > > > That's not true; the diff created it mode 1777. > > Ah, I missed that. Yes that would be a problem in the diff. > > > A smaller secondary concern is if you can convince software using this > > space, > > from remote, to hog the space too much, and/or lose track of files in there. > > Which would also create the fallout problems of "/var is full". > > > > It's a matter of how other /var-using software misbehaves or fails in > > those circumstances. These concerns have been ignored too long. > > Yes, absolutely correct. Logs or tempfiles filling up /var are a problem, > and in the gotweb application Tracey and I created it is indeed possible > for requests to trigger large tempfiles. We need to look at that and come > up with a better solution. > We could check whether httpd/slowcgi could help with this somehow and try > to come up with something that works for any application and not just ours. > fwiw my usual approach is to put /var/www on a separate filesystem ..
Re: Include /var/www/tmp into base install
On Tue, Apr 07, 2020 at 09:51:15AM -0600, Theo de Raadt wrote: > Stefan Sperling wrote: > > > On Tue, Apr 07, 2020 at 09:37:02AM -0600, Theo de Raadt wrote: > > > > The idea was to have /var/www/tmp created by default, but with > > > > www:www ownership. > > > > > Create the directory. Now as a user, completely fill it. > > > > The proposal is to create tmp with www:www ownership, writable only for > > that user, not like the old /var/tmp which was writable by anyone. > > That's not true; the diff created it mode 1777. Ah, I missed that. Yes that would be a problem in the diff. > A smaller secondary concern is if you can convince software using this space, > from remote, to hog the space too much, and/or lose track of files in there. > Which would also create the fallout problems of "/var is full". > > It's a matter of how other /var-using software misbehaves or fails in > those circumstances. These concerns have been ignored too long. Yes, absolutely correct. Logs or tempfiles filling up /var are a problem, and in the gotweb application Tracey and I created it is indeed possible for requests to trigger large tempfiles. We need to look at that and come up with a better solution. We could check whether httpd/slowcgi could help with this somehow and try to come up with something that works for any application and not just ours.
Re: pppx(4): prevent concurency with pppx_if_destroy()
On Tue, Apr 07, 2020 at 04:43:55PM +0200, Martin Pieuchot wrote: > On 07/04/20(Tue) 17:14, Vitaliy Makkoveev wrote: > > As Claudio Jeker noticed, NET_LOCK() can release KERNEL_LOCK(). pppx(4) > > code has some NET_LOCK() dances which make it unsafe. [...] > > The easiest way to fix that is to move if_detach() out of pppx_if_destroy(). Yes, but we are going to move pppx(4) code out from locking. My diff prevents concurent access to pxi_if by pppx_del_session(). Yes pppx_del_session() can grab pxi_if owned by other threads (in the future, after KERNEL_LOCK() will gone), but after refcounters this will be fine and we can push NET_LOCK() deeper. > It generally makes sense to call if_detach() first then free/close the > descriptor of a driver. However some drivers have callbacks and in that > case you might want to teardown those first then call if_detach(). > All the code from pppx_if_destroy() and pppx_add_session() has the wrong ordered netif usage. > if_detach() will require the NET_LOCK() for some time. However > pseudo-driver should start protecting their own data structure with > different locks. >
Re: fix wifi media: line during background scan
On Tue, Apr 07, 2020 at 01:42:48PM +0200, Stefan Sperling wrote: > I've noticed that wireless interfaces in 11n mode show a "media:" line > in ifconfig such as this while a background scan is in progress: > > media: IEEE802.11 autoselect (OFDM6) > > What is expected is a line showing active 11n mode, such as: > > media: IEEE802.11 autoselect (HT-MCS0 mode 11n) > > This happens because ieee80211_media_status() sees ic->ic_curmode as > AUTO during background scans. Also, because net80211 forgets to reset > ic_curmode back to MODE_11N when the background scan has finished, the > displayed mode remains "autoselect (OFDM6)" until the interface is reset. > > This is just a cosmetic issue. > Internally, the interface operates in 11n mode regardless. > > ok? OK > > diff 5e4be56314753be1a3ad288aa6b16bcb5257b37c /usr/src > blob - 410c33358e72c4063f9f71bf69f6c72ecfc558d9 > file + sys/net80211/ieee80211.c > --- sys/net80211/ieee80211.c > +++ sys/net80211/ieee80211.c > @@ -728,6 +728,9 @@ ieee80211_media_status(struct ifnet *ifp, struct ifmed > ic->ic_curmode == IEEE80211_MODE_11AC) > imr->ifm_active |= ieee80211_mcs2media(ic, > ni->ni_txmcs, ic->ic_curmode); > + else if (ni->ni_flags & IEEE80211_NODE_HT) /* in MODE_AUTO */ > + imr->ifm_active |= ieee80211_mcs2media(ic, > + ni->ni_txmcs, IEEE80211_MODE_11N); > else > /* calculate rate subtype */ > imr->ifm_active |= ieee80211_rate2media(ic, > blob - 6656d29d160c26dce86fb44e3f5e715e42b7c42c > file + sys/net80211/ieee80211_node.c > --- sys/net80211/ieee80211_node.c > +++ sys/net80211/ieee80211_node.c > @@ -1441,6 +1441,19 @@ ieee80211_end_scan(struct ifnet *ifp) > ic->ic_bgscan_fail *= 2; > } > ic->ic_flags &= ~IEEE80211_F_BGSCAN; > + > + /* > + * HT is negotiated during association so we must use > + * ic_bss to check HT. The nodes tree was re-populated > + * during background scan and therefore selbs and curbs > + * may not carry HT information. > + */ > + ni = ic->ic_bss; > + if (ni->ni_flags & IEEE80211_NODE_HT) > + ieee80211_setmode(ic, IEEE80211_MODE_11N); > + else > + ieee80211_setmode(ic, > + ieee80211_chan2mode(ic, ni->ni_chan)); > return; > } >
Re: Include /var/www/tmp into base install
Stefan Sperling wrote: > On Tue, Apr 07, 2020 at 09:37:02AM -0600, Theo de Raadt wrote: > > > The idea was to have /var/www/tmp created by default, but with > > > www:www ownership. > > > Create the directory. Now as a user, completely fill it. > > The proposal is to create tmp with www:www ownership, writable only for > that user, not like the old /var/tmp which was writable by anyone. That's not true; the diff created it mode 1777. A smaller secondary concern is if you can convince software using this space, from remote, to hog the space too much, and/or lose track of files in there. Which would also create the fallout problems of "/var is full". It's a matter of how other /var-using software misbehaves or fails in those circumstances. These concerns have been ignored too long.
Re: Include /var/www/tmp into base install
On Tue, Apr 07, 2020 at 09:37:02AM -0600, Theo de Raadt wrote: > > The idea was to have /var/www/tmp created by default, but with > > www:www ownership. > Create the directory. Now as a user, completely fill it. The proposal is to create tmp with www:www ownership, writable only for that user, not like the old /var/tmp which was writable by anyone. Currently ports create per-application temp directories for this purpose when they get installed. I think this is fine and helps to keep unrelated things apart, so I don't see a reason to create a global 'www:www' tmp unless it helps ports sigificantly.
Re: Include /var/www/tmp into base install
Stuart Henderson wrote: > On 2020/04/07 09:01, Theo de Raadt wrote: > > This is horrible, as a user can fill the /var filesystem. > > they already can with /var/www/logs. On my machines not running this software, they cannot cause any effect on that directory. The software utilization of 1% of machines should not create risk on the other 99% of machines. Regardless, we should be moving towards making /var over-utilization safer, rather than less safe.
Re: Include /var/www/tmp into base install
Tracey Emery wrote: > On Tue, Apr 07, 2020 at 11:17:23AM -0400, Bryan Steele wrote: > > On Tue, Apr 07, 2020 at 04:56:31PM +0200, Martijn van Duren wrote: > > > This came up during u2k20 while discussing tempfiles for gotweb inside a > > > chroot. At the moment we don't include it by default and ports have to > > > create it themselves. Since I assume we want web applications to run > > > inside a /var/www chroot as much as possible and even some libc > > > functions depend on /tmp being available I'd argue we should include it > > > by default. > > > > WIth FastCGI, perhaps I'm confused, but why do web applications need to > > be inside the /var/www chroot? Can't they be anywhere, or even have a > > seperate chroot directory? Should we be handling things things that > > are not in base? > > Both slowcgi(8) and httpd(8) chroot to /var/www and are set to the www > user. The idea was to have /var/www/tmp created by default, but with > www:www ownership. This would eliminate multiple ports from creating the > directory and allow daily to clean the dir. > > To Theo's point, how was /var/tmp used in the past that it caused > problems? I'm struggling to find any info in past mailing lists. Please, please, please why don't you try it. Create the directory. Now as a user, completely fill it. Now your /var is full. Just do it, on an actual machine. Actually do it, don't just talk about it. Now be observant and consider the consequences. This isn't rocket science.
Re: Include /var/www/tmp into base install
On 2020/04/07 09:01, Theo de Raadt wrote: > This is horrible, as a user can fill the /var filesystem. they already can with /var/www/logs. On 2020/04/07 11:17, Bryan Steele wrote: > WIth FastCGI, perhaps I'm confused, but why do web applications need to > be inside the /var/www chroot? Can't they be anywhere, or even have a > seperate chroot directory? They can be, but slowcgi defaults are to chroot to /var/www and some cgi programs want access to files withing the web server's chroot directory. > Should we be handling things things that > are not in base? *shrug* > > Index: etc//mtree/4.4BSD.dist > > === > > RCS file: /cvs/src/etc/mtree/4.4BSD.dist,v > > retrieving revision 1.314 > > diff -u -p -r1.314 4.4BSD.dist > > --- etc//mtree/4.4BSD.dist 29 Nov 2019 03:28:20 - 1.314 > > +++ etc//mtree/4.4BSD.dist 7 Apr 2020 14:37:15 - > > @@ -749,6 +749,7 @@ var > > .. > > runtype=dir uname=root gname=daemon > > mode=755 > > .. > > +tmptype=dir uname=root gname=wheel > > mode=01777 > > .. > > > > # ./var/audit > > > > this wasn't tested :)
Re: Include /var/www/tmp into base install
On Tue, Apr 07, 2020 at 11:17:23AM -0400, Bryan Steele wrote:
> On Tue, Apr 07, 2020 at 04:56:31PM +0200, Martijn van Duren wrote:
> > This came up during u2k20 while discussing tempfiles for gotweb inside a
> > chroot. At the moment we don't include it by default and ports have to
> > create it themselves. Since I assume we want web applications to run
> > inside a /var/www chroot as much as possible and even some libc
> > functions depend on /tmp being available I'd argue we should include it
> > by default.
>
> WIth FastCGI, perhaps I'm confused, but why do web applications need to
> be inside the /var/www chroot? Can't they be anywhere, or even have a
> seperate chroot directory? Should we be handling things things that
> are not in base?
Both slowcgi(8) and httpd(8) chroot to /var/www and are set to the www
user. The idea was to have /var/www/tmp created by default, but with
www:www ownership. This would eliminate multiple ports from creating the
directory and allow daily to clean the dir.
To Theo's point, how was /var/tmp used in the past that it caused
problems? I'm struggling to find any info in past mailing lists.
>
> > I also choose to make the directory 1777, similar to a normal /tmp,
> > since both multiple slowcgi or php-fpm pools can run simultaneously
> > under different users.
> >
> > The cleanup functions don't reflect the current /tmp cleanup style, but
> > we can move the existing find statements to -delete in a separate patch.
> >
> > I already had some positive feedback during u2k20 on the concept.
> > OK?
> >
> > martijn@
> >
> > Index: etc//daily
> > ===
> > RCS file: /cvs/src/etc/daily,v
> > retrieving revision 1.93
> > diff -u -p -r1.93 daily
> > --- etc//daily 9 Sep 2019 20:02:26 - 1.93
> > +++ etc//daily 7 Apr 2020 14:37:15 -
> > @@ -55,6 +55,11 @@ if [ -d /tmp -a ! -L /tmp ]; then
> > ! -path ./.ICE-unix ! -name . \
> > -execdir rmdir -- {} \; >/dev/null 2>&1; }
> > fi
> > +if [ -d /var/www/tmp -a ! -L /var/www/tmp ]; then
> > + cd /var/www/tmp && {
> > + find -x . -type f -atime +7 -delete 2>/dev/null
> > + find -x . -type d -empty -delete 2>/dev/null
> > +fi
> >
> > # Additional junk directory cleanup would go like this:
> > #if [ -d /scratch -a ! -L /scratch ]; then
> > Index: etc//rc
> > ===
> > RCS file: /cvs/src/etc/rc,v
> > retrieving revision 1.543
> > diff -u -p -r1.543 rc
> > --- etc//rc 24 Jan 2020 06:17:37 - 1.543
> > +++ etc//rc 7 Apr 2020 14:37:15 -
> > @@ -532,7 +532,7 @@ if [[ -f /etc/ptmp ]]; then
> > 'password file may be incorrect -- /etc/ptmp exists'
> > fi
> >
> > -echo clearing /tmp
> > +echo clearing temporary directories
> >
> > # Prune quickly with one rm, then use find to clean up /tmp/[lqv]*
> > # (not needed with mfs /tmp, but doesn't hurt there...).
> > @@ -540,6 +540,7 @@ echo clearing /tmp
> > (cd /tmp &&
> > find . -maxdepth 1 ! -name . ! -name lost+found ! -name quota.user \
> > ! -name quota.group ! -name vi.recover -execdir rm -rf -- {} \;)
> > +(cd /var/www/tmp && find . -x -delete)
> >
> > # Create Unix sockets directories for X if needed and make sure they have
> > # correct permissions.
> > Index: etc//mtree/4.4BSD.dist
> > ===
> > RCS file: /cvs/src/etc/mtree/4.4BSD.dist,v
> > retrieving revision 1.314
> > diff -u -p -r1.314 4.4BSD.dist
> > --- etc//mtree/4.4BSD.dist 29 Nov 2019 03:28:20 - 1.314
> > +++ etc//mtree/4.4BSD.dist 7 Apr 2020 14:37:15 -
> > @@ -749,6 +749,7 @@ var
> > ..
> > runtype=dir uname=root gname=daemon
> > mode=755
> > ..
> > +tmptype=dir uname=root gname=wheel
> > mode=01777
> > ..
> >
> > # ./var/audit
> >
> >
--
Tracey Emery
Re: Include /var/www/tmp into base install
On Tue, Apr 07, 2020 at 04:56:31PM +0200, Martijn van Duren wrote:
> This came up during u2k20 while discussing tempfiles for gotweb inside a
> chroot. At the moment we don't include it by default and ports have to
> create it themselves. Since I assume we want web applications to run
> inside a /var/www chroot as much as possible and even some libc
> functions depend on /tmp being available I'd argue we should include it
> by default.
WIth FastCGI, perhaps I'm confused, but why do web applications need to
be inside the /var/www chroot? Can't they be anywhere, or even have a
seperate chroot directory? Should we be handling things things that
are not in base?
> I also choose to make the directory 1777, similar to a normal /tmp,
> since both multiple slowcgi or php-fpm pools can run simultaneously
> under different users.
>
> The cleanup functions don't reflect the current /tmp cleanup style, but
> we can move the existing find statements to -delete in a separate patch.
>
> I already had some positive feedback during u2k20 on the concept.
> OK?
>
> martijn@
>
> Index: etc//daily
> ===
> RCS file: /cvs/src/etc/daily,v
> retrieving revision 1.93
> diff -u -p -r1.93 daily
> --- etc//daily9 Sep 2019 20:02:26 - 1.93
> +++ etc//daily7 Apr 2020 14:37:15 -
> @@ -55,6 +55,11 @@ if [ -d /tmp -a ! -L /tmp ]; then
> ! -path ./.ICE-unix ! -name . \
> -execdir rmdir -- {} \; >/dev/null 2>&1; }
> fi
> +if [ -d /var/www/tmp -a ! -L /var/www/tmp ]; then
> + cd /var/www/tmp && {
> + find -x . -type f -atime +7 -delete 2>/dev/null
> + find -x . -type d -empty -delete 2>/dev/null
> +fi
>
> # Additional junk directory cleanup would go like this:
> #if [ -d /scratch -a ! -L /scratch ]; then
> Index: etc//rc
> ===
> RCS file: /cvs/src/etc/rc,v
> retrieving revision 1.543
> diff -u -p -r1.543 rc
> --- etc//rc 24 Jan 2020 06:17:37 - 1.543
> +++ etc//rc 7 Apr 2020 14:37:15 -
> @@ -532,7 +532,7 @@ if [[ -f /etc/ptmp ]]; then
> 'password file may be incorrect -- /etc/ptmp exists'
> fi
>
> -echo clearing /tmp
> +echo clearing temporary directories
>
> # Prune quickly with one rm, then use find to clean up /tmp/[lqv]*
> # (not needed with mfs /tmp, but doesn't hurt there...).
> @@ -540,6 +540,7 @@ echo clearing /tmp
> (cd /tmp &&
> find . -maxdepth 1 ! -name . ! -name lost+found ! -name quota.user \
> ! -name quota.group ! -name vi.recover -execdir rm -rf -- {} \;)
> +(cd /var/www/tmp && find . -x -delete)
>
> # Create Unix sockets directories for X if needed and make sure they have
> # correct permissions.
> Index: etc//mtree/4.4BSD.dist
> ===
> RCS file: /cvs/src/etc/mtree/4.4BSD.dist,v
> retrieving revision 1.314
> diff -u -p -r1.314 4.4BSD.dist
> --- etc//mtree/4.4BSD.dist29 Nov 2019 03:28:20 - 1.314
> +++ etc//mtree/4.4BSD.dist7 Apr 2020 14:37:15 -
> @@ -749,6 +749,7 @@ var
> ..
> run type=dir uname=root gname=daemon mode=755
> ..
> +tmp type=dir uname=root gname=wheel mode=01777
> ..
>
> # ./var/audit
>
>
Re: Include /var/www/tmp into base install
This is horrible, as a user can fill the /var filesystem.
That is why we got rid of /var/tmp before, and tried to reduce the risk on
/tmp. Now you want to bring the problem back.
Martijn van Duren wrote:
> This came up during u2k20 while discussing tempfiles for gotweb inside a
> chroot. At the moment we don't include it by default and ports have to
> create it themselves. Since I assume we want web applications to run
> inside a /var/www chroot as much as possible and even some libc
> functions depend on /tmp being available I'd argue we should include it
> by default.
>
> I also choose to make the directory 1777, similar to a normal /tmp,
> since both multiple slowcgi or php-fpm pools can run simultaneously
> under different users.
>
> The cleanup functions don't reflect the current /tmp cleanup style, but
> we can move the existing find statements to -delete in a separate patch.
>
> I already had some positive feedback during u2k20 on the concept.
> OK?
>
> martijn@
>
> Index: etc//daily
> ===
> RCS file: /cvs/src/etc/daily,v
> retrieving revision 1.93
> diff -u -p -r1.93 daily
> --- etc//daily9 Sep 2019 20:02:26 - 1.93
> +++ etc//daily7 Apr 2020 14:37:15 -
> @@ -55,6 +55,11 @@ if [ -d /tmp -a ! -L /tmp ]; then
> ! -path ./.ICE-unix ! -name . \
> -execdir rmdir -- {} \; >/dev/null 2>&1; }
> fi
> +if [ -d /var/www/tmp -a ! -L /var/www/tmp ]; then
> + cd /var/www/tmp && {
> + find -x . -type f -atime +7 -delete 2>/dev/null
> + find -x . -type d -empty -delete 2>/dev/null
> +fi
>
> # Additional junk directory cleanup would go like this:
> #if [ -d /scratch -a ! -L /scratch ]; then
> Index: etc//rc
> ===
> RCS file: /cvs/src/etc/rc,v
> retrieving revision 1.543
> diff -u -p -r1.543 rc
> --- etc//rc 24 Jan 2020 06:17:37 - 1.543
> +++ etc//rc 7 Apr 2020 14:37:15 -
> @@ -532,7 +532,7 @@ if [[ -f /etc/ptmp ]]; then
> 'password file may be incorrect -- /etc/ptmp exists'
> fi
>
> -echo clearing /tmp
> +echo clearing temporary directories
>
> # Prune quickly with one rm, then use find to clean up /tmp/[lqv]*
> # (not needed with mfs /tmp, but doesn't hurt there...).
> @@ -540,6 +540,7 @@ echo clearing /tmp
> (cd /tmp &&
> find . -maxdepth 1 ! -name . ! -name lost+found ! -name quota.user \
> ! -name quota.group ! -name vi.recover -execdir rm -rf -- {} \;)
> +(cd /var/www/tmp && find . -x -delete)
>
> # Create Unix sockets directories for X if needed and make sure they have
> # correct permissions.
> Index: etc//mtree/4.4BSD.dist
> ===
> RCS file: /cvs/src/etc/mtree/4.4BSD.dist,v
> retrieving revision 1.314
> diff -u -p -r1.314 4.4BSD.dist
> --- etc//mtree/4.4BSD.dist29 Nov 2019 03:28:20 - 1.314
> +++ etc//mtree/4.4BSD.dist7 Apr 2020 14:37:15 -
> @@ -749,6 +749,7 @@ var
> ..
> run type=dir uname=root gname=daemon mode=755
> ..
> +tmp type=dir uname=root gname=wheel mode=01777
> ..
>
> # ./var/audit
>
Re: pppx(4): prevent concurency with pppx_if_destroy()
Forgot to release lock in pppx_del_session() error case...
Index: sys/net/if_pppx.c
===
RCS file: /cvs/src/sys/net/if_pppx.c,v
retrieving revision 1.81
diff -u -p -r1.81 if_pppx.c
--- sys/net/if_pppx.c 7 Apr 2020 07:11:22 - 1.81
+++ sys/net/if_pppx.c 7 Apr 2020 14:39:28 -
@@ -170,7 +170,8 @@ RBT_HEAD(pppx_ifs, pppx_if) pppx_ifs = R
RBT_PROTOTYPE(pppx_ifs, pppx_if, pxi_entry, pppx_if_cmp);
intpppx_if_next_unit(void);
-struct pppx_if *pppx_if_find(struct pppx_dev *, int, int);
+struct pppx_if *pppx_if_find_locked(struct pppx_dev *, int, int);
+static inline struct pppx_if *pppx_if_find(struct pppx_dev *, int, int);
intpppx_add_session(struct pppx_dev *,
struct pipex_session_req *);
intpppx_del_session(struct pppx_dev *,
@@ -594,8 +595,19 @@ pppxclose(dev_t dev, int flags, int mode
/* XXX */
NET_LOCK();
- while ((pxi = LIST_FIRST(&pxd->pxd_pxis)))
+ rw_enter_write(&pppx_ifs_lk);
+ while ((pxi = LIST_FIRST(&pxd->pxd_pxis))) {
+ if (RBT_REMOVE(pppx_ifs, &pppx_ifs, pxi) == NULL)
+ panic("%s: pppx_ifs modified while lock was held",
+ __func__);
+ LIST_REMOVE(pxi, pxi_list);
+ rw_exit_write(&pppx_ifs_lk);
+
pppx_if_destroy(pxd, pxi);
+
+ rw_enter_write(&pppx_ifs_lk);
+ }
+ rw_exit_write(&pppx_ifs_lk);
NET_UNLOCK();
LIST_REMOVE(pxd, pxd_entry);
@@ -641,24 +653,37 @@ pppx_if_next_unit(void)
}
struct pppx_if *
-pppx_if_find(struct pppx_dev *pxd, int session_id, int protocol)
+pppx_if_find_locked(struct pppx_dev *pxd, int session_id, int protocol)
{
struct pppx_if *s, *p;
+
+ rw_assert_anylock(&pppx_ifs_lk);
+
s = malloc(sizeof(*s), M_DEVBUF, M_WAITOK | M_ZERO);
s->pxi_key.pxik_session_id = session_id;
s->pxi_key.pxik_protocol = protocol;
- rw_enter_read(&pppx_ifs_lk);
p = RBT_FIND(pppx_ifs, &pppx_ifs, s);
if (p && p->pxi_ready == 0)
p = NULL;
- rw_exit_read(&pppx_ifs_lk);
free(s, M_DEVBUF, sizeof(*s));
return (p);
}
+static inline struct pppx_if *
+pppx_if_find(struct pppx_dev *pxd, int session_id, int protocol)
+{
+ struct pppx_if *pxi;
+
+ rw_enter_read(&pppx_ifs_lk);
+ pxi = pppx_if_find_locked(pxd, session_id, protocol);
+ rw_exit_read(&pppx_ifs_lk);
+
+ return pxi;
+}
+
int
pppx_add_session(struct pppx_dev *pxd, struct pipex_session_req *req)
{
@@ -948,14 +973,24 @@ pppx_del_session(struct pppx_dev *pxd, s
{
struct pppx_if *pxi;
- pxi = pppx_if_find(pxd, req->pcr_session_id, req->pcr_protocol);
- if (pxi == NULL)
+ rw_enter_write(&pppx_ifs_lk);
+ pxi = pppx_if_find_locked(pxd, req->pcr_session_id,
+ req->pcr_protocol);
+ if (pxi == NULL) {
+ rw_exit_write(&pppx_ifs_lk);
return (EINVAL);
+ }
+
+ if (RBT_REMOVE(pppx_ifs, &pppx_ifs, pxi) == NULL)
+ panic("%s: pppx_ifs modified while lock was held", __func__);
+ LIST_REMOVE(pxi, pxi_list);
+ rw_exit_write(&pppx_ifs_lk);
req->pcr_stat = pxi->pxi_session.stat;
pppx_if_destroy(pxd, pxi);
- return (0);
+
+ return 0;
}
int
@@ -1037,12 +1072,6 @@ pppx_if_destroy(struct pppx_dev *pxd, st
NET_UNLOCK();
if_detach(ifp);
NET_LOCK();
-
- rw_enter_write(&pppx_ifs_lk);
- if (RBT_REMOVE(pppx_ifs, &pppx_ifs, pxi) == NULL)
- panic("%s: pppx_ifs modified while lock was held", __func__);
- LIST_REMOVE(pxi, pxi_list);
- rw_exit_write(&pppx_ifs_lk);
pool_put(pppx_if_pl, pxi);
}
Include /var/www/tmp into base install
This came up during u2k20 while discussing tempfiles for gotweb inside a
chroot. At the moment we don't include it by default and ports have to
create it themselves. Since I assume we want web applications to run
inside a /var/www chroot as much as possible and even some libc
functions depend on /tmp being available I'd argue we should include it
by default.
I also choose to make the directory 1777, similar to a normal /tmp,
since both multiple slowcgi or php-fpm pools can run simultaneously
under different users.
The cleanup functions don't reflect the current /tmp cleanup style, but
we can move the existing find statements to -delete in a separate patch.
I already had some positive feedback during u2k20 on the concept.
OK?
martijn@
Index: etc//daily
===
RCS file: /cvs/src/etc/daily,v
retrieving revision 1.93
diff -u -p -r1.93 daily
--- etc//daily 9 Sep 2019 20:02:26 - 1.93
+++ etc//daily 7 Apr 2020 14:37:15 -
@@ -55,6 +55,11 @@ if [ -d /tmp -a ! -L /tmp ]; then
! -path ./.ICE-unix ! -name . \
-execdir rmdir -- {} \; >/dev/null 2>&1; }
fi
+if [ -d /var/www/tmp -a ! -L /var/www/tmp ]; then
+ cd /var/www/tmp && {
+ find -x . -type f -atime +7 -delete 2>/dev/null
+ find -x . -type d -empty -delete 2>/dev/null
+fi
# Additional junk directory cleanup would go like this:
#if [ -d /scratch -a ! -L /scratch ]; then
Index: etc//rc
===
RCS file: /cvs/src/etc/rc,v
retrieving revision 1.543
diff -u -p -r1.543 rc
--- etc//rc 24 Jan 2020 06:17:37 - 1.543
+++ etc//rc 7 Apr 2020 14:37:15 -
@@ -532,7 +532,7 @@ if [[ -f /etc/ptmp ]]; then
'password file may be incorrect -- /etc/ptmp exists'
fi
-echo clearing /tmp
+echo clearing temporary directories
# Prune quickly with one rm, then use find to clean up /tmp/[lqv]*
# (not needed with mfs /tmp, but doesn't hurt there...).
@@ -540,6 +540,7 @@ echo clearing /tmp
(cd /tmp &&
find . -maxdepth 1 ! -name . ! -name lost+found ! -name quota.user \
! -name quota.group ! -name vi.recover -execdir rm -rf -- {} \;)
+(cd /var/www/tmp && find . -x -delete)
# Create Unix sockets directories for X if needed and make sure they have
# correct permissions.
Index: etc//mtree/4.4BSD.dist
===
RCS file: /cvs/src/etc/mtree/4.4BSD.dist,v
retrieving revision 1.314
diff -u -p -r1.314 4.4BSD.dist
--- etc//mtree/4.4BSD.dist 29 Nov 2019 03:28:20 - 1.314
+++ etc//mtree/4.4BSD.dist 7 Apr 2020 14:37:15 -
@@ -749,6 +749,7 @@ var
..
runtype=dir uname=root gname=daemon mode=755
..
+tmptype=dir uname=root gname=wheel mode=01777
..
# ./var/audit
Re: pppx(4): prevent concurency with pppx_if_destroy()
On 07/04/20(Tue) 17:14, Vitaliy Makkoveev wrote: > As Claudio Jeker noticed, NET_LOCK() can release KERNEL_LOCK(). pppx(4) > code has some NET_LOCK() dances which make it unsafe. [...] The easiest way to fix that is to move if_detach() out of pppx_if_destroy(). It generally makes sense to call if_detach() first then free/close the descriptor of a driver. However some drivers have callbacks and in that case you might want to teardown those first then call if_detach(). if_detach() will require the NET_LOCK() for some time. However pseudo-driver should start protecting their own data structure with different locks.
pppx(4): prevent concurency with pppx_if_destroy()
As Claudio Jeker noticed, NET_LOCK() can release KERNEL_LOCK(). pppx(4)
code has some NET_LOCK() dances which make it unsafe. Concurent thread
can receive CPU and enter to pppx_if_destroy() while we dance with
NET_LOCK(). The idea is to deny access to pxi at destruction stage.
If pxi_if is removed from pppx_ifs tree under pppx_ifs_lk nobody will
access to it after pppx_ifs_lk release. Already used pppx_if can't be
accessed by pppx_del_session() because all this code is under
NET_LOCK().
I don't want to do all pxi_if destruction under pppx_ifs_lk. So new
function pppx_if_find_locked() introduced. It does the same what
pppx_if_find() does but pppx_ifs_lk should be held.
So grab the lock, find and remove pxi_if from tree and list, realease
lock, kill pppx_if.
I wish to add refconters to pppx_if in future to protect it the tun(4)
style. This diff will be the first step in this direction.
Index: sys/net/if_pppx.c
===
RCS file: /cvs/src/sys/net/if_pppx.c,v
retrieving revision 1.81
diff -u -p -r1.81 if_pppx.c
--- sys/net/if_pppx.c 7 Apr 2020 07:11:22 - 1.81
+++ sys/net/if_pppx.c 7 Apr 2020 13:41:07 -
@@ -170,7 +170,8 @@ RBT_HEAD(pppx_ifs, pppx_if) pppx_ifs = R
RBT_PROTOTYPE(pppx_ifs, pppx_if, pxi_entry, pppx_if_cmp);
intpppx_if_next_unit(void);
-struct pppx_if *pppx_if_find(struct pppx_dev *, int, int);
+struct pppx_if *pppx_if_find_locked(struct pppx_dev *, int, int);
+static inline struct pppx_if *pppx_if_find(struct pppx_dev *, int, int);
intpppx_add_session(struct pppx_dev *,
struct pipex_session_req *);
intpppx_del_session(struct pppx_dev *,
@@ -594,8 +595,19 @@ pppxclose(dev_t dev, int flags, int mode
/* XXX */
NET_LOCK();
- while ((pxi = LIST_FIRST(&pxd->pxd_pxis)))
+ rw_enter_write(&pppx_ifs_lk);
+ while ((pxi = LIST_FIRST(&pxd->pxd_pxis))) {
+ if (RBT_REMOVE(pppx_ifs, &pppx_ifs, pxi) == NULL)
+ panic("%s: pppx_ifs modified while lock was held",
+ __func__);
+ LIST_REMOVE(pxi, pxi_list);
+ rw_exit_write(&pppx_ifs_lk);
+
pppx_if_destroy(pxd, pxi);
+
+ rw_enter_write(&pppx_ifs_lk);
+ }
+ rw_exit_write(&pppx_ifs_lk);
NET_UNLOCK();
LIST_REMOVE(pxd, pxd_entry);
@@ -641,24 +653,37 @@ pppx_if_next_unit(void)
}
struct pppx_if *
-pppx_if_find(struct pppx_dev *pxd, int session_id, int protocol)
+pppx_if_find_locked(struct pppx_dev *pxd, int session_id, int protocol)
{
struct pppx_if *s, *p;
+
+ rw_assert_anylock(&pppx_ifs_lk);
+
s = malloc(sizeof(*s), M_DEVBUF, M_WAITOK | M_ZERO);
s->pxi_key.pxik_session_id = session_id;
s->pxi_key.pxik_protocol = protocol;
- rw_enter_read(&pppx_ifs_lk);
p = RBT_FIND(pppx_ifs, &pppx_ifs, s);
if (p && p->pxi_ready == 0)
p = NULL;
- rw_exit_read(&pppx_ifs_lk);
free(s, M_DEVBUF, sizeof(*s));
return (p);
}
+static inline struct pppx_if *
+pppx_if_find(struct pppx_dev *pxd, int session_id, int protocol)
+{
+ struct pppx_if *pxi;
+
+ rw_enter_read(&pppx_ifs_lk);
+ pxi = pppx_if_find_locked(pxd, session_id, protocol);
+ rw_exit_read(&pppx_ifs_lk);
+
+ return pxi;
+}
+
int
pppx_add_session(struct pppx_dev *pxd, struct pipex_session_req *req)
{
@@ -948,14 +973,22 @@ pppx_del_session(struct pppx_dev *pxd, s
{
struct pppx_if *pxi;
- pxi = pppx_if_find(pxd, req->pcr_session_id, req->pcr_protocol);
+ rw_enter_write(&pppx_ifs_lk);
+ pxi = pppx_if_find_locked(pxd, req->pcr_session_id,
+ req->pcr_protocol);
if (pxi == NULL)
return (EINVAL);
+ if (RBT_REMOVE(pppx_ifs, &pppx_ifs, pxi) == NULL)
+ panic("%s: pppx_ifs modified while lock was held", __func__);
+ LIST_REMOVE(pxi, pxi_list);
+ rw_exit_write(&pppx_ifs_lk);
+
req->pcr_stat = pxi->pxi_session.stat;
pppx_if_destroy(pxd, pxi);
- return (0);
+
+ return 0;
}
int
@@ -1037,12 +1070,6 @@ pppx_if_destroy(struct pppx_dev *pxd, st
NET_UNLOCK();
if_detach(ifp);
NET_LOCK();
-
- rw_enter_write(&pppx_ifs_lk);
- if (RBT_REMOVE(pppx_ifs, &pppx_ifs, pxi) == NULL)
- panic("%s: pppx_ifs modified while lock was held", __func__);
- LIST_REMOVE(pxi, pxi_list);
- rw_exit_write(&pppx_ifs_lk);
pool_put(pppx_if_pl, pxi);
}
[PATCH 0/5] gost: add support for magma and kuznyechik ciphers
Russian standards body has issues a standard GOST R 34.12-2015 defining two block ciphers: magma and kuznyechik. English descriptions of these ciphers are defined in draft-dolmatov-magma (in RFC editor queue) and RFC 7801 respectively. These patches add support for basic constructions using these ciphers.
[PATCH 1/5] modes: add functions implementing common code for 64-bit ciphers
64-bit ciphers are old, but it would be good to use common code for
their implementations.
Signed-off-by: Dmitry Baryshkov
---
src/lib/libcrypto/modes/cbc64.c | 202
src/lib/libcrypto/modes/cfb64.c | 169 ++
src/lib/libcrypto/modes/ctr64.c | 174 +++
src/lib/libcrypto/modes/modes.h | 26
src/lib/libcrypto/modes/ofb64.c | 119 +++
5 files changed, 690 insertions(+)
create mode 100644 src/lib/libcrypto/modes/cbc64.c
create mode 100644 src/lib/libcrypto/modes/cfb64.c
create mode 100644 src/lib/libcrypto/modes/ctr64.c
create mode 100644 src/lib/libcrypto/modes/ofb64.c
diff --git a/src/lib/libcrypto/modes/cbc64.c b/src/lib/libcrypto/modes/cbc64.c
new file mode 100644
index ..ec65ac5d3468
--- /dev/null
+++ b/src/lib/libcrypto/modes/cbc64.c
@@ -0,0 +1,202 @@
+/* $OpenBSD: cbc64.c,v 1.4 2015/02/10 09:46:30 miod Exp $ */
+/*
+ * Copyright (c) 2008 The OpenSSL Project. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright
+ *notice, this list of conditions and the following disclaimer.
+ *
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *notice, this list of conditions and the following disclaimer in
+ *the documentation and/or other materials provided with the
+ *distribution.
+ *
+ * 3. All advertising materials mentioning features or use of this
+ *software must display the following acknowledgment:
+ *"This product includes software developed by the OpenSSL Project
+ *for use in the OpenSSL Toolkit. (http://www.openssl.org/)"
+ *
+ * 4. The names "OpenSSL Toolkit" and "OpenSSL Project" must not be used to
+ *endorse or promote products derived from this software without
+ *prior written permission. For written permission, please contact
+ *openssl-c...@openssl.org.
+ *
+ * 5. Products derived from this software may not be called "OpenSSL"
+ *nor may "OpenSSL" appear in their names without prior written
+ *permission of the OpenSSL Project.
+ *
+ * 6. Redistributions of any form whatsoever must retain the following
+ *acknowledgment:
+ *"This product includes software developed by the OpenSSL Project
+ *for use in the OpenSSL Toolkit (http://www.openssl.org/)"
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE OpenSSL PROJECT ``AS IS'' AND ANY
+ * EXPRESSED OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE OpenSSL PROJECT OR
+ * ITS CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
+ * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
+ * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED
+ * OF THE POSSIBILITY OF SUCH DAMAGE.
+ *
+ *
+ */
+
+#include
+#include "modes_lcl.h"
+#include
+
+#ifndef MODES_DEBUG
+# ifndef NDEBUG
+# define NDEBUG
+# endif
+#endif
+
+#undef STRICT_ALIGNMENT
+#ifdef __STRICT_ALIGNMENT
+#define STRICT_ALIGNMENT 1
+#else
+#define STRICT_ALIGNMENT 0
+#endif
+
+void CRYPTO_cbc64_encrypt(const unsigned char *in, unsigned char *out,
+ size_t len, const void *key,
+ unsigned char ivec[8], block64_f block)
+{
+ size_t n;
+ const unsigned char *iv = ivec;
+
+#if !defined(OPENSSL_SMALL_FOOTPRINT)
+ if (STRICT_ALIGNMENT &&
+ ((size_t)in|(size_t)out|(size_t)ivec)%sizeof(size_t) != 0) {
+ while (len>=8) {
+ for(n=0; n<8; ++n)
+ out[n] = in[n] ^ iv[n];
+ (*block)(out, out, key);
+ iv = out;
+ len -= 8;
+ in += 8;
+ out += 8;
+ }
+ } else {
+ while (len>=8) {
+ for(n=0; n<8; n+=sizeof(size_t))
+ *(size_t*)(out+n) =
+ *(size_t*)(in+n) ^ *(size_t*)(iv+n);
+ (*block)(out, out, key);
+ iv = out;
+ len -= 8;
+ in += 8;
+ out += 8;
+ }
+ }
+#endif
+ while (len) {
+ for(n=0; n<8 && n=8) {
+ (*block)(in, out, key);
+
[PATCH 3/5] gost: use key_meshing for specifying section size
In preparation to adding ACPKM support, switch key_meshing to be a
section size rather than just a flag.
Signed-off-by: Dmitry Baryshkov
---
src/lib/libcrypto/gost/gost.h | 2 +-
src/lib/libcrypto/gost/gost2814789.c | 8
src/lib/libcrypto/gost/gost89_params.c | 2 +-
3 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/src/lib/libcrypto/gost/gost.h b/src/lib/libcrypto/gost/gost.h
index b6e5b51c40d4..a9ed5a1c50df 100644
--- a/src/lib/libcrypto/gost/gost.h
+++ b/src/lib/libcrypto/gost/gost.h
@@ -69,7 +69,7 @@ typedef struct gost2814789_key_st {
unsigned int key[8];
unsigned int k87[256],k65[256],k43[256],k21[256];
unsigned int count;
- unsigned key_meshing : 1;
+ unsigned int key_meshing;
} GOST2814789_KEY;
int Gost2814789_set_sbox(GOST2814789_KEY *key, int nid);
diff --git a/src/lib/libcrypto/gost/gost2814789.c
b/src/lib/libcrypto/gost/gost2814789.c
index 5016ed004b32..e5c7f6a2b3dd 100644
--- a/src/lib/libcrypto/gost/gost2814789.c
+++ b/src/lib/libcrypto/gost/gost2814789.c
@@ -169,7 +169,7 @@ void
Gost2814789_ecb_encrypt(const unsigned char *in, unsigned char *out,
GOST2814789_KEY *key, const int enc)
{
- if (key->key_meshing && key->count == 1024) {
+ if (key->key_meshing && key->count == key->key_meshing) {
Gost2814789_cryptopro_key_mesh(key);
key->count = 0;
}
@@ -183,7 +183,7 @@ Gost2814789_ecb_encrypt(const unsigned char *in, unsigned
char *out,
static inline void
Gost2814789_encrypt_mesh(unsigned char *iv, GOST2814789_KEY *key)
{
- if (key->key_meshing && key->count == 1024) {
+ if (key->key_meshing && key->count == key->key_meshing) {
Gost2814789_cryptopro_key_mesh(key);
Gost2814789_encrypt(iv, iv, key);
key->count = 0;
@@ -196,7 +196,7 @@ static inline void
Gost2814789_mac_mesh(const unsigned char *data, unsigned char *mac,
GOST2814789_KEY *key)
{
- if (key->key_meshing && key->count == 1024) {
+ if (key->key_meshing && key->count == key->key_meshing) {
Gost2814789_cryptopro_key_mesh(key);
key->count = 0;
}
@@ -328,7 +328,7 @@ Gost2814789_cnt_next(unsigned char *ivec, unsigned char
*out,
if (key->count == 0)
Gost2814789_encrypt(ivec, ivec, key);
- if (key->key_meshing && key->count == 1024) {
+ if (key->key_meshing && key->count == key->key_meshing) {
Gost2814789_cryptopro_key_mesh(key);
Gost2814789_encrypt(ivec, ivec, key);
key->count = 0;
diff --git a/src/lib/libcrypto/gost/gost89_params.c
b/src/lib/libcrypto/gost/gost89_params.c
index 526710cb043f..c454fd7afc40 100644
--- a/src/lib/libcrypto/gost/gost89_params.c
+++ b/src/lib/libcrypto/gost/gost89_params.c
@@ -191,7 +191,7 @@ Gost2814789_set_sbox(GOST2814789_KEY *key, int nid)
continue;
b = gost_cipher_list[i].sblock;
- key->key_meshing = gost_cipher_list[i].key_meshing;
+ key->key_meshing = gost_cipher_list[i].key_meshing ? 1024 : 0;
break;
}
--
2.25.1
[PATCH 2/5] gost: drop key_len from Gost28147_set_key
There is no point in specifying key length to Gost28147_set_key,
everybody just passes 256 (or 32 * 8) no matter what.
Signed-off-by: Dmitry Baryshkov
---
src/lib/libcrypto/evp/e_gost2814789.c | 4 +++-
src/lib/libcrypto/evp/m_gost2814789.c | 3 ++-
src/lib/libcrypto/gost/gost.h | 3 +--
src/lib/libcrypto/gost/gost2814789.c| 2 +-
src/lib/libcrypto/gost/gost89_keywrap.c | 6 +++---
src/lib/libcrypto/gost/gost89_params.c | 12 +++-
src/lib/libcrypto/gost/gostr341194.c| 8
7 files changed, 17 insertions(+), 21 deletions(-)
diff --git a/src/lib/libcrypto/evp/e_gost2814789.c
b/src/lib/libcrypto/evp/e_gost2814789.c
index 730de4fed1d9..e3c608f0eb4f 100644
--- a/src/lib/libcrypto/evp/e_gost2814789.c
+++ b/src/lib/libcrypto/evp/e_gost2814789.c
@@ -93,7 +93,9 @@ gost2814789_init_key(EVP_CIPHER_CTX *ctx, const unsigned char
*key,
{
EVP_GOST2814789_CTX *c = ctx->cipher_data;
- return Gost2814789_set_key(&c->ks, key, ctx->key_len * 8);
+ Gost2814789_set_key(&c->ks, key);
+
+ return 1;
}
int
diff --git a/src/lib/libcrypto/evp/m_gost2814789.c
b/src/lib/libcrypto/evp/m_gost2814789.c
index 279af872e022..779ccf07df14 100644
--- a/src/lib/libcrypto/evp/m_gost2814789.c
+++ b/src/lib/libcrypto/evp/m_gost2814789.c
@@ -82,7 +82,8 @@ gost2814789_md_ctrl(EVP_MD_CTX *ctx, int cmd, int p1, void
*p2)
switch (cmd) {
case EVP_MD_CTRL_SET_KEY:
- return Gost2814789_set_key(&gctx->cipher, p2, p1);
+ Gost2814789_set_key(&gctx->cipher, p2);
+ return 1;
case EVP_MD_CTRL_GOST_SET_SBOX:
return Gost2814789_set_sbox(&gctx->cipher, p1);
}
diff --git a/src/lib/libcrypto/gost/gost.h b/src/lib/libcrypto/gost/gost.h
index 092f96fb60ce..b6e5b51c40d4 100644
--- a/src/lib/libcrypto/gost/gost.h
+++ b/src/lib/libcrypto/gost/gost.h
@@ -73,8 +73,7 @@ typedef struct gost2814789_key_st {
} GOST2814789_KEY;
int Gost2814789_set_sbox(GOST2814789_KEY *key, int nid);
-int Gost2814789_set_key(GOST2814789_KEY *key,
- const unsigned char *userKey, const int bits);
+void Gost2814789_set_key(GOST2814789_KEY *key, const unsigned char *userKey);
void Gost2814789_ecb_encrypt(const unsigned char *in, unsigned char *out,
GOST2814789_KEY *key, const int enc);
void Gost2814789_cfb64_encrypt(const unsigned char *in, unsigned char *out,
diff --git a/src/lib/libcrypto/gost/gost2814789.c
b/src/lib/libcrypto/gost/gost2814789.c
index e285413ed460..5016ed004b32 100644
--- a/src/lib/libcrypto/gost/gost2814789.c
+++ b/src/lib/libcrypto/gost/gost2814789.c
@@ -461,7 +461,7 @@ GOST2814789IMIT(const unsigned char *d, size_t n, unsigned
char *md, int nid,
md = m;
GOST2814789IMIT_Init(&c, nid);
memcpy(c.mac, iv, 8);
- Gost2814789_set_key(&c.cipher, key, 256);
+ Gost2814789_set_key(&c.cipher, key);
GOST2814789IMIT_Update(&c, d, n);
GOST2814789IMIT_Final(md, &c);
explicit_bzero(&c, sizeof(c));
diff --git a/src/lib/libcrypto/gost/gost89_keywrap.c
b/src/lib/libcrypto/gost/gost89_keywrap.c
index a754c4d56ea1..47a11ad0c44e 100644
--- a/src/lib/libcrypto/gost/gost89_keywrap.c
+++ b/src/lib/libcrypto/gost/gost89_keywrap.c
@@ -85,7 +85,7 @@ key_diversify_crypto_pro(GOST2814789_KEY *ctx, const unsigned
char *inputKey,
p = S;
l2c (s1, p);
l2c (s2, p);
- Gost2814789_set_key(ctx, outputKey, 256);
+ Gost2814789_set_key(ctx, outputKey);
mask = 0;
Gost2814789_cfb64_encrypt(outputKey, outputKey, 32, ctx, S,
&mask, 1);
@@ -102,7 +102,7 @@ gost_key_wrap_crypto_pro(int nid, const unsigned char
*keyExchangeKey,
Gost2814789_set_sbox(&ctx, nid);
key_diversify_crypto_pro(&ctx, keyExchangeKey, ukm, kek_ukm);
- Gost2814789_set_key(&ctx, kek_ukm, 256);
+ Gost2814789_set_key(&ctx, kek_ukm);
memcpy(wrappedKey, ukm, 8);
Gost2814789_encrypt(sessionKey + 0, wrappedKey + 8 + 0, &ctx);
Gost2814789_encrypt(sessionKey + 8, wrappedKey + 8 + 8, &ctx);
@@ -122,7 +122,7 @@ gost_key_unwrap_crypto_pro(int nid, const unsigned char
*keyExchangeKey,
Gost2814789_set_sbox(&ctx, nid);
/* First 8 bytes of wrapped Key is ukm */
key_diversify_crypto_pro(&ctx, keyExchangeKey, wrappedKey, kek_ukm);
- Gost2814789_set_key(&ctx, kek_ukm, 256);
+ Gost2814789_set_key(&ctx, kek_ukm);
Gost2814789_decrypt(wrappedKey + 8 + 0, sessionKey + 0, &ctx);
Gost2814789_decrypt(wrappedKey + 8 + 8, sessionKey + 8, &ctx);
Gost2814789_decrypt(wrappedKey + 8 + 16, sessionKey + 16, &ctx);
diff --git a/src/lib/libcrypto/gost/gost89_params.c
b/src/lib/libcrypto/gost/gost89_params.c
index 35d8f62fe960..526710cb043f 100644
--- a/src/lib/libcrypto/gost/gost89_params.c
+++ b/src/lib/libcrypto/gost/gost89_params.c
@@ -212,21 +212,15 @@ Gost2814789_se
[PATCH 4/5] gost: add support for magma cipher
GOST R 34.12-2015 defines Magma cipher (a variant of GOST 28147-89 with
fixed S-BOX and endianness change), see draft-dolmatov-magma.
Sponsored by ROSA Linux
Signed-off-by: Dmitry Baryshkov
---
src/lib/libcrypto/Symbols.list | 5 +
src/lib/libcrypto/evp/c_all.c | 5 +
src/lib/libcrypto/evp/e_magma.c| 123 +
src/lib/libcrypto/evp/evp.h| 5 +
src/lib/libcrypto/gost/gost.h | 29 +
src/lib/libcrypto/gost/gost2814789.c | 71
src/lib/libcrypto/gost/gost89_params.c | 24
src/lib/libcrypto/gost/gost_locl.h | 12 ++
src/lib/libcrypto/objects/obj_mac.num | 9 ++
src/lib/libcrypto/objects/objects.txt | 14 +++
src/regress/lib/libcrypto/evp/evptests.txt | 18 +++
11 files changed, 315 insertions(+)
create mode 100644 src/lib/libcrypto/evp/e_magma.c
diff --git a/src/lib/libcrypto/Symbols.list b/src/lib/libcrypto/Symbols.list
index 662eb4dc291e..6102af2f8b24 100644
--- a/src/lib/libcrypto/Symbols.list
+++ b/src/lib/libcrypto/Symbols.list
@@ -1725,6 +1725,11 @@ EVP_idea_cfb
EVP_idea_cfb64
EVP_idea_ecb
EVP_idea_ofb
+EVP_magma_cbc
+EVP_magma_cfb64
+EVP_magma_ctr
+EVP_magma_ecb
+EVP_magma_ofb
EVP_md4
EVP_md5
EVP_md5_sha1
diff --git a/src/lib/libcrypto/evp/c_all.c b/src/lib/libcrypto/evp/c_all.c
index 9e9d39d5abaa..59342beb7a00 100644
--- a/src/lib/libcrypto/evp/c_all.c
+++ b/src/lib/libcrypto/evp/c_all.c
@@ -229,6 +229,11 @@ OpenSSL_add_all_ciphers_internal(void)
EVP_add_cipher(EVP_gost2814789_ecb());
EVP_add_cipher(EVP_gost2814789_cfb64());
EVP_add_cipher(EVP_gost2814789_cnt());
+ EVP_add_cipher(EVP_magma_ecb());
+ EVP_add_cipher(EVP_magma_cbc());
+ EVP_add_cipher(EVP_magma_cfb64());
+ EVP_add_cipher(EVP_magma_ofb());
+ EVP_add_cipher(EVP_magma_ctr());
#endif
#ifndef OPENSSL_NO_SM4
diff --git a/src/lib/libcrypto/evp/e_magma.c b/src/lib/libcrypto/evp/e_magma.c
new file mode 100644
index ..90130808a00a
--- /dev/null
+++ b/src/lib/libcrypto/evp/e_magma.c
@@ -0,0 +1,123 @@
+/* $OpenBSD: e_magma.c,v 1.4 2017/01/29 17:49:23 beck Exp $ */
+/*
+ * Copyright (c) 2020 Dmitry Baryshkov
+ *
+ * Sponsored by ROSA Linux
+ *
+ * Permission to use, copy, modify, and/or distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY
+ * SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION
+ * OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN
+ * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+#include
+
+#include
+
+#ifndef OPENSSL_NO_GOST
+#include
+#include
+#include
+#include
+#include "evp_locl.h"
+
+typedef struct {
+ MAGMA_KEY ks;
+} EVP_MAGMA_CTX;
+
+static int
+magma_init_key(EVP_CIPHER_CTX *ctx, const unsigned char *key,
+const unsigned char *iv, int enc)
+{
+ EVP_MAGMA_CTX *c = ctx->cipher_data;
+
+ Magma_set_key(&c->ks, key);
+
+ return 1;
+}
+
+static int
+magma_ctl(EVP_CIPHER_CTX *ctx, int type, int arg, void *ptr)
+{
+ switch (type) {
+ case EVP_CTRL_PBE_PRF_NID:
+ if (ptr != NULL) {
+ *((int *)ptr) = NID_id_tc26_hmac_gost_3411_12_256;
+ return 1;
+ } else {
+ return 0;
+ }
+ default:
+ return -1;
+ }
+}
+
+static void
+Magma_cbc_encrypt(const unsigned char *in, unsigned char *out, size_t len,
+ const MAGMA_KEY *key, unsigned char *ivec, const int enc)
+{
+ if (enc)
+ CRYPTO_cbc64_encrypt(in, out, len, key, ivec,
+ (block64_f)Magma_encrypt);
+ else
+ CRYPTO_cbc64_decrypt(in, out, len, key, ivec,
+ (block64_f)Magma_decrypt);
+}
+
+static void
+Magma_cfb64_encrypt(const unsigned char *in, unsigned char *out, size_t length,
+ const MAGMA_KEY *key, unsigned char *ivec, int *num, const int
enc)
+{
+ CRYPTO_cfb64_encrypt(in, out, length, key, ivec, num, enc,
+ (block64_f)Magma_encrypt);
+}
+
+static void
+Magma_ecb_encrypt(const unsigned char *in, unsigned char *out, const MAGMA_KEY
*key,
+ const int enc)
+{
+ if (enc)
+ Magma_encrypt(in, out, key);
+ else
+ Magma_decrypt(in, out, key);
+}
+
+static void
+Magma_ofb64_encrypt(const unsigned char *in, unsigned char *out, size_t length,
+ const MAGMA_KEY *key, unsigned char *ivec, int *num)
+
[PATCH 3/3] pkcs12: add support for GOST PFX files
Russian standard body has changed the way MAC key is calculated for
PKCS12 files. Generate proper keys depending on the digest type used for
MAC generation.
Sponsored by ROSA Linux
Signed-off-by: Dmitry Baryshkov
---
src/lib/libcrypto/pkcs12/p12_key.c | 18 ++
src/lib/libcrypto/pkcs12/p12_mutl.c | 28 +---
src/lib/libcrypto/pkcs12/pkcs12.h | 5 +
3 files changed, 44 insertions(+), 7 deletions(-)
diff --git a/src/lib/libcrypto/pkcs12/p12_key.c
b/src/lib/libcrypto/pkcs12/p12_key.c
index d419a9d83598..9a5297a23131 100644
--- a/src/lib/libcrypto/pkcs12/p12_key.c
+++ b/src/lib/libcrypto/pkcs12/p12_key.c
@@ -195,3 +195,21 @@ end:
EVP_MD_CTX_cleanup(&ctx);
return ret;
}
+
+int
+PKCS12_key_gen_gost(const char *pass, int passlen, unsigned char *salt,
+int saltlen, int iter, int n, unsigned char *out,
+const EVP_MD *md_type)
+{
+ unsigned char buf[96];
+
+ if (n != PKCS12_GOST_KEY_LEN)
+ return 0;
+
+ if (!PKCS5_PBKDF2_HMAC(pass, passlen, salt, saltlen, iter, md_type,
sizeof(buf), buf))
+ return 0;
+
+ memcpy(out, buf + sizeof(buf) - PKCS12_GOST_KEY_LEN,
PKCS12_GOST_KEY_LEN);
+
+ return 1;
+}
diff --git a/src/lib/libcrypto/pkcs12/p12_mutl.c
b/src/lib/libcrypto/pkcs12/p12_mutl.c
index f3132ec75f68..023bbbd92db1 100644
--- a/src/lib/libcrypto/pkcs12/p12_mutl.c
+++ b/src/lib/libcrypto/pkcs12/p12_mutl.c
@@ -74,6 +74,7 @@ PKCS12_gen_mac(PKCS12 *p12, const char *pass, int passlen,
unsigned char *mac, unsigned int *maclen)
{
const EVP_MD *md_type;
+ int md_type_nid;
HMAC_CTX hmac;
unsigned char key[EVP_MAX_MD_SIZE], *salt;
int saltlen, iter;
@@ -97,13 +98,26 @@ PKCS12_gen_mac(PKCS12 *p12, const char *pass, int passlen,
PKCS12error(PKCS12_R_UNKNOWN_DIGEST_ALGORITHM);
return 0;
}
- md_size = EVP_MD_size(md_type);
- if (md_size < 0)
- return 0;
- if (!PKCS12_key_gen(pass, passlen, salt, saltlen, PKCS12_MAC_ID, iter,
- md_size, key, md_type)) {
- PKCS12error(PKCS12_R_KEY_GEN_ERROR);
- return 0;
+ md_type_nid = EVP_MD_type(md_type);
+ if ((md_type_nid == NID_id_GostR3411_94 ||
+md_type_nid == NID_id_tc26_gost3411_2012_256 ||
+md_type_nid == NID_id_tc26_gost3411_2012_512) &&
+ getenv("LEGACY_GOST_PKCS12") == NULL) {
+ md_size = PKCS12_GOST_KEY_LEN;
+ if (!PKCS12_key_gen_gost(pass, passlen, salt, saltlen, iter,
+ md_size, key, md_type)) {
+ PKCS12error(PKCS12_R_KEY_GEN_ERROR);
+ return 0;
+ }
+ } else {
+ md_size = EVP_MD_size(md_type);
+ if (md_size < 0)
+ return 0;
+ if (!PKCS12_key_gen(pass, passlen, salt, saltlen,
PKCS12_MAC_ID, iter,
+ md_size, key, md_type)) {
+ PKCS12error(PKCS12_R_KEY_GEN_ERROR);
+ return 0;
+ }
}
HMAC_CTX_init(&hmac);
if (!HMAC_Init_ex(&hmac, key, md_size, md_type, NULL) ||
diff --git a/src/lib/libcrypto/pkcs12/pkcs12.h
b/src/lib/libcrypto/pkcs12/pkcs12.h
index 56635f9d7e0a..4dab109bbc3a 100644
--- a/src/lib/libcrypto/pkcs12/pkcs12.h
+++ b/src/lib/libcrypto/pkcs12/pkcs12.h
@@ -91,6 +91,11 @@ extern "C" {
#define PKCS12_add_friendlyname PKCS12_add_friendlyname_asc
#endif
+#define PKCS12_GOST_KEY_LEN 32
+int PKCS12_key_gen_gost(const char *pass, int passlen, unsigned char *salt,
+int saltlen, int iter, int n, unsigned char *out,
+const EVP_MD *md_type);
+
/* MS key usage constants */
#define KEY_EX 0x10
--
2.25.1
[PATCH 1/3] Add OIDs for GOST R 34.11-2012 HMAC functions
Add OIDs for HMAC using Streebog (GOST R 34.11-2012) hash function. Sponsored by ROSA Linux Signed-off-by: Dmitry Baryshkov --- src/lib/libcrypto/objects/obj_mac.num | 2 ++ src/lib/libcrypto/objects/objects.txt | 2 ++ 2 files changed, 4 insertions(+) diff --git a/src/lib/libcrypto/objects/obj_mac.num b/src/lib/libcrypto/objects/obj_mac.num index 3f0b5666fb51..ba75ec246eb5 100644 --- a/src/lib/libcrypto/objects/obj_mac.num +++ b/src/lib/libcrypto/objects/obj_mac.num @@ -996,3 +996,5 @@ id_tc26_gost_3410_12_256_paramSetC 995 id_tc26_gost_3410_12_256_paramSetD 996 id_tc26_gost_3410_12_512_paramSetTest 997 id_tc26_gost_3410_12_512_paramSetC 998 +id_tc26_hmac_gost_3411_12_256 999 +id_tc26_hmac_gost_3411_12_512 1000 diff --git a/src/lib/libcrypto/objects/objects.txt b/src/lib/libcrypto/objects/objects.txt index 42f31c3cbec1..8e533530f28d 100644 --- a/src/lib/libcrypto/objects/objects.txt +++ b/src/lib/libcrypto/objects/objects.txt @@ -1372,6 +1372,8 @@ member-body 643 7 1 : tc26 tc26 1 2 2 : streebog256 : GOST R 34.11-2012 (256 bit) !Cname id-tc26-gost3411-2012-512 tc26 1 2 3 : streebog512 : GOST R 34-11-2012 (512 bit) +tc26 1 4 1 : id-tc26-hmac-gost-3411-12-256 : HMAC STREEBOG 256 +tc26 1 4 2 : id-tc26-hmac-gost-3411-12-512 : HMAC STREEBOG 512 tc26 2 1 1 1 : id-tc26-gost-3410-12-256-paramSetA : GOST R 34.10-2012 (256 bit) ParamSet A tc26 2 1 1 2 : id-tc26-gost-3410-12-256-paramSetB : GOST R 34.10-2012 (256 bit) ParamSet B tc26 2 1 1 3 : id-tc26-gost-3410-12-256-paramSetC : GOST R 34.10-2012 (256 bit) ParamSet C -- 2.25.1
[PATCH 2/3] Populate PBE table with GOST R 34.11-2012 HMAC ids
Allow using GOST R 34.11-2012 in PBE/PBKDF2/PKCS#5.
Sponsored by ROSA Linux
Signed-off-by: Dmitry Baryshkov
---
src/lib/libcrypto/evp/evp_pbe.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/src/lib/libcrypto/evp/evp_pbe.c b/src/lib/libcrypto/evp/evp_pbe.c
index de08c8d78c52..4305d026425a 100644
--- a/src/lib/libcrypto/evp/evp_pbe.c
+++ b/src/lib/libcrypto/evp/evp_pbe.c
@@ -114,6 +114,8 @@ static const EVP_PBE_CTL builtin_pbe[] = {
{EVP_PBE_TYPE_PRF, NID_hmacWithSHA384, -1, NID_sha384, 0},
{EVP_PBE_TYPE_PRF, NID_hmacWithSHA512, -1, NID_sha512, 0},
{EVP_PBE_TYPE_PRF, NID_id_HMACGostR3411_94, -1, NID_id_GostR3411_94, 0},
+ {EVP_PBE_TYPE_PRF, NID_id_tc26_hmac_gost_3411_12_256, -1,
NID_id_tc26_gost3411_2012_256, 0},
+ {EVP_PBE_TYPE_PRF, NID_id_tc26_hmac_gost_3411_12_512, -1,
NID_id_tc26_gost3411_2012_512, 0},
};
int
--
2.25.1
Re: pppx(4): kill forgotten splx(9) stuff
On Tue, Apr 07, 2020 at 01:51:45PM +0300, Vitaliy Makkoveev wrote:
> On Tue, Apr 07, 2020 at 11:54:01AM +0200, Claudio Jeker wrote:
> > Unsure about this one here. I would prefer if the panic remained for now
> > (mainly because of the XXXSMP NET_UNLOCK() dance just above). I wonder if
> > the
> > order of this could not be modified so that the NET_LOCK is released after
> > the RBT_REMOVE.
> Well, let's clenup these dances before. Diff below just breaks giant
> NET_LOCK() in pppxioctl() to many small.
Will be required in the distant future, forget please :)
>
> Index: net/if_pppx.c
> ===
> RCS file: /cvs/src/sys/net/if_pppx.c,v
> retrieving revision 1.81
> diff -u -p -r1.81 if_pppx.c
> --- net/if_pppx.c 7 Apr 2020 07:11:22 - 1.81
> +++ net/if_pppx.c 7 Apr 2020 10:46:34 -
> @@ -433,7 +433,6 @@ pppxioctl(dev_t dev, u_long cmd, caddr_t
> struct pppx_dev *pxd = pppx_dev2pxd(dev);
> int error = 0;
>
> - NET_LOCK();
> switch (cmd) {
> case PIPEXSMODE:
> /*
> @@ -447,46 +446,59 @@ pppxioctl(dev_t dev, u_long cmd, caddr_t
> break;
>
> case PIPEXASESSION:
> + NET_LOCK();
> error = pppx_add_session(pxd,
> (struct pipex_session_req *)addr);
> + NET_UNLOCK();
> break;
>
> case PIPEXDSESSION:
> + NET_LOCK();
> error = pppx_del_session(pxd,
> (struct pipex_session_close_req *)addr);
> + NET_UNLOCK();
> break;
>
> case PIPEXCSESSION:
> + NET_LOCK();
> error = pppx_config_session(pxd,
> (struct pipex_session_config_req *)addr);
> + NET_UNLOCK();
> break;
>
> case PIPEXGSTAT:
> + NET_LOCK();
> error = pppx_get_stat(pxd,
> (struct pipex_session_stat_req *)addr);
> + NET_UNLOCK();
> break;
>
> case PIPEXGCLOSED:
> + NET_LOCK();
> error = pppx_get_closed(pxd,
> (struct pipex_session_list_req *)addr);
> + NET_UNLOCK();
> break;
>
> case PIPEXSIFDESCR:
> + NET_LOCK();
> error = pppx_set_session_descr(pxd,
> (struct pipex_session_descr_req *)addr);
> + NET_UNLOCK();
> break;
>
> case FIONBIO:
> break;
> case FIONREAD:
> + NET_LOCK();
> *(int *)addr = mq_hdatalen(&pxd->pxd_svcq);
> + NET_UNLOCK();
> break;
>
> default:
> error = ENOTTY;
> break;
> }
> - NET_UNLOCK();
>
> return (error);
> }
fix wifi media: line during background scan
I've noticed that wireless interfaces in 11n mode show a "media:" line in ifconfig such as this while a background scan is in progress: media: IEEE802.11 autoselect (OFDM6) What is expected is a line showing active 11n mode, such as: media: IEEE802.11 autoselect (HT-MCS0 mode 11n) This happens because ieee80211_media_status() sees ic->ic_curmode as AUTO during background scans. Also, because net80211 forgets to reset ic_curmode back to MODE_11N when the background scan has finished, the displayed mode remains "autoselect (OFDM6)" until the interface is reset. This is just a cosmetic issue. Internally, the interface operates in 11n mode regardless. ok? diff 5e4be56314753be1a3ad288aa6b16bcb5257b37c /usr/src blob - 410c33358e72c4063f9f71bf69f6c72ecfc558d9 file + sys/net80211/ieee80211.c --- sys/net80211/ieee80211.c +++ sys/net80211/ieee80211.c @@ -728,6 +728,9 @@ ieee80211_media_status(struct ifnet *ifp, struct ifmed ic->ic_curmode == IEEE80211_MODE_11AC) imr->ifm_active |= ieee80211_mcs2media(ic, ni->ni_txmcs, ic->ic_curmode); + else if (ni->ni_flags & IEEE80211_NODE_HT) /* in MODE_AUTO */ + imr->ifm_active |= ieee80211_mcs2media(ic, + ni->ni_txmcs, IEEE80211_MODE_11N); else /* calculate rate subtype */ imr->ifm_active |= ieee80211_rate2media(ic, blob - 6656d29d160c26dce86fb44e3f5e715e42b7c42c file + sys/net80211/ieee80211_node.c --- sys/net80211/ieee80211_node.c +++ sys/net80211/ieee80211_node.c @@ -1441,6 +1441,19 @@ ieee80211_end_scan(struct ifnet *ifp) ic->ic_bgscan_fail *= 2; } ic->ic_flags &= ~IEEE80211_F_BGSCAN; + + /* +* HT is negotiated during association so we must use +* ic_bss to check HT. The nodes tree was re-populated +* during background scan and therefore selbs and curbs +* may not carry HT information. +*/ + ni = ic->ic_bss; + if (ni->ni_flags & IEEE80211_NODE_HT) + ieee80211_setmode(ic, IEEE80211_MODE_11N); + else + ieee80211_setmode(ic, + ieee80211_chan2mode(ic, ni->ni_chan)); return; }
Re: pppx(4): kill forgotten splx(9) stuff
On Tue, Apr 07, 2020 at 11:54:01AM +0200, Claudio Jeker wrote:
> Unsure about this one here. I would prefer if the panic remained for now
> (mainly because of the XXXSMP NET_UNLOCK() dance just above). I wonder if the
> order of this could not be modified so that the NET_LOCK is released after
> the RBT_REMOVE.
Well, let's clenup these dances before. Diff below just breaks giant
NET_LOCK() in pppxioctl() to many small.
Index: net/if_pppx.c
===
RCS file: /cvs/src/sys/net/if_pppx.c,v
retrieving revision 1.81
diff -u -p -r1.81 if_pppx.c
--- net/if_pppx.c 7 Apr 2020 07:11:22 - 1.81
+++ net/if_pppx.c 7 Apr 2020 10:46:34 -
@@ -433,7 +433,6 @@ pppxioctl(dev_t dev, u_long cmd, caddr_t
struct pppx_dev *pxd = pppx_dev2pxd(dev);
int error = 0;
- NET_LOCK();
switch (cmd) {
case PIPEXSMODE:
/*
@@ -447,46 +446,59 @@ pppxioctl(dev_t dev, u_long cmd, caddr_t
break;
case PIPEXASESSION:
+ NET_LOCK();
error = pppx_add_session(pxd,
(struct pipex_session_req *)addr);
+ NET_UNLOCK();
break;
case PIPEXDSESSION:
+ NET_LOCK();
error = pppx_del_session(pxd,
(struct pipex_session_close_req *)addr);
+ NET_UNLOCK();
break;
case PIPEXCSESSION:
+ NET_LOCK();
error = pppx_config_session(pxd,
(struct pipex_session_config_req *)addr);
+ NET_UNLOCK();
break;
case PIPEXGSTAT:
+ NET_LOCK();
error = pppx_get_stat(pxd,
(struct pipex_session_stat_req *)addr);
+ NET_UNLOCK();
break;
case PIPEXGCLOSED:
+ NET_LOCK();
error = pppx_get_closed(pxd,
(struct pipex_session_list_req *)addr);
+ NET_UNLOCK();
break;
case PIPEXSIFDESCR:
+ NET_LOCK();
error = pppx_set_session_descr(pxd,
(struct pipex_session_descr_req *)addr);
+ NET_UNLOCK();
break;
case FIONBIO:
break;
case FIONREAD:
+ NET_LOCK();
*(int *)addr = mq_hdatalen(&pxd->pxd_svcq);
+ NET_UNLOCK();
break;
default:
error = ENOTTY;
break;
}
- NET_UNLOCK();
return (error);
}
bug: fifo kqfilter & FREAD|FWRITE
fifo_poll() honors FREAD and FWRITE. It doesn't return events
incompatible with the open flags. Diff below makes the kqfilters
behave like that as well.
ok?
Index: kern/tty_tty.c
===
RCS file: /cvs/src/sys/kern/tty_tty.c,v
retrieving revision 1.24
diff -u -p -r1.24 tty_tty.c
--- kern/tty_tty.c 2 May 2018 02:24:56 - 1.24
+++ kern/tty_tty.c 7 Apr 2020 10:36:08 -
@@ -160,5 +160,5 @@ cttykqfilter(dev_t dev, struct knote *kn
if (ttyvp == NULL)
return (ENXIO);
- return (VOP_KQFILTER(ttyvp, kn));
+ return (VOP_KQFILTER(ttyvp, FREAD|FWRITE, kn));
}
Index: kern/vfs_vnops.c
===
RCS file: /cvs/src/sys/kern/vfs_vnops.c,v
retrieving revision 1.113
diff -u -p -r1.113 vfs_vnops.c
--- kern/vfs_vnops.c22 Feb 2020 11:58:29 - 1.113
+++ kern/vfs_vnops.c7 Apr 2020 10:36:08 -
@@ -619,7 +619,7 @@ vn_closefile(struct file *fp, struct pro
int
vn_kqfilter(struct file *fp, struct knote *kn)
{
- return (VOP_KQFILTER(fp->f_data, kn));
+ return (VOP_KQFILTER(fp->f_data, fp->f_flag, kn));
}
int
Index: kern/vfs_vops.c
===
RCS file: /cvs/src/sys/kern/vfs_vops.c,v
retrieving revision 1.27
diff -u -p -r1.27 vfs_vops.c
--- kern/vfs_vops.c 31 Mar 2020 06:54:05 - 1.27
+++ kern/vfs_vops.c 7 Apr 2020 10:36:08 -
@@ -307,10 +307,11 @@ VOP_POLL(struct vnode *vp, int fflag, in
}
int
-VOP_KQFILTER(struct vnode *vp, struct knote *kn)
+VOP_KQFILTER(struct vnode *vp, int fflag, struct knote *kn)
{
struct vop_kqfilter_args a;
a.a_vp = vp;
+ a.a_fflag = fflag;
a.a_kn = kn;
if (vp->v_op->vop_kqfilter == NULL)
Index: sys/vnode.h
===
RCS file: /cvs/src/sys/sys/vnode.h,v
retrieving revision 1.155
diff -u -p -r1.155 vnode.h
--- sys/vnode.h 20 Jan 2020 23:21:56 - 1.155
+++ sys/vnode.h 7 Apr 2020 10:36:08 -
@@ -412,9 +412,10 @@ int VOP_POLL(struct vnode *, int, int, s
struct vop_kqfilter_args {
struct vnode *a_vp;
+ int a_fflag;
struct knote *a_kn;
};
-int VOP_KQFILTER(struct vnode *, struct knote *);
+int VOP_KQFILTER(struct vnode *, int, struct knote *);
struct vop_revoke_args {
struct vnode *a_vp;
Index: miscfs/fifofs/fifo_vnops.c
===
RCS file: /cvs/src/sys/miscfs/fifofs/fifo_vnops.c,v
retrieving revision 1.73
diff -u -p -r1.73 fifo_vnops.c
--- miscfs/fifofs/fifo_vnops.c 20 Feb 2020 16:56:52 - 1.73
+++ miscfs/fifofs/fifo_vnops.c 7 Apr 2020 10:37:42 -
@@ -512,10 +512,14 @@ fifo_kqfilter(void *v)
switch (ap->a_kn->kn_filter) {
case EVFILT_READ:
+ if (!(ap->a_fflag & FREAD))
+ return (EINVAL);
ap->a_kn->kn_fop = &fiforead_filtops;
sb = &so->so_rcv;
break;
case EVFILT_WRITE:
+ if (!(ap->a_fflag & FWRITE))
+ return (EINVAL);
ap->a_kn->kn_fop = &fifowrite_filtops;
sb = &so->so_snd;
break;
fifo kqueue bug
The write socket should be passed to the write filter otherwise checks
are performed against the wrong socket.
ok?
Index: miscfs/fifofs/fifo_vnops.c
===
RCS file: /cvs/src/sys/miscfs/fifofs/fifo_vnops.c,v
retrieving revision 1.73
diff -u -p -r1.73 fifo_vnops.c
--- miscfs/fifofs/fifo_vnops.c 20 Feb 2020 16:56:52 - 1.73
+++ miscfs/fifofs/fifo_vnops.c 7 Apr 2020 10:06:13 -
@@ -507,16 +507,19 @@ int
fifo_kqfilter(void *v)
{
struct vop_kqfilter_args *ap = v;
- struct socket *so = (struct socket *)ap->a_vp->v_fifoinfo->fi_readsock;
+ struct socket *so;
+ struct fifoinfo *fip = ap->a_vp->v_fifoinfo;
struct sockbuf *sb;
switch (ap->a_kn->kn_filter) {
case EVFILT_READ:
ap->a_kn->kn_fop = &fiforead_filtops;
+ so = fip->fi_readsock;
sb = &so->so_rcv;
break;
case EVFILT_WRITE:
ap->a_kn->kn_fop = &fifowrite_filtops;
+ so = fip->fi_writesock;
sb = &so->so_snd;
break;
default:
Re: pppx(4): kill forgotten splx(9) stuff
On Tue, Apr 07, 2020 at 12:36:29PM +0300, Vitaliy Makkoveev wrote:
> pppx_if containing tree and per pppx_dev list are protected by rwlock so
> these splx(9) related dances and commentaries are not actual.
> Also pxd_svcq protected by NET_LOCK().
>
> Index: sys/net/if_pppx.c
> ===
> RCS file: /cvs/src/sys/net/if_pppx.c,v
> retrieving revision 1.81
> diff -u -p -r1.81 if_pppx.c
> --- sys/net/if_pppx.c 7 Apr 2020 07:11:22 - 1.81
> +++ sys/net/if_pppx.c 7 Apr 2020 09:06:35 -
> @@ -126,7 +126,7 @@ struct pppx_dev {
> struct selinfo pxd_wsel;
> struct mutexpxd_wsel_mtx;
>
> - /* queue of packets for userland to service - protected by splnet */
> + /* queue of packets for userland to service - protected by NET_LOCK() */
> struct mbuf_queue pxd_svcq;
> int pxd_waiting;
> LIST_HEAD(,pppx_if) pxd_pxis;
> @@ -622,7 +622,6 @@ pppx_if_next_unit(void)
>
> rw_assert_wrlock(&pppx_ifs_lk);
>
> - /* this is safe without splnet since we're not modifying it */
> do {
> int found = 0;
> RBT_FOREACH(pxi, pppx_ifs, &pppx_ifs) {
> @@ -842,7 +841,6 @@ pppx_add_session(struct pppx_dev *pxd, s
> pxi->pxi_key.pxik_protocol = req->pr_protocol;
> pxi->pxi_dev = pxd;
>
> - /* this is safe without splnet since we're not modifying it */
> if (RBT_FIND(pppx_ifs, &pppx_ifs, pxi) != NULL) {
> pool_put(pppx_if_pl, pxi);
> error = EADDRINUSE;
> @@ -850,8 +848,7 @@ pppx_add_session(struct pppx_dev *pxd, s
> goto out;
> }
>
> - if (RBT_INSERT(pppx_ifs, &pppx_ifs, pxi) != NULL)
> - panic("%s: pppx_ifs modified while lock was held", __func__);
> + RBT_INSERT(pppx_ifs, &pppx_ifs, pxi);
I would merge this into the RBT_FIND() above like this:
if (RBT_INSERT(pppx_ifs, &pppx_ifs, pxi) != NULL) {
pool_put(pppx_if_pl, pxi);
error = EADDRINUSE;
rw_exit_write(&pppx_ifs_lk);
goto out;
}
This does the same thing but without two lookups.
> LIST_INSERT_HEAD(&pxd->pxd_pxis, pxi, pxi_list);
> rw_exit_write(&pppx_ifs_lk);
>
> @@ -1039,8 +1036,7 @@ pppx_if_destroy(struct pppx_dev *pxd, st
> NET_LOCK();
>
> rw_enter_write(&pppx_ifs_lk);
> - if (RBT_REMOVE(pppx_ifs, &pppx_ifs, pxi) == NULL)
> - panic("%s: pppx_ifs modified while lock was held", __func__);
> + RBT_REMOVE(pppx_ifs, &pppx_ifs, pxi);
> LIST_REMOVE(pxi, pxi_list);
> rw_exit_write(&pppx_ifs_lk);
>
Unsure about this one here. I would prefer if the panic remained for now
(mainly because of the XXXSMP NET_UNLOCK() dance just above). I wonder if the
order of this could not be modified so that the NET_LOCK is released after
the RBT_REMOVE.
--
:wq Claudio
pppx(4): kill forgotten splx(9) stuff
pppx_if containing tree and per pppx_dev list are protected by rwlock so
these splx(9) related dances and commentaries are not actual.
Also pxd_svcq protected by NET_LOCK().
Index: sys/net/if_pppx.c
===
RCS file: /cvs/src/sys/net/if_pppx.c,v
retrieving revision 1.81
diff -u -p -r1.81 if_pppx.c
--- sys/net/if_pppx.c 7 Apr 2020 07:11:22 - 1.81
+++ sys/net/if_pppx.c 7 Apr 2020 09:06:35 -
@@ -126,7 +126,7 @@ struct pppx_dev {
struct selinfo pxd_wsel;
struct mutexpxd_wsel_mtx;
- /* queue of packets for userland to service - protected by splnet */
+ /* queue of packets for userland to service - protected by NET_LOCK() */
struct mbuf_queue pxd_svcq;
int pxd_waiting;
LIST_HEAD(,pppx_if) pxd_pxis;
@@ -622,7 +622,6 @@ pppx_if_next_unit(void)
rw_assert_wrlock(&pppx_ifs_lk);
- /* this is safe without splnet since we're not modifying it */
do {
int found = 0;
RBT_FOREACH(pxi, pppx_ifs, &pppx_ifs) {
@@ -842,7 +841,6 @@ pppx_add_session(struct pppx_dev *pxd, s
pxi->pxi_key.pxik_protocol = req->pr_protocol;
pxi->pxi_dev = pxd;
- /* this is safe without splnet since we're not modifying it */
if (RBT_FIND(pppx_ifs, &pppx_ifs, pxi) != NULL) {
pool_put(pppx_if_pl, pxi);
error = EADDRINUSE;
@@ -850,8 +848,7 @@ pppx_add_session(struct pppx_dev *pxd, s
goto out;
}
- if (RBT_INSERT(pppx_ifs, &pppx_ifs, pxi) != NULL)
- panic("%s: pppx_ifs modified while lock was held", __func__);
+ RBT_INSERT(pppx_ifs, &pppx_ifs, pxi);
LIST_INSERT_HEAD(&pxd->pxd_pxis, pxi, pxi_list);
rw_exit_write(&pppx_ifs_lk);
@@ -1039,8 +1036,7 @@ pppx_if_destroy(struct pppx_dev *pxd, st
NET_LOCK();
rw_enter_write(&pppx_ifs_lk);
- if (RBT_REMOVE(pppx_ifs, &pppx_ifs, pxi) == NULL)
- panic("%s: pppx_ifs modified while lock was held", __func__);
+ RBT_REMOVE(pppx_ifs, &pppx_ifs, pxi);
LIST_REMOVE(pxi, pxi_list);
rw_exit_write(&pppx_ifs_lk);
Re: iked(8): simplify data in sc_sock4 and sc_sock6
Hi,
thank you, most of this diff looks good to me. I left some comments inline.
On Sun, Apr 05, 2020 at 01:58:04AM +0900, Wataru Ashihara wrote:
> The data wich sc_sock4 has is a little bit complicated:
>
>
> sc_sock4[0], sc_sock6[0] sc_sock4[1], sc_sock6[1]
>
> no flag (non NAT-T) :500 (IKE_SA_INIT, IKE_AUTH) :4500 (not used)
> no flag (NAT-T) :500 (IKE_SA_INIT) :4500 (IKE_AUTH)
> -t :500 (IKE_SA_INIT, IKE_AUTH) :500 (never used) (!)
> -T :4500 (IKE_SA_INIT, IKE_AUTH) NULL (never used)
> -p4500 :4500 (IKE_SA_INIT, IKE_AUTH) :4500 (never used) (!)
> -p14500 :14500 (IKE_SA_INIT, IKE_AUTH) NULL (never used)
>
>
> Here "IKE_AUTH" includes CREATE_CHILD_SA and INFORMATIONAL as well and
> the same applies to sc_sock6. This patch eliminates the never-used
> data:
>
>
> first call second call
> (sc_sock4[0], sc_sock6[0]) (sc_sock4[1],
> sc_sock6[1])
>
> no flag (non NAT-T) :500 (IKE_SA_INIT, IKE_AUTH) :4500 (not used)
> no flag (NAT-T) :500 (IKE_SA_INIT) :4500 (IKE_AUTH)
> -t :500 (IKE_SA_INIT, IKE_AUTH) NULL (never used)
> -T (== -p4500) :4500 (IKE_SA_INIT, IKE_AUTH) NULL (never used)
> -p14500 :14500 (IKE_SA_INIT, IKE_AUTH) NULL (never used)
>
Looks good.
>
> I introduced "enum natt_mode" because "if" conditions in
> parent_configure() had got to be rather complicated:
>
> if (((env->sc_opts & IKED_OPT_NATT) == 0) || ((env->sc_opts &
> IKED_OPT_NONATT) != 0))
> config_setsocket(env, &ss, htons(IKED_IKE_PORT), PROC_IKEV2);
> if (((env->sc_opts & IKED_OPT_NATT) != 0) || ((env->sc_opts &
> IKED_OPT_NONATT) == 0))
> config_setsocket(env, &ss, htons(env->sc_nattport), PROC_IKEV2);
>
Makes sense, good idea.
>
> Index: sbin/iked/config.c
> ===
> RCS file: /cvs/src/sbin/iked/config.c,v
> retrieving revision 1.55
> diff -u -r1.55 config.c
> --- sbin/iked/config.c24 Mar 2020 13:32:36 - 1.55
> +++ sbin/iked/config.c4 Apr 2020 16:48:55 -
> @@ -545,6 +545,24 @@
> return (0);
> }
>
> +/**
> + * The first call of this function sets the UDP socket for IKE_INIT.
> + * The second one is optional, setting the UDP socket for IKE_AUTH,
> + * CREATE_CHILD_SA, and INFORMATIONAL.
> + *
> + * The port of each is determined by command line options:
> + *
> + *
>
> + * first call second call
> + * (sc_sock4[0], sc_sock6[0]) (sc_sock4[1],
> sc_sock6[1])
> + *
>
> + * no flag (non NAT-T) :500 (IKE_SA_INIT, IKE_AUTH) :4500 (not used)
> + * no flag (NAT-T) :500 (IKE_SA_INIT) :4500 (IKE_AUTH)
> + * -t :500 (IKE_SA_INIT, IKE_AUTH) NULL (never used)
> + * -T (== -p4500) :4500 (IKE_SA_INIT, IKE_AUTH) NULL (never used)
> + * -p14500 :14500 (IKE_SA_INIT, IKE_AUTH) NULL (never used)
> + *
>
> + */
I feel like this comment is a bit too verbose. I understand how the table
helped you understand what is going on here, but now that you cleaned this
up, wouldn't it be sufficient to write something like:
/*
* The first call of this function sets the UDP socket for IKEv2.
* The second call is optional, setting the UDP socket used for NAT-T.
*/
> int
> config_setsocket(struct iked *env, struct sockaddr_storage *ss,
> in_port_t port, enum privsep_procid id)
> @@ -562,7 +580,7 @@
> config_getsocket(struct iked *env, struct imsg *imsg,
> void (*cb)(int, short, void *))
> {
> - struct iked_socket *sock, **sptr, **nptr;
> + struct iked_socket *sock, **sock0, **sock1;
>
> log_debug("%s: received socket fd %d", __func__, imsg->fd);
>
> @@ -577,23 +595,24 @@
>
> switch (sock->sock_addr.ss_family) {
> case AF_INET:
> - sptr = &env->sc_sock4[0];
> - nptr = &env->sc_sock4[1];
> + sock0 = &env->sc_sock4[0];
> + sock1 = &env->sc_sock4[1];
> break;
> case AF_INET6:
> - sptr = &env->sc_s
FW: Add mprotect_ept ioctl to vmm(4)
> On Fri, Feb 07, 2020 at 01:25:38PM -0800, Mike Larkin wrote:
> > On Fri, Feb 07, 2020 at 04:20:16AM +, Adam Steen wrote:
> > > Hi
> > >
> > > Please see the attached patch to add an 'IOCTL handler to sets the access
> > > protections of the ept'
> > >
> > > vmd(8) does not make use of this change, but solo5, which uses vmm(4) as
> > > a backend hypervisor. The code calling 'VMM_IOC_MPROTECT_EPT' is
> > > available here
> > > https://github.com/Solo5/solo5/compare/master...adamsteen:wnox
> > >
> > > there are changes to vmd too, but this is just to ensure completeness,
> > > if mprotect ept is called in the future, we would want the vm to be
> > > stopped if we get a protection fault.
> > >
> > > I was unsure what todo if called with execute only permissions on a cpu
> > > that
> > > does not support it. I went with add read permissions and logging the
> > > fact, instead of returning EINVAL.
> > >
> > > Cheers
> > > Adam
> > >
> >
> > I have been giving Adam feedback on this diff for a while. There are a few
> > minor comments below, but I think this is ok if someone wants to commit it
> > after
> > the fixes below are incorporated.
> >
> > -ml
> >
>
> See updated comment below.
>
> -ml
> > > + /* No W^X permissions */
> > > + if ((prot & PROT_MASK) != prot &&
> > > + (prot & (PROT_WRITE | PROT_EXEC)) == (PROT_WRITE | PROT_EXEC)) {
> > > + DPRINTF("%s: No W^X permissions\n", __func__);
> > > + return (EINVAL);
> > > + }
> >
> > I would probably reword this to "W+X permission requested".
> >
> > > +/*
> > > + * vmx_mprotect_ept
> > > + *
> > > + * apply the ept protections to the requested pages, faulting the page if
> >
> > "faulting in"
> >
> > > + * required.
> > > + */
> > > +int
> > > +vmx_mprotect_ept(vm_map_t vm_map, paddr_t sgpa, paddr_t egpa, int prot)
> > > +{
> > > + struct vmx_invept_descriptor vid;
> > > + pmap_t pmap;
> > > + pt_entry_t *pte;
> > > + paddr_t addr;
> > > + int ret = 0;
> > > > +
> > > + pmap = vm_map->pmap;
> > > +
> > > + for (addr = sgpa; addr < egpa; addr += PAGE_SIZE) {
> > > + pte = vmx_pmap_find_pte_ept(pmap, addr);
> > > + if (pte == NULL) {
> >
> > if (pte & PG_V) == 0
> >
>
> After reading a reply from Adam, I think the original suggested way is fine.
> My idea of checking PG_V is fine for RWX EPT entries, but as soon as anyone
> uses XO entries, this check wouldn't work.
>
> -ml
Hi
I have incorporated the fixes ml requested above and a fixed a few nits
from pd@, but with the crazyness of life these days, i haven't been able to it
commited, so i attaching it again below.
Cheers
Adam
diff refs/heads/master refs/heads/mprotect_ept
blob - b0a08291108adc4b21680e4265c520873c13ebff
blob + ce8e21406dde9a5ad4a8505b5cacf10acf4a7379
--- sys/arch/amd64/amd64/vmm.c
+++ sys/arch/amd64/amd64/vmm.c
@@ -124,6 +124,7 @@ int vm_get_info(struct vm_info_params *);
int vm_resetcpu(struct vm_resetcpu_params *);
int vm_intr_pending(struct vm_intr_params *);
int vm_rwregs(struct vm_rwregs_params *, int);
+int vm_mprotect_ept(struct vm_mprotect_ept_params *);
int vm_rwvmparams(struct vm_rwvmparams_params *, int);
int vm_find(uint32_t, struct vm **);
int vcpu_readregs_vmx(struct vcpu *, uint64_t, struct vcpu_reg_state *);
@@ -186,6 +187,8 @@ int svm_fault_page(struct vcpu *, paddr_t);
int vmx_fault_page(struct vcpu *, paddr_t);
int vmx_handle_np_fault(struct vcpu *);
int svm_handle_np_fault(struct vcpu *);
+int vmx_mprotect_ept(vm_map_t, paddr_t, paddr_t, int);
+pt_entry_t *vmx_pmap_find_pte_ept(pmap_t, paddr_t);
int vmm_alloc_vpid(uint16_t *);
void vmm_free_vpid(uint16_t);
const char *vcpu_state_decode(u_int);
@@ -494,6 +497,9 @@ vmmioctl(dev_t dev, u_long cmd, caddr_t data, int flag
case VMM_IOC_WRITEREGS:
ret = vm_rwregs((struct vm_rwregs_params *)data, 1);
break;
+ case VMM_IOC_MPROTECT_EPT:
+ ret = vm_mprotect_ept((struct vm_mprotect_ept_params *)data);
+ break;
case VMM_IOC_READVMPARAMS:
ret = vm_rwvmparams((struct vm_rwvmparams_params *)data, 0);
break;
@@ -532,6 +538,7 @@ pledge_ioctl_vmm(struct proc *p, long com)
case VMM_IOC_INTR:
case VMM_IOC_READREGS:
case VMM_IOC_WRITEREGS:
+ case VMM_IOC_MPROTECT_EPT:
case VMM_IOC_READVMPARAMS:
case VMM_IOC_WRITEVMPARAMS:
return (0);
@@ -807,6 +814,288 @@ vm_rwregs(struct vm_rwregs_params *vrwp, int dir)
}
/*
+ * vm_mprotect_ept
+ *
+ * IOCTL handler to sets the access protections of the ept
+ *
+ * Parameters:
+ * vmep: decribes the memory for which the protect will be applied..
+ *
+ * Return values:
+ * 0: if successful
+ * ENOENT: if the VM defined by 'vmep' cannot be found
+ * EINVAL: if the sgpa or size is not page aligned, the prot is invalid,
+ * size is too large (512GB), there is wraparound
+ * (like start = 512GB-1 and end = 512GB-2),
+ * the address specified
