Re: ipsec: document locks of some structures

2021-07-18 Thread Vitaliy Makkoveev
ping?

The diff below updated to the most recent source.

Index: sys/netinet/ip_ipsp.h
===
RCS file: /cvs/src/sys/netinet/ip_ipsp.h,v
retrieving revision 1.203
diff -u -p -r1.203 ip_ipsp.h
--- sys/netinet/ip_ipsp.h   18 Jul 2021 18:19:22 -  1.203
+++ sys/netinet/ip_ipsp.h   18 Jul 2021 22:19:28 -
@@ -45,6 +45,12 @@
 #include 
 #include 
 
+/*
+ * Locks used to protect struct members in this file:
+ * I   Immutable after creation
+ * N   netlock
+ */
+
 union sockaddr_union {
struct sockaddr sa;
struct sockaddr_in  sin;
@@ -226,37 +232,38 @@ struct ipsec_id {
 };
 
 struct ipsec_ids {
-   LIST_ENTRY(ipsec_ids)   id_gc_list;
-   RBT_ENTRY(ipsec_ids)id_node_id;
-   RBT_ENTRY(ipsec_ids)id_node_flow;
-   struct ipsec_id *id_local;
-   struct ipsec_id *id_remote;
-   u_int32_t   id_flow;
-   int id_refcount;
-   u_int   id_gc_ttl;
+   LIST_ENTRY(ipsec_ids)   id_gc_list; /* [N] */
+   RBT_ENTRY(ipsec_ids)id_node_id; /* [N] */
+   RBT_ENTRY(ipsec_ids)id_node_flow;   /* [N] */
+   struct ipsec_id *id_local;  /* [I] */
+   struct ipsec_id *id_remote; /* [I] */
+   u_int32_t   id_flow;/* [I] */
+   int id_refcount;/* [N] */
+   u_int   id_gc_ttl;  /* [N] */
 };
 RBT_HEAD(ipsec_ids_flows, ipsec_ids);
 RBT_HEAD(ipsec_ids_tree, ipsec_ids);
 
 struct ipsec_acquire {
-   union sockaddr_unionipa_addr;
-   u_int32_t   ipa_seq;
-   struct sockaddr_encap   ipa_info;
-   struct sockaddr_encap   ipa_mask;
+   union sockaddr_unionipa_addr;   /* [I] */
+   u_int32_t   ipa_seq;/* [I] */
+   struct sockaddr_encap   ipa_info;   /* [I] */
+   struct sockaddr_encap   ipa_mask;   /* [I] */
struct timeout  ipa_timeout;
-   struct ipsec_policy *ipa_policy;
-   struct inpcb*ipa_pcb;
-   TAILQ_ENTRY(ipsec_acquire)  ipa_ipo_next;
-   TAILQ_ENTRY(ipsec_acquire)  ipa_next;
+   struct ipsec_policy *ipa_policy;/* [I] */
+   struct inpcb*ipa_pcb;   /* [I] */
+   TAILQ_ENTRY(ipsec_acquire)  ipa_ipo_next;   /* [N] */
+   TAILQ_ENTRY(ipsec_acquire)  ipa_next;   /* [N] */
 };
 
 struct ipsec_policy {
struct radix_node   ipo_nodes[2];   /* radix tree glue */
-   struct sockaddr_encap   ipo_addr;
-   struct sockaddr_encap   ipo_mask;
+   struct sockaddr_encap   ipo_addr;   /* [I] */
+   struct sockaddr_encap   ipo_mask;   /* [I] */
 
-   union sockaddr_unionipo_src;/* Local address to use */
-   union sockaddr_unionipo_dst;/* Remote gateway -- if it's 
zeroed:
+   union sockaddr_unionipo_src;/* [N] Local address to use */
+   union sockaddr_unionipo_dst;/* [N] Remote gateway --
+* if it's zeroed:
 * - on output, we try to
 * contact the remote host
 * directly (if needed).
@@ -267,22 +274,28 @@ struct ipsec_policy {
 * mode was used.
 */
 
-   u_int64_t   ipo_last_searched;  /* Timestamp of last 
lookup */
-
-   u_int8_tipo_flags;  /* See IPSP_POLICY_* 
definitions */
-   u_int8_tipo_type;   /* USE/ACQUIRE/... */
-   u_int8_tipo_sproto; /* ESP/AH; if zero, use system 
dflts */
-   u_int   ipo_rdomain;
-
-   int ipo_ref_count;
-
-   struct tdb  *ipo_tdb;   /* Cached entry */
-
-   struct ipsec_ids*ipo_ids;
+   u_int64_t   ipo_last_searched;  /* [N] Timestamp
+  of last lookup */
 
-   TAILQ_HEAD(ipo_acquires_head, ipsec_acquire) ipo_acquires; /* List of 
acquires */
-   TAILQ_ENTRY(ipsec_policy)   ipo_tdb_next;   /* List TDB policies */
-   TAILQ_ENTRY(ipsec_policy)   ipo_list;   /* List of all policies 
*/
+   u_int8_tipo_flags;  /* [N] See IPSP_POLICY_*
+  definitions */
+   u_int8_tipo_type;   /* [N] USE/ACQUIRE/... */
+   u_int8_tipo_sproto; /* [N] ESP/AH; if zero,
+  use system df

Re: dhcpleased/resolved: omit ISP DNS in DHCP offer

2021-07-18 Thread Sebastien Marie
On Sun, Jul 18, 2021 at 07:10:06PM +0200, stolen data wrote:
> After reading the man pages of the replacements for dhclient(8) it looks
> like it will no longer be possible to prevent DNS entries in a DHCP offer
> from ending up in the resolv.conf - as is practically always the case with
> a residential Internet connection with DHCP. I don't want my ISP's DNS
> servers *anywhere* in my resolv.conf, so I tell dhclient(8) to SUPERSEDE
> DOMAIN-NAME-SERVERS, but now it looks like OpenBSD will no longer allow me
> the choice and instead decides for me that DNS entries in DHCPOFFER must
> always be included and made available for eventual use. Why? This is taking
> control out of the hands of users and I cannot see any sense in the
> rationale.

you don't want /etc/resolv.conf be controlled by dhcp ? just disable
resolvd daemon, and /etc/resolv.conf is your.

$ doas rcctl stop resolvd
$ doas rcctl disable resolvd

alternatively, if you are using unwind(8), resolvd(8) will
automatically use unwind nameserver instead of dhcp/slaacd ones.

Regards.
-- 
Sebastien Marie



Re: dhcpleased/resolved: omit ISP DNS in DHCP offer

2021-07-18 Thread Theo de Raadt
stolen data  wrote:

> After reading the man pages of the replacements for dhclient(8) it looks
> like it will no longer be possible to prevent DNS entries in a DHCP offer
> from ending up in the resolv.conf - as is practically always the case with
> a residential Internet connection with DHCP. I don't want my ISP's DNS
> servers *anywhere* in my resolv.conf, so I tell dhclient(8) to SUPERSEDE
> DOMAIN-NAME-SERVERS, but now it looks like OpenBSD will no longer allow me
> the choice and instead decides for me that DNS entries in DHCPOFFER must
> always be included and made available for eventual use. Why? This is taking
> control out of the hands of users and I cannot see any sense in the
> rationale.

What you want is trivial, using unwind(8) and unwind.conf(8)

You comments ooze of drama and noone is appreciating it.  If you don't
like what we are doing to our operating system for ourselves, either
stop the insulting tone or run some other operating system.

Sadly, you have have control over how things work unless you write your
own complete operating system, or maintain large diffs to another system
on a continual basis.  You are only a whining user -- every single
operating system group will treat you exactly the same.  Either you get
used to dissapointment, or you adapt and your use of a PREVIOUS KNOBSET
("dhclient + supersede") to a NEW KNOBSET ("dhcpleased + resolvd + unwind +
unwind.conf with/without forwarder or preference + operating on 127.0.0.1)
which we firmly believe is a better solution for the average case.

Additionally, unwind performs opportunistic DNSSEC and other preventative
validations.  What is not to like?

At this point you should be incredibly gracious you even get a reply which
provides alternative ways of doing things.



dhcpleased/resolved: omit ISP DNS in DHCP offer

2021-07-18 Thread stolen data
After reading the man pages of the replacements for dhclient(8) it looks
like it will no longer be possible to prevent DNS entries in a DHCP offer
from ending up in the resolv.conf - as is practically always the case with
a residential Internet connection with DHCP. I don't want my ISP's DNS
servers *anywhere* in my resolv.conf, so I tell dhclient(8) to SUPERSEDE
DOMAIN-NAME-SERVERS, but now it looks like OpenBSD will no longer allow me
the choice and instead decides for me that DNS entries in DHCPOFFER must
always be included and made available for eventual use. Why? This is taking
control out of the hands of users and I cannot see any sense in the
rationale.


Re: dhcpleased: default route with classless static routes option

2021-07-18 Thread Bjorn Ketelaars
On Sun 18/07/2021 10:38, Florian Obser wrote:
> On 2021-07-18 01:02 +02, Bjorn Ketelaars  wrote:
> > On Sat 17/07/2021 17:12, Florian Obser wrote:
> >> 
> >> 
> >> On 17 July 2021 13:16:59 CEST, Bjorn Ketelaars  wrote:
> >> >An inconsistency exists between dhclient(8) and dhcpleased(8) when
> >> >receiving the Classless Static Routes option: dhcpleased creates a
> >> >default route, while dhclient does not.
> >> >
> >> >If I'm not mistaken, the behaviour of dhclient is correct. From
> >> >rfc3442:
> >> >"If the DHCP server returns both a Classless Static Routes option and a
> >> >Router option, the DHCP client MUST ignore the Router option."
> >> >
> >> 
> >> Correct. But you are fixing it in the wrong place. It doesn't say to 
> >> ignore a default route, which can be transmitted via classless static 
> >> routes option (it's 0/0).
> >
> > That makes sense. New diff...
> 
> Not quite. This assumes that the router option comes first, a DHCP
> server could send us something like this:
> 
> DHO_ROUTERS
> DHO_CLASSLESS_STATIC_ROUTES
> DHO_ROUTERS
> DHO_CLASSLESS_STATIC_ROUTES
> 
> Which would be quite broken, but hey...
> 
> How about this? Do you actually have a test case for this?

I have a simple test case: DHCP server from ISP (more specific IPTV
platform from KPN), which sends a single router option followed by a
classless static router option. With your diff the router option is
ignored.

This fixes my issue, thank you!



Re: dhcpleased: default route with classless static routes option

2021-07-18 Thread Florian Obser
On 2021-07-18 01:02 +02, Bjorn Ketelaars  wrote:
> On Sat 17/07/2021 17:12, Florian Obser wrote:
>> 
>> 
>> On 17 July 2021 13:16:59 CEST, Bjorn Ketelaars  wrote:
>> >An inconsistency exists between dhclient(8) and dhcpleased(8) when
>> >receiving the Classless Static Routes option: dhcpleased creates a
>> >default route, while dhclient does not.
>> >
>> >If I'm not mistaken, the behaviour of dhclient is correct. From
>> >rfc3442:
>> >"If the DHCP server returns both a Classless Static Routes option and a
>> >Router option, the DHCP client MUST ignore the Router option."
>> >
>> 
>> Correct. But you are fixing it in the wrong place. It doesn't say to ignore 
>> a default route, which can be transmitted via classless static routes option 
>> (it's 0/0).
>
> That makes sense. New diff...

Not quite. This assumes that the router option comes first, a DHCP
server could send us something like this:

DHO_ROUTERS
DHO_CLASSLESS_STATIC_ROUTES
DHO_ROUTERS
DHO_CLASSLESS_STATIC_ROUTES

Which would be quite broken, but hey...

How about this? Do you actually have a test case for this?

dhcpd(8) correctly only hands me a routers option or classless static
routes option, never both.

diff --git engine.c engine.c
index ad8202c9a33..700d7c5ae80 100644
--- engine.c
+++ engine.c
@@ -613,7 +613,7 @@ parse_dhcp(struct dhcpleased_iface *iface, struct imsg_dhcp 
*dhcp)
uint32_t rebinding_time = 0;
uint8_t *p, dho = DHO_PAD, dho_len;
uint8_t  dhcp_message_type = 0;
-   int  routes_len = 0;
+   int  routes_len = 0, routers = 0, csr = 0;
char from[sizeof("xx:xx:xx:xx:xx:xx")];
char to[sizeof("xx:xx:xx:xx:xx:xx")];
char hbuf_src[INET_ADDRSTRLEN];
@@ -836,19 +836,28 @@ parse_dhcp(struct dhcpleased_iface *iface, struct 
imsg_dhcp *dhcp)
if (dho_len % sizeof(routes[routes_len].gw) != 0)
goto wrong_length;
 
-   while (routes_len < MAX_DHCP_ROUTES && dho_len > 0) {
-   memcpy(&routes[routes_len].gw, p,
-   sizeof(routes[routes_len].gw));
-   if (log_getverbose() > 1) {
-   log_debug("DHO_ROUTER: %s",
-   inet_ntop(AF_INET,
-   &routes[routes_len].gw, hbuf,
-   sizeof(hbuf)));
+   /*
+* Ignore routers option if classless static routes
+* are present (RFC3442).
+*/
+   if (!csr) {
+   routers = 1;
+   while (routes_len < MAX_DHCP_ROUTES &&
+   dho_len > 0) {
+   memcpy(&routes[routes_len].gw, p,
+   sizeof(routes[routes_len].gw));
+   if (log_getverbose() > 1) {
+   log_debug("DHO_ROUTER: %s",
+   inet_ntop(AF_INET,
+   &routes[routes_len].gw,
+   hbuf, sizeof(hbuf)));
+   }
+   p += sizeof(routes[routes_len].gw);
+   rem -= sizeof(routes[routes_len].gw);
+   dho_len -=
+   sizeof(routes[routes_len].gw);
+   routes_len++;
}
-   p += sizeof(routes[routes_len].gw);
-   rem -= sizeof(routes[routes_len].gw);
-   dho_len -= sizeof(routes[routes_len].gw);
-   routes_len++;
}
if (dho_len != 0) {
/* ignore > MAX_DHCP_ROUTES routes */
@@ -939,6 +948,15 @@ parse_dhcp(struct dhcpleased_iface *iface, struct 
imsg_dhcp *dhcp)
case DHO_CLASSLESS_STATIC_ROUTES: {
int prefixlen, compressed_prefixlen;
 
+   csr = 1;
+   if (routers) {
+   /*
+* Ignore routers option if classless static
+* routes are present (RFC3442).
+*/
+   routers = 0;
+   routes_len = 0;
+   }
while (routes_len < MAX_DHCP_ROUTES && dho_len > 0