Re: patch for httpd implementing clickjacking protection
On 07.02., Peter J. Philipp wrote:
> On Tue, Feb 07, 2023 at 10:41:34AM +, Stuart Henderson wrote:
> > On 2023/02/07 10:20, Peter J. Philipp wrote:
> > > Hi,
> > >
> > > Arslan Kabeer (on the Internet) made me aware of clickjacking being done
> > > on
> > > my site using OpenBSD httpd. This following patch implements a RFC 7034
> > > protection called "noiframe" which disallows other sites (but not the same
> > > site) to add an iframe to my site.
> > >
> > > The config change is like this:
> > >
> > > ->
> > > location "/" {
> > > directory index index.html
> > > noiframe
> >
> > Using a specific keyword for every site protection header that
> > somebody might want seems a bit much. (There are other settings for
> > x-frame-options, other headers like content-security-policy and
> > x-content-type-options, and various deprecated ones).
> >
> > Wouldn't a general-purpose "set header X with the value Y" make
> > more sense?
>
> Yes this makes more sense. Ignore my patch then, it was whipped up this
> morning when I got the vulnerability report from Arslan. I'm unable to
> look deeper and general purposely into this, though, I have other TODO's.
>
> It seems a mystery to me however how to add this header into httpd based
> off the manual page if that is the hint. Perhaps the maintainer of this
> program now has an idea what we need and can schedule programming for it.
>
> Best Regards,
> -peter
>
Using relayd(8) in front of httpd(8) you can easily handle any HTTP header
the way you like: add or remove headers from both requests and responses.
Cheers,
Bruno
Re: pf max-src-{states,conn} without overload/flush useless?
On Thu, 09 Feb 2023 at 02:42:22 +0100, Alexandr Nedvedicky wrote:
> this is my test terminal on remote host:
> router$ for i in `seq 5` ; do nc 192.168.2.175 22 & done
> [1] 32472
> [2] 97453
> [3] 7192
> [4] 50386
> [5] 57517
> router$ SSH-2.0-OpenSSH_9.1
> SSH-2.0-OpenSSH_9.1
> SSH-2.0-OpenSSH_9.1
>
> there are three SSH version strings which indicates
> I got 3 working connection of 5 attempts.
Interesting, I tried with your SSH example and it allowed me to
connect 5 times.
$ for i in `seq 5` ; do nc 192.168.1.240 22 & done
[2] 68892
[3] 6303
[4] 63554
[5] 87833
[6] 49997
$ SSH-2.0-OpenSSH_9.1
SSH-2.0-OpenSSH_9.1
SSH-2.0-OpenSSH_9.1
SSH-2.0-OpenSSH_9.1
SSH-2.0-OpenSSH_9.1
vm:~$ doas pfctl -sr
block return all
pass out all flags S/SA
pass in on egress inet6 proto tcp from any to ::1 port = 22 flags S/SA keep
state (source-track rule, max-src-conn 3)
pass in on egress inet proto tcp from any to 127.0.0.1 port = 22 flags S/SA
keep state (source-track rule, max-src-conn 3)
pass in on egress inet proto tcp from any to 192.168.1.240 port = 22 flags S/SA
keep state (source-track rule, max-src-conn 3)
This is with:
OpenBSD 7.2-current (GENERIC.MP) #2014: Tue Feb 7 16:24:04 MST 2023
dera...@arm64.openbsd.org:/usr/src/sys/arch/arm64/compile/GENERIC.MP
> > diff --git sys/net/pf.c sys/net/pf.c
> > index 8cb1326a160..89703feab12 100644
> > --- sys/net/pf.c
> > +++ sys/net/pf.c
> > @@ -481,12 +481,10 @@ pf_src_connlimit(struct pf_state **stp)
> > if ((sn = pf_get_src_node((*stp), PF_SN_NONE)) == NULL)
> > return (0);
> >
> > - sn->conn++;
> > - (*stp)->src.tcp_est = 1;
> > pf_add_threshold(>conn_rate);
> >
> > if ((*stp)->rule.ptr->max_src_conn &&
> > - (*stp)->rule.ptr->max_src_conn < sn->conn) {
> > + sn->conn >= (*stp)->rule.ptr->max_src_conn) {
> > pf_status.lcounters[LCNT_SRCCONN]++;
> > bad++;
> > }
> > @@ -497,8 +495,11 @@ pf_src_connlimit(struct pf_state **stp)
> > bad++;
> > }
> >
> > - if (!bad)
> > + if (!bad) {
> > + sn->conn++;
> > + (*stp)->src.tcp_est = 1;
> > return (0);
> > + }
> >
> > if ((*stp)->rule.ptr->overload_tbl) {
> > struct pfr_addr p;
>
> it seems to me the change to pf_src_connlimit() does
> not alter behavior. I think change to pf_src_connlimit()
> can be dropped.
But don't we not want to increment the source node's connection
count since we're not going to accept the connection (in the !bad
case)? I'm not sure what kind of bookkeeping that would screw up.
> > @@ -4919,14 +4920,14 @@ pf_tcp_track_full(struct pf_pdesc *pd, struct
> > pf_state **stp, u_short *reason,
> > pf_set_protostate(*stp, psrc, TCPS_CLOSING);
> > if (th->th_flags & TH_ACK) {
> > if (dst->state == TCPS_SYN_SENT) {
> > - pf_set_protostate(*stp, pdst,
> > - TCPS_ESTABLISHED);
> > - if (src->state == TCPS_ESTABLISHED &&
> > + if (src->state == TCPS_SYN_SENT &&
> > !SLIST_EMPTY(&(*stp)->src_nodes) &&
> > pf_src_connlimit(stp)) {
> > REASON_SET(reason, PFRES_SRCLIMIT);
> > return (PF_DROP);
> > }
> > + pf_set_protostate(*stp, pdst,
> > + TCPS_ESTABLISHED);
> > } else if (dst->state == TCPS_CLOSING)
> > pf_set_protostate(*stp, pdst,
> > TCPS_FIN_WAIT_2);
> >
>
> If I understand things right then in current pf we check conn limit
> when we see acknowledgment of 3rd client's ACK sent by server. Not sure
> if I sound clear here so let me draw little diagram:
>
> SYN > sent by client moves state to
>
> TCPS_SYN_SENT : TCPS_LISTEN/TCPS_CLOSED (1)
>
> SYN | ACK < sent by server moves state to
>
> TCPS_SYN_SENT : TCPS_SYN_SENT (2)
>
> ACK > 3rd ack sent by client move state to:
>
> TCPS_ESTABLISHED : TCPS_SYN_SENT (3)
>
> ACK < server acknowledges client's 3rd ack moves state to
>
> TCPS_ESTABLISHED : TCPS_ESTABLISHED (4)
>
> currently we do conn limit check in step (4). Your change moves this
> to earlier step (3) (given I understand things right here).
> It's awfully early here I need sleep on this.
Yes, that was my understanding too. We wait until the remote has
done enough work to be a valid connection but then block it before
sending the final ack.
> can you give it a try with your slightly modified diff? just drop
> changes to pf_src_connlimit() and keep those in pf_tcp_track_full() which
> I believe is the only
Re: pf max-src-{states,conn} without overload/flush useless?
Hello,
I did test similar rules on my system
OpenBSD 7.2-current (GENERIC.MP) #978: Sun Jan 22 11:41:04 MST 2023
these are my rules:
set skip on lo
block return# block stateless traffic
pass out log# establish keep-state
pass in on iwn0 proto tcp from 192.168.2.0/24 to self port 22 \
keep state (source-track rule, max-src-conn 3)
this is my test terminal on remote host:
router$ for i in `seq 5` ; do nc 192.168.2.175 22 & done
[1] 32472
[2] 97453
[3] 7192
[4] 50386
[5] 57517
router$ SSH-2.0-OpenSSH_9.1
SSH-2.0-OpenSSH_9.1
SSH-2.0-OpenSSH_9.1
there are three SSH version strings which indicates
I got 3 working connection of 5 attempts.
I'm not sure why it seems to work for me (ssh) while
it is broken for you (http). I'll give it a try with
webserver.
On Wed, Feb 08, 2023 at 04:34:55PM -0600, joshua stein wrote:
> diff --git sys/net/pf.c sys/net/pf.c
> index 8cb1326a160..89703feab12 100644
> --- sys/net/pf.c
> +++ sys/net/pf.c
> @@ -481,12 +481,10 @@ pf_src_connlimit(struct pf_state **stp)
> if ((sn = pf_get_src_node((*stp), PF_SN_NONE)) == NULL)
> return (0);
>
> - sn->conn++;
> - (*stp)->src.tcp_est = 1;
> pf_add_threshold(>conn_rate);
>
> if ((*stp)->rule.ptr->max_src_conn &&
> - (*stp)->rule.ptr->max_src_conn < sn->conn) {
> + sn->conn >= (*stp)->rule.ptr->max_src_conn) {
> pf_status.lcounters[LCNT_SRCCONN]++;
> bad++;
> }
> @@ -497,8 +495,11 @@ pf_src_connlimit(struct pf_state **stp)
> bad++;
> }
>
> - if (!bad)
> + if (!bad) {
> + sn->conn++;
> + (*stp)->src.tcp_est = 1;
> return (0);
> + }
>
> if ((*stp)->rule.ptr->overload_tbl) {
> struct pfr_addr p;
it seems to me the change to pf_src_connlimit() does
not alter behavior. I think change to pf_src_connlimit()
can be dropped.
> @@ -4919,14 +4920,14 @@ pf_tcp_track_full(struct pf_pdesc *pd, struct
> pf_state **stp, u_short *reason,
> pf_set_protostate(*stp, psrc, TCPS_CLOSING);
> if (th->th_flags & TH_ACK) {
> if (dst->state == TCPS_SYN_SENT) {
> - pf_set_protostate(*stp, pdst,
> - TCPS_ESTABLISHED);
> - if (src->state == TCPS_ESTABLISHED &&
> + if (src->state == TCPS_SYN_SENT &&
> !SLIST_EMPTY(&(*stp)->src_nodes) &&
> pf_src_connlimit(stp)) {
> REASON_SET(reason, PFRES_SRCLIMIT);
> return (PF_DROP);
> }
> + pf_set_protostate(*stp, pdst,
> + TCPS_ESTABLISHED);
> } else if (dst->state == TCPS_CLOSING)
> pf_set_protostate(*stp, pdst,
> TCPS_FIN_WAIT_2);
>
If I understand things right then in current pf we check conn limit
when we see acknowledgment of 3rd client's ACK sent by server. Not sure
if I sound clear here so let me draw little diagram:
SYN > sent by client moves state to
TCPS_SYN_SENT : TCPS_LISTEN/TCPS_CLOSED (1)
SYN | ACK < sent by server moves state to
TCPS_SYN_SENT : TCPS_SYN_SENT (2)
ACK > 3rd ack sent by client move state to:
TCPS_ESTABLISHED : TCPS_SYN_SENT (3)
ACK < server acknowledges client's 3rd ack moves state to
TCPS_ESTABLISHED : TCPS_ESTABLISHED (4)
currently we do conn limit check in step (4). Your change moves this
to earlier step (3) (given I understand things right here).
It's awfully early here I need sleep on this.
can you give it a try with your slightly modified diff? just drop
changes to pf_src_connlimit() and keep those in pf_tcp_track_full() which
I believe is the only relevant part.
thanks and
regards
sashan
pf max-src-{states,conn} without overload/flush useless?
I want to limit incoming connections on a server to 5 per IP. I
don't want to put violators into a pf table (overload) or kill the
other connections (flush), I just want to not accept new connections
from that IP once their limit is reached and then accept them again
when they are under the limit. Is this broken or am I holding it
wrong?
With a simple pf.conf on the server specifying max-src-conn of 5:
set skip on lo
block return
pass out
pass in on egress proto tcp to port 80 keep state \
(source-track rule, max-src-conn 5)
Run a dumb webserver that prints the pf states on each new
connection, and sleeps for a while before responding to keep the
connection open:
$ doas ruby
require "webrick"
server = WEBrick::HTTPServer.new(:Port => 80)
server.mount_proc "/" do |req, res|
puts "states for #{req.remote_ip}:"
system "pfctl -ss | grep ' #{req.remote_ip}.*ESTABLISHED'"
sleep 30
end
trap "INT" do
server.shutdown
end
server.start
^D
Now from another machine (192.168.1.196 in this case) send 20
requests at once to the server (192.168.1.240):
$ for f in `jot 20`; do ftp -o - http://192.168.1.240/ &; sleep 0.5; done
And on the server, you can see that there are now many more than 5
established states:
states for 192.168.1.196:
all tcp 192.168.1.240:80 <- 192.168.1.196:16727 ESTABLISHED:ESTABLISHED
all tcp 192.168.1.240:80 <- 192.168.1.196:24725 ESTABLISHED:ESTABLISHED
all tcp 192.168.1.240:80 <- 192.168.1.196:2436 ESTABLISHED:ESTABLISHED
all tcp 192.168.1.240:80 <- 192.168.1.196:16529 ESTABLISHED:ESTABLISHED
all tcp 192.168.1.240:80 <- 192.168.1.196:4323 ESTABLISHED:ESTABLISHED
all tcp 192.168.1.240:80 -> 192.168.1.196:45576 ESTABLISHED:ESTABLISHED
all tcp 192.168.1.240:80 -> 192.168.1.196:36693 ESTABLISHED:ESTABLISHED
all tcp 192.168.1.240:80 -> 192.168.1.196:2976 ESTABLISHED:ESTABLISHED
all tcp 192.168.1.240:80 -> 192.168.1.196:9395 ESTABLISHED:ESTABLISHED
all tcp 192.168.1.240:80 -> 192.168.1.196:46616 ESTABLISHED:ESTABLISHED
all tcp 192.168.1.240:80 -> 192.168.1.196:46122 ESTABLISHED:ESTABLISHED
all tcp 192.168.1.240:80 -> 192.168.1.196:22969 ESTABLISHED:ESTABLISHED
all tcp 192.168.1.240:80 -> 192.168.1.196:48742 ESTABLISHED:ESTABLISHED
all tcp 192.168.1.240:80 -> 192.168.1.196:43477 ESTABLISHED:ESTABLISHED
all tcp 192.168.1.240:80 -> 192.168.1.196:29154 ESTABLISHED:ESTABLISHED
all tcp 192.168.1.240:80 -> 192.168.1.196:1876 ESTABLISHED:ESTABLISHED
all tcp 192.168.1.240:80 -> 192.168.1.196:47743 ESTABLISHED:ESTABLISHED
all tcp 192.168.1.240:80 -> 192.168.1.196:43833 ESTABLISHED:ESTABLISHED
all tcp 192.168.1.240:80 -> 192.168.1.196:12074 ESTABLISHED:ESTABLISHED
all tcp 192.168.1.240:80 -> 192.168.1.196:17816 ESTABLISHED:SYN_SENT
Looking at pf.c, the logic in pf_tcp_track_full seems to indicate
that it's accepting the connection (moving it to TCPS_ESTABLISHED),
then checking pf_src_connlimit, but the PF_DROP gets ignored because
the connection is already established.
Shouldn't it not accept the connection if pf_src_connlimit returns
1? This does that:
diff --git sys/net/pf.c sys/net/pf.c
index 8cb1326a160..89703feab12 100644
--- sys/net/pf.c
+++ sys/net/pf.c
@@ -481,12 +481,10 @@ pf_src_connlimit(struct pf_state **stp)
if ((sn = pf_get_src_node((*stp), PF_SN_NONE)) == NULL)
return (0);
- sn->conn++;
- (*stp)->src.tcp_est = 1;
pf_add_threshold(>conn_rate);
if ((*stp)->rule.ptr->max_src_conn &&
- (*stp)->rule.ptr->max_src_conn < sn->conn) {
+ sn->conn >= (*stp)->rule.ptr->max_src_conn) {
pf_status.lcounters[LCNT_SRCCONN]++;
bad++;
}
@@ -497,8 +495,11 @@ pf_src_connlimit(struct pf_state **stp)
bad++;
}
- if (!bad)
+ if (!bad) {
+ sn->conn++;
+ (*stp)->src.tcp_est = 1;
return (0);
+ }
if ((*stp)->rule.ptr->overload_tbl) {
struct pfr_addr p;
@@ -4919,14 +4920,14 @@ pf_tcp_track_full(struct pf_pdesc *pd, struct pf_state
**stp, u_short *reason,
pf_set_protostate(*stp, psrc, TCPS_CLOSING);
if (th->th_flags & TH_ACK) {
if (dst->state == TCPS_SYN_SENT) {
- pf_set_protostate(*stp, pdst,
- TCPS_ESTABLISHED);
- if (src->state == TCPS_ESTABLISHED &&
+ if (src->state == TCPS_SYN_SENT &&
!SLIST_EMPTY(&(*stp)->src_nodes) &&
pf_src_connlimit(stp)) {
REASON_SET(reason, PFRES_SRCLIMIT);
return (PF_DROP);
iked(8): support multiple name servers as client
Hi,
iked currently enforces an arbitrary limit of only a single remote name
server. As we have found out, a good reason to support more than one
is to have a backup when the connection to that server fails for some
reason.
With the diff below we can support all the name servers we get and
fall back to the next one if one connection dies.
ok?
Index: vroute.c
===
RCS file: /cvs/src/sbin/iked/vroute.c,v
retrieving revision 1.17
diff -u -p -r1.17 vroute.c
--- vroute.c18 Jul 2022 19:32:16 - 1.17
+++ vroute.c8 Feb 2023 20:46:13 -
@@ -76,12 +76,14 @@ TAILQ_HEAD(vroute_routes, vroute_route);
struct vroute_dns {
struct sockaddr_storagevd_addr;
int vd_ifidx;
+ TAILQ_ENTRY(vroute_dns) vd_entry;
};
+TAILQ_HEAD(vroute_dnss, vroute_dns);
struct iked_vroute_sc {
struct vroute_addrs ivr_addrs;
struct vroute_routes ivr_routes;
- struct vroute_dns *ivr_dns;
+ struct vroute_dnss ivr_dnss;
struct event ivr_routeev;
int ivr_iosock;
int ivr_iosock6;
@@ -103,6 +105,7 @@ vroute_rtmsg_cb(int fd, short events, vo
{
struct iked *env = (struct iked *) arg;
struct iked_vroute_sc *ivr = env->sc_vroute;
+ struct vroute_dns *dns;
static uint8_t *buf;
struct rt_msghdr*rtm;
ssize_t n;
@@ -133,11 +136,12 @@ vroute_rtmsg_cb(int fd, short events, vo
switch(rtm->rtm_type) {
case RTM_PROPOSAL:
- if (rtm->rtm_priority == RTP_PROPOSAL_SOLICIT &&
- ivr->ivr_dns) {
- log_debug("%s: got solicit", __func__);
- vroute_dodns(env, (struct sockaddr
*)>ivr_dns->vd_addr, 1,
- ivr->ivr_dns->vd_ifidx);
+ if (rtm->rtm_priority == RTP_PROPOSAL_SOLICIT) {
+ TAILQ_FOREACH(dns, >ivr_dnss, vd_entry) {
+ log_debug("%s: got solicit", __func__);
+ vroute_dodns(env, (struct sockaddr *)
>vd_addr,
+ 1, dns->vd_ifidx);
+ }
}
break;
default:
@@ -171,6 +175,7 @@ vroute_init(struct iked *env)
fatal("%s: setsockopt(ROUTE_MSGFILTER)", __func__);
TAILQ_INIT(>ivr_addrs);
+ TAILQ_INIT(>ivr_dnss);
TAILQ_INIT(>ivr_routes);
ivr->ivr_pid = getpid();
@@ -189,6 +194,7 @@ vroute_cleanup(struct iked *env)
struct iked_vroute_sc *ivr = env->sc_vroute;
struct vroute_addr *addr;
struct vroute_route *route;
+ struct vroute_dns *dns;
while ((addr = TAILQ_FIRST(>ivr_addrs))) {
if_indextoname(addr->va_ifidx, ifname);
@@ -209,10 +215,11 @@ vroute_cleanup(struct iked *env)
free(route);
}
- if (ivr->ivr_dns) {
- vroute_dodns(env, (struct sockaddr *)>ivr_dns->vd_addr, 0,
- ivr->ivr_dns->vd_ifidx);
- free(ivr->ivr_dns);
+ while ((dns = TAILQ_FIRST(>ivr_dnss))) {
+ vroute_dodns(env, (struct sockaddr *)>vd_addr, 0,
+ dns->vd_ifidx);
+ TAILQ_REMOVE(>ivr_dnss, dns, vd_entry);
+ free(dns);
}
}
@@ -334,7 +341,6 @@ vroute_setdns(struct iked *env, int add,
int
vroute_getdns(struct iked *env, struct imsg *imsg)
{
- struct iked_vroute_sc *ivr = env->sc_vroute;
struct sockaddr *dns;
uint8_t *ptr;
size_t left;
@@ -361,12 +367,8 @@ vroute_getdns(struct iked *env, struct i
add = (imsg->hdr.type == IMSG_VDNS_ADD);
if (add) {
- if (ivr->ivr_dns != NULL)
- return (0);
vroute_insertdns(env, ifidx, dns);
} else {
- if (ivr->ivr_dns == NULL)
- return (0);
vroute_removedns(env, ifidx, dns);
}
@@ -432,19 +434,23 @@ vroute_insertdns(struct iked *env, int i
memcpy(>vd_addr, addr, addr->sa_len);
dns->vd_ifidx = ifidx;
- ivr->ivr_dns = dns;
+ TAILQ_INSERT_TAIL(>ivr_dnss, dns, vd_entry);
}
void
vroute_removedns(struct iked *env, int ifidx, struct sockaddr *addr)
{
struct iked_vroute_sc *ivr = env->sc_vroute;
+ struct vroute_dns *dns, *tdns;
+
+ TAILQ_FOREACH_SAFE(dns, >ivr_dnss, vd_entry, tdns) {
+ if (ifidx != dns->vd_ifidx)
+ continue;
+ if (sockaddr_cmp(addr, (struct sockaddr *) >vd_addr, -1))
+ continue;
- if (ifidx == ivr->ivr_dns->vd_ifidx &&
- sockaddr_cmp(addr, (struct sockaddr *)
-
omit ksh MAIL* bits in SMALL builds
Anyone checking their mailboxes in the installer's interactive shell?
MAIL If set, the user will be informed of the arrival of mail in
the named file. This parameter is ignored if the MAILPATH
parameter is set.
MAILCHECK ...
MAILPATH ...
Put it behind !SMALL to save some bits; no `grep -r MAIL distrib/' hits.
It could also be a feature like -DVI, but vi mode actually seems reasonable
to flip in custom builds while I can't think of a use case for -DMAIL.
Feedback? Objection? OK?
Index: bin/ksh/main.c
===
RCS file: /cvs/src/bin/ksh/main.c,v
retrieving revision 1.99
diff -u -p -r1.99 main.c
--- bin/ksh/main.c 8 Feb 2023 17:22:10 - 1.99
+++ bin/ksh/main.c 8 Feb 2023 21:14:00 -
@@ -87,7 +87,11 @@ static const char *initcoms [] = {
"typeset", "-x", "SHELL", "PATH", "HOME", "PWD", "OLDPWD", NULL,
"typeset", "-ir", "PPID", NULL,
"typeset", "-i", "OPTIND=1", NULL,
+#ifndef SMALL
"eval", "typeset -i RANDOM MAILCHECK=\"${MAILCHECK-600}\"
SECONDS=\"${SECONDS-0}\" TMOUT=\"${TMOUT-0}\"", NULL,
+#else
+ "eval", "typeset -i RANDOM SECONDS=\"${SECONDS-0}\"
TMOUT=\"${TMOUT-0}\"", NULL,
+#endif /* SMALL */
"alias",
/* Standard ksh aliases */
"hash=alias -t", /* not "alias -t --": hash -r needs to work */
@@ -615,7 +619,9 @@ shell(Source *volatile s, volatile int t
if (interactive) {
got_sigwinch = 1;
j_notify();
+#ifndef SMALL
mcheck();
+#endif /* SMALL */
set_prompt(PS1);
}
Index: bin/ksh/var.c
===
RCS file: /cvs/src/bin/ksh/var.c,v
retrieving revision 1.72
diff -u -p -r1.72 var.c
--- bin/ksh/var.c 5 Mar 2021 15:22:03 - 1.72
+++ bin/ksh/var.c 8 Feb 2023 21:15:21 -
@@ -108,9 +108,11 @@ initvar(void)
{ "HISTSIZE", V_HISTSIZE },
{ "EDITOR", V_EDITOR },
{ "VISUAL", V_VISUAL },
+#ifndef SMALL
{ "MAIL", V_MAIL },
{ "MAILCHECK", V_MAILCHECK },
{ "MAILPATH", V_MAILPATH },
+#endif /* SMALL */
{ "RANDOM", V_RANDOM },
{ "SECONDS",V_SECONDS },
{ "TMOUT", V_TMOUT },
@@ -1029,6 +1031,7 @@ setspec(struct tbl *vp)
x_cols = l;
}
break;
+#ifndef SMALL
case V_MAIL:
mbset(str_val(vp));
break;
@@ -1040,6 +1043,7 @@ setspec(struct tbl *vp)
mcset(intval(vp));
vp->flag |= SPECIAL;
break;
+#endif /* SMALL */
case V_RANDOM:
vp->flag &= ~SPECIAL;
srand_deterministic((unsigned int)intval(vp));
@@ -1099,17 +1103,21 @@ unsetspec(struct tbl *vp)
afree(tmpdir, APERM);
tmpdir = NULL;
break;
+#ifndef SMALL
case V_MAIL:
mbset(NULL);
break;
case V_MAILPATH:
mpset(NULL);
break;
+#endif /* SMALL */
case V_HISTCONTROL:
sethistcontrol(NULL);
break;
case V_LINENO:
+#ifndef SMALL
case V_MAILCHECK: /* at ksh leaves previous value in place */
+#endif /* SMALL */
case V_RANDOM:
case V_SECONDS:
case V_TMOUT: /* at ksh leaves previous value in place */
Index: distrib/special/ksh/Makefile
===
RCS file: /cvs/src/distrib/special/ksh/Makefile,v
retrieving revision 1.6
diff -u -p -r1.6 Makefile
--- distrib/special/ksh/Makefile8 Feb 2023 17:22:10 - 1.6
+++ distrib/special/ksh/Makefile8 Feb 2023 20:41:00 -
@@ -2,7 +2,7 @@
PROG= ksh
SRCS= alloc.c c_ksh.c c_sh.c c_test.c c_ulimit.c edit.c emacs.c eval.c \
- exec.c expr.c history.c io.c jobs.c lex.c mail.c main.c \
+ exec.c expr.c history.c io.c jobs.c lex.c main.c \
misc.c path.c shf.c syn.c table.c trap.c tree.c tty.c var.c \
vi.c
Re: omit ksh version bits in SMALL builds
Sure
Klemens Nanni wrote:
> Everytime I check ramdisks I wonder of what use the ksh string is:
> $ what bsd.rd
> bsd.rd:
> OpenBSD 7.2-current (RAMDISK_CD) #965: Sun Feb 5 09:58:01 MST
> 2023
> PD KSH v5.2.14 99/07/13.2
> $OpenBSD: cert.pem,v 1.25 2022/07/11 09:05:16 sthen Exp $
>
> ksh(1) says
> KSH_VERSION
> The version of the shell and the date the version was created
> (read-only).
>
> The installer does not care about SH_VERSION or KSH_VERSION or \V and \v PS1
> escape sequences for full and short version strings.
>
> Disable it with SMALL to save a few bits and get
> $ what obj/bsd.rd
> obj/bsd.rd:
> OpenBSD 7.2-current (RAMDISK_CD) #5: Wed Feb 8 04:10:37 CET
> 2023
> $OpenBSD: cert.pem,v 1.25 2022/07/11 09:05:16 sthen Exp $
>
>
> Feedback? Objection? OK?
>
>
> Index: bin/ksh/lex.c
> ===
> RCS file: /cvs/src/bin/ksh/lex.c,v
> retrieving revision 1.78
> diff -u -p -r1.78 lex.c
> --- bin/ksh/lex.c 15 Jan 2018 14:58:05 - 1.78
> +++ bin/ksh/lex.c 8 Feb 2023 02:47:26 -
> @@ -1335,6 +1335,7 @@ dopprompt(const char *sp, int ntruncate,
> case 'u': /* '\' 'u' username */
> strlcpy(strbuf, username, sizeof strbuf);
> break;
> +#ifndef SMALL
> case 'v': /* '\' 'v' version (short) */
> p = strchr(ksh_version, ' ');
> if (p)
> @@ -1350,6 +1351,7 @@ dopprompt(const char *sp, int ntruncate,
> case 'V': /* '\' 'V' version (long) */
> strlcpy(strbuf, ksh_version, sizeof strbuf);
> break;
> +#endif /* SMALL */
> case 'w': /* '\' 'w' cwd */
> p = str_val(global("PWD"));
> n = strlen(str_val(global("HOME")));
> Index: bin/ksh/main.c
> ===
> RCS file: /cvs/src/bin/ksh/main.c,v
> retrieving revision 1.98
> diff -u -p -r1.98 main.c
> --- bin/ksh/main.c28 Jun 2019 13:34:59 - 1.98
> +++ bin/ksh/main.c8 Feb 2023 02:53:49 -
> @@ -81,7 +81,9 @@ static const char initifs[] = "IFS= \t\n
> static const char initsubs[] = "${PS2=> } ${PS3=#? } ${PS4=+ }";
>
> static const char *initcoms [] = {
> +#ifndef SMALL
> "typeset", "-r", "KSH_VERSION", NULL,
> +#endif /* SMALL */
> "typeset", "-x", "SHELL", "PATH", "HOME", "PWD", "OLDPWD", NULL,
> "typeset", "-ir", "PPID", NULL,
> "typeset", "-i", "OPTIND=1", NULL,
> @@ -110,7 +112,9 @@ static const char *initcoms [] = {
>
> char username[_PW_NAME_LEN + 1];
>
> +#ifndef SMALL
> #define version_param (initcoms[2])
> +#endif /* SMALL */
>
> /* The shell uses its own variation on argv, to build variables like
> * $0 and $@.
> @@ -247,7 +251,9 @@ main(int argc, char *argv[])
> (strlen(kshname) >= 3 &&
> !strcmp([strlen(kshname) - 3], "/sh"))) {
> Flag(FSH) = 1;
> +#ifndef SMALL
> version_param = "SH_VERSION";
> +#endif /* SMALL */
> }
>
> /* Set edit mode to emacs by default, may be overridden
> @@ -296,8 +302,10 @@ main(int argc, char *argv[])
> }
> ppid = getppid();
> setint(global("PPID"), (int64_t) ppid);
> +#ifndef SMALL
> /* setstr can't fail here */
> setstr(global(version_param), ksh_version, KSH_RETURN_ERROR);
> +#endif /* SMALL */
>
> /* execute initialization statements */
> for (wp = (char**) initcoms; *wp != NULL; wp++) {
> Index: distrib/special/ksh/Makefile
> ===
> RCS file: /cvs/src/distrib/special/ksh/Makefile,v
> retrieving revision 1.5
> diff -u -p -r1.5 Makefile
> --- distrib/special/ksh/Makefile 1 Aug 2017 14:30:07 - 1.5
> +++ distrib/special/ksh/Makefile 8 Feb 2023 02:45:49 -
> @@ -4,7 +4,7 @@ PROG= ksh
> SRCS=alloc.c c_ksh.c c_sh.c c_test.c c_ulimit.c edit.c emacs.c
> eval.c \
> exec.c expr.c history.c io.c jobs.c lex.c mail.c main.c \
> misc.c path.c shf.c syn.c table.c trap.c tree.c tty.c var.c \
> - version.c vi.c
> + vi.c
>
> DEFS=-Wall -DEMACS -DSMALL
> CFLAGS+=${DEFS} -I. -I${.CURDIR}/../../../bin/ksh
> -I${.CURDIR}/../../../lib/libc/gen
>
Re: wsmouse(4): Apple-like multi-touch buttons
On 23-02-07 02:12PM, Tobias Heider wrote: > On Mon, Sep 19, 2022 at 11:16:51AM +0200, Ulf Brosziewski wrote: > > Is there enough interest in this feature among OpenBSD users? I haven't > > seen many requests for it, if any. Moreover, is it a good idea to configure > > different input methods on this or that hardware just because another OS > > has different defaults? > > > > Just in case the answer to these questions turns out to be "yes", here are > > some remarks on the diff. > > I do still believe that there is interest in this feature based on the > feedback > I got from other devs. Having it available as a non-default option as > kettenis@ > said would be good enough. > Below is a revised version of the diff that adds a new mouse.tp.mtbuttons > config > option. It can either be enabled via wsconsctl mouse.tp.mtbuttons=1 or by > adding mouse.tp.mtbuttons=1 to your /etc/wsconsctl.conf. > I'm definitely interested and would like to see this option made available. Thanks for working on it, tobhe! I'd be happy to test any further diffs too -- Mark Jamsek GPG: F2FF 13DE 6A06 C471 CA80 E6E2 2930 DC66 86EE CF68
Re: Unlock select(2) and pselect(2)
On Wed, Feb 08, 2023 at 12:40:50PM +0100, Mark Kettenis wrote:
> > Date: Wed, 8 Feb 2023 14:17:14 +0300
> > From: Vitaliy Makkoveev
> >
> > On Tue, Feb 07, 2023 at 05:42:40PM +0300, Vitaliy Makkoveev wrote:
> > >
> > > > > Otherwise, if you are concerning about serialized `p_sigmask' and
> > > > > P_SIGSUSPEND, dosigsuspend() should be left under kernel lock:
> > > > >
> > > > > if (sigmask) {
> > > > > KERNEL_LOCK();
> > > > > dosigsuspend(p, *sigmask &~ sigcantmask);
> > > > > KERNEL_UNLOCK();
> > > > > }
> > > >
> > > > This is a hack but would allow progress. Now I would prefer if
> > > > dosigsuspend() is made safe so that the KERNEL_LOCK is not needed. Which
> > > > would also allow sigsuspend to be called with NOLOCK.
> > > >
> > >
> > > Makes sense, but sigsuspend(2) will not be unlocked together with
> > > select(2), so dosigsuspend() could be left under kernel lock for a
> > > while.
> > >
> >
> > Now, I want to keep dosigsuspend() under the kernel lock. To me it's
> > better to unlock it together with sigsuspend(2) to separate possible
> > signal related fallout. This still makes sense because signal mask is
> > the optional arg for pselect(2) only. Except the local event data
> > conversion, poll(2) internals are the same as select(2) internals, it
> > makes sense to unlock them both.
> >
> > This doesn't mean I dropped dosigsuspend(), I'm still waiting feedback
> > for barriers.
>
> Splitting it this way makes sense to me.
I agree. Will throw that onto my bgpd test box that will test the poll
case.
--
:wq Claudio
Re: Unlock select(2) and pselect(2)
> Date: Wed, 8 Feb 2023 14:17:14 +0300
> From: Vitaliy Makkoveev
>
> On Tue, Feb 07, 2023 at 05:42:40PM +0300, Vitaliy Makkoveev wrote:
> >
> > > > Otherwise, if you are concerning about serialized `p_sigmask' and
> > > > P_SIGSUSPEND, dosigsuspend() should be left under kernel lock:
> > > >
> > > > if (sigmask) {
> > > > KERNEL_LOCK();
> > > > dosigsuspend(p, *sigmask &~ sigcantmask);
> > > > KERNEL_UNLOCK();
> > > > }
> > >
> > > This is a hack but would allow progress. Now I would prefer if
> > > dosigsuspend() is made safe so that the KERNEL_LOCK is not needed. Which
> > > would also allow sigsuspend to be called with NOLOCK.
> > >
> >
> > Makes sense, but sigsuspend(2) will not be unlocked together with
> > select(2), so dosigsuspend() could be left under kernel lock for a
> > while.
> >
>
> Now, I want to keep dosigsuspend() under the kernel lock. To me it's
> better to unlock it together with sigsuspend(2) to separate possible
> signal related fallout. This still makes sense because signal mask is
> the optional arg for pselect(2) only. Except the local event data
> conversion, poll(2) internals are the same as select(2) internals, it
> makes sense to unlock them both.
>
> This doesn't mean I dropped dosigsuspend(), I'm still waiting feedback
> for barriers.
Splitting it this way makes sense to me.
> Index: sys/kern/sys_generic.c
> ===
> RCS file: /cvs/src/sys/kern/sys_generic.c,v
> retrieving revision 1.151
> diff -u -p -r1.151 sys_generic.c
> --- sys/kern/sys_generic.c27 Dec 2022 20:13:03 - 1.151
> +++ sys/kern/sys_generic.c8 Feb 2023 10:56:08 -
> @@ -596,15 +596,16 @@ dopselect(struct proc *p, int nd, fd_set
> struct timespec zerots = {};
> fd_mask bits[6];
> fd_set *pibits[3], *pobits[3];
> - int error, ncollected = 0, nevents = 0;
> + int error, nfiles, ncollected = 0, nevents = 0;
> u_int ni;
>
> if (nd < 0)
> return (EINVAL);
> - if (nd > p->p_fd->fd_nfiles) {
> - /* forgiving; slightly wrong */
> - nd = p->p_fd->fd_nfiles;
> - }
> +
> + nfiles = READ_ONCE(p->p_fd->fd_nfiles);
> + if (nd > nfiles)
> + nd = nfiles;
> +
> ni = howmany(nd, NFDBITS) * sizeof(fd_mask);
> if (ni > sizeof(bits[0])) {
> caddr_t mbits;
> @@ -643,8 +644,11 @@ dopselect(struct proc *p, int nd, fd_set
> }
> #endif
>
> - if (sigmask)
> + if (sigmask) {
> + KERNEL_LOCK();
> dosigsuspend(p, *sigmask &~ sigcantmask);
> + KERNEL_UNLOCK();
> + }
>
> /* Register kqueue events */
> error = pselregister(p, pibits, pobits, nd, , );
> @@ -948,8 +952,11 @@ doppoll(struct proc *p, struct pollfd *f
> if ((error = copyin(fds, pl, sz)) != 0)
> goto bad;
>
> - if (sigmask)
> + if (sigmask) {
> + KERNEL_LOCK();
> dosigsuspend(p, *sigmask &~ sigcantmask);
> + KERNEL_UNLOCK();
> + }
>
> /* Register kqueue events */
> ppollregister(p, pl, nfds, , );
> Index: sys/kern/syscalls.master
> ===
> RCS file: /cvs/src/sys/kern/syscalls.master,v
> retrieving revision 1.239
> diff -u -p -r1.239 syscalls.master
> --- sys/kern/syscalls.master 7 Jan 2023 05:24:58 - 1.239
> +++ sys/kern/syscalls.master 8 Feb 2023 10:56:10 -
> @@ -165,7 +165,7 @@
> struct itimerval *oitv); }
> 70 STD NOLOCK { int sys_getitimer(int which, \
> struct itimerval *itv); }
> -71 STD { int sys_select(int nd, fd_set *in, fd_set *ou, \
> +71 STD NOLOCK { int sys_select(int nd, fd_set *in, fd_set *ou, \
> fd_set *ex, struct timeval *tv); }
> 72 STD NOLOCK { int sys_kevent(int fd, \
> const struct kevent *changelist, int nchanges, \
> @@ -230,10 +230,10 @@
> u_int flags, int atflags); }
> 108 STD NOLOCK { int sys_pledge(const char *promises, \
> const char *execpromises); }
> -109 STD { int sys_ppoll(struct pollfd *fds, \
> +109 STD NOLOCK { int sys_ppoll(struct pollfd *fds, \
> u_int nfds, const struct timespec *ts, \
> const sigset_t *mask); }
> -110 STD { int sys_pselect(int nd, fd_set *in, fd_set *ou, \
> +110 STD NOLOCK { int sys_pselect(int nd, fd_set *in, fd_set *ou, \
> fd_set *ex, const struct timespec *ts, \
> const sigset_t *mask); }
> 111 STD { int sys_sigsuspend(int mask); }
> @@ -448,7 +448,7 @@
> 250 STD NOLOCK { int sys_minherit(void *addr, size_t len, \
> int
Re: Unlock select(2) and pselect(2)
On Tue, Feb 07, 2023 at 05:42:40PM +0300, Vitaliy Makkoveev wrote:
>
> > > Otherwise, if you are concerning about serialized `p_sigmask' and
> > > P_SIGSUSPEND, dosigsuspend() should be left under kernel lock:
> > >
> > > if (sigmask) {
> > > KERNEL_LOCK();
> > > dosigsuspend(p, *sigmask &~ sigcantmask);
> > > KERNEL_UNLOCK();
> > > }
> >
> > This is a hack but would allow progress. Now I would prefer if
> > dosigsuspend() is made safe so that the KERNEL_LOCK is not needed. Which
> > would also allow sigsuspend to be called with NOLOCK.
> >
>
> Makes sense, but sigsuspend(2) will not be unlocked together with
> select(2), so dosigsuspend() could be left under kernel lock for a
> while.
>
Now, I want to keep dosigsuspend() under the kernel lock. To me it's
better to unlock it together with sigsuspend(2) to separate possible
signal related fallout. This still makes sense because signal mask is
the optional arg for pselect(2) only. Except the local event data
conversion, poll(2) internals are the same as select(2) internals, it
makes sense to unlock them both.
This doesn't mean I dropped dosigsuspend(), I'm still waiting feedback
for barriers.
Index: sys/kern/sys_generic.c
===
RCS file: /cvs/src/sys/kern/sys_generic.c,v
retrieving revision 1.151
diff -u -p -r1.151 sys_generic.c
--- sys/kern/sys_generic.c 27 Dec 2022 20:13:03 - 1.151
+++ sys/kern/sys_generic.c 8 Feb 2023 10:56:08 -
@@ -596,15 +596,16 @@ dopselect(struct proc *p, int nd, fd_set
struct timespec zerots = {};
fd_mask bits[6];
fd_set *pibits[3], *pobits[3];
- int error, ncollected = 0, nevents = 0;
+ int error, nfiles, ncollected = 0, nevents = 0;
u_int ni;
if (nd < 0)
return (EINVAL);
- if (nd > p->p_fd->fd_nfiles) {
- /* forgiving; slightly wrong */
- nd = p->p_fd->fd_nfiles;
- }
+
+ nfiles = READ_ONCE(p->p_fd->fd_nfiles);
+ if (nd > nfiles)
+ nd = nfiles;
+
ni = howmany(nd, NFDBITS) * sizeof(fd_mask);
if (ni > sizeof(bits[0])) {
caddr_t mbits;
@@ -643,8 +644,11 @@ dopselect(struct proc *p, int nd, fd_set
}
#endif
- if (sigmask)
+ if (sigmask) {
+ KERNEL_LOCK();
dosigsuspend(p, *sigmask &~ sigcantmask);
+ KERNEL_UNLOCK();
+ }
/* Register kqueue events */
error = pselregister(p, pibits, pobits, nd, , );
@@ -948,8 +952,11 @@ doppoll(struct proc *p, struct pollfd *f
if ((error = copyin(fds, pl, sz)) != 0)
goto bad;
- if (sigmask)
+ if (sigmask) {
+ KERNEL_LOCK();
dosigsuspend(p, *sigmask &~ sigcantmask);
+ KERNEL_UNLOCK();
+ }
/* Register kqueue events */
ppollregister(p, pl, nfds, , );
Index: sys/kern/syscalls.master
===
RCS file: /cvs/src/sys/kern/syscalls.master,v
retrieving revision 1.239
diff -u -p -r1.239 syscalls.master
--- sys/kern/syscalls.master7 Jan 2023 05:24:58 - 1.239
+++ sys/kern/syscalls.master8 Feb 2023 10:56:10 -
@@ -165,7 +165,7 @@
struct itimerval *oitv); }
70 STD NOLOCK { int sys_getitimer(int which, \
struct itimerval *itv); }
-71 STD { int sys_select(int nd, fd_set *in, fd_set *ou, \
+71 STD NOLOCK { int sys_select(int nd, fd_set *in, fd_set *ou, \
fd_set *ex, struct timeval *tv); }
72 STD NOLOCK { int sys_kevent(int fd, \
const struct kevent *changelist, int nchanges, \
@@ -230,10 +230,10 @@
u_int flags, int atflags); }
108STD NOLOCK { int sys_pledge(const char *promises, \
const char *execpromises); }
-109STD { int sys_ppoll(struct pollfd *fds, \
+109STD NOLOCK { int sys_ppoll(struct pollfd *fds, \
u_int nfds, const struct timespec *ts, \
const sigset_t *mask); }
-110STD { int sys_pselect(int nd, fd_set *in, fd_set *ou, \
+110STD NOLOCK { int sys_pselect(int nd, fd_set *in, fd_set *ou, \
fd_set *ex, const struct timespec *ts, \
const sigset_t *mask); }
111STD { int sys_sigsuspend(int mask); }
@@ -448,7 +448,7 @@
250STD NOLOCK { int sys_minherit(void *addr, size_t len, \
int inherit); }
251OBSOL rfork
-252STD { int sys_poll(struct pollfd *fds, \
+252STD NOLOCK { int sys_poll(struct pollfd *fds, \
u_int nfds, int timeout); }
253STD NOLOCK { int
ANSI function definitions in dump and fsck_ffs
People have made passes at using ANSI-style function definitions in the
past. Some functions were missed and this upsets clang 15.
Index: dump/traverse.c
===
RCS file: /cvs/src/sbin/dump/traverse.c,v
retrieving revision 1.39
diff -u -p -r1.39 traverse.c
--- dump/traverse.c 26 Apr 2018 17:40:48 - 1.39
+++ dump/traverse.c 4 Feb 2023 20:49:04 -
@@ -717,10 +717,7 @@ ufs2_blksout(daddr_t *blkp, int frags, i
* Dump a map to the tape.
*/
void
-dumpmap(map, type, ino)
- char *map;
- int type;
- ino_t ino;
+dumpmap(char *map, int type, ino_t ino)
{
int i;
char *cp;
@@ -736,8 +733,7 @@ dumpmap(map, type, ino)
* Write a header record to the dump tape.
*/
void
-writeheader(ino)
- ino_t ino;
+writeheader(ino_t ino)
{
int32_t sum, cnt, *lp;
Index: fsck_ffs/dir.c
===
RCS file: /cvs/src/sbin/fsck_ffs/dir.c,v
retrieving revision 1.32
diff -u -p -r1.32 dir.c
--- fsck_ffs/dir.c 20 Jan 2015 18:22:21 - 1.32
+++ fsck_ffs/dir.c 4 Feb 2023 20:49:52 -
@@ -439,10 +439,7 @@ linkup(ino_t orphan, ino_t parentdir)
* fix an entry in a directory.
*/
int
-changeino(dir, name, newnum)
- ino_t dir;
- char *name;
- ino_t newnum;
+changeino(ino_t dir, char *name, ino_t newnum)
{
struct inodesc idesc;
