Re: remove net.inet6.ip6.soiikey from userland

2023-05-21 Thread Klemens Nanni
On Sat, May 20, 2023 at 07:47:46PM +0200, Florian Obser wrote:
> On 2023-05-20 19:37 +02, Paul de Weerd  wrote:
> > On Sat, May 20, 2023 at 05:33:11PM +0200, Florian Obser wrote:
> > | In case this turns out to be useful for unlocking work in the kernel.
> > | 
> > | It's a minimum diff, if we want to go this way we probably want to move
> > | init_soiikey() to the engine process and stop bouncing through the main
> > | process when an interface changes.
> > | 
> > | This changes behaviour: in -current we can change the sysctl and down/up
> > | an interface to read the new value, with this diff that no longer
> > | works. slaacd will (and can) only read the file on startup.

Having to restart slaacd when changing soii.key at runtime seems fine to me,
it's a seed and not really a config file, anyway.

> > | 
> > | This has consequences for the installer: slaacd starts before
> > | /mnt/etc/soii.key is available in the upgrade case. Which in turn means
> > | that we get a different IPv6 address in the installer than in the
> > | running system. I don't know how big of an issue that is.
> >
> > Can't speak for others, but my use case for soii-addresses is for
> > incoming connections - outgoing ones should use the temporary privacy
> > addressed.  So for the installer it doesn't matter.
> >
> > My guess is that this goes for many (most? all?) users of
> > soii-addresses, so it should be safe to not have those in the
> > installer during upgrades.
> 
> I'm worried that someone runs a completely locked down network.
> They installed a server, figured out the random but stable IP address
> and added it to their firewall and only allow communication from known
> IP addresses.
> 
> They will be fine if they use sysupgrade(8). The installed base is
> probably smaller than 6.

Lots of time still until release to find out for real.



Re: remove net.inet6.ip6.soiikey from userland

2023-05-20 Thread Florian Obser
On 2023-05-20 19:37 +02, Paul de Weerd  wrote:
> On Sat, May 20, 2023 at 05:33:11PM +0200, Florian Obser wrote:
> | In case this turns out to be useful for unlocking work in the kernel.
> | 
> | It's a minimum diff, if we want to go this way we probably want to move
> | init_soiikey() to the engine process and stop bouncing through the main
> | process when an interface changes.
> | 
> | This changes behaviour: in -current we can change the sysctl and down/up
> | an interface to read the new value, with this diff that no longer
> | works. slaacd will (and can) only read the file on startup.
> | 
> | This has consequences for the installer: slaacd starts before
> | /mnt/etc/soii.key is available in the upgrade case. Which in turn means
> | that we get a different IPv6 address in the installer than in the
> | running system. I don't know how big of an issue that is.
>
> Can't speak for others, but my use case for soii-addresses is for
> incoming connections - outgoing ones should use the temporary privacy
> addressed.  So for the installer it doesn't matter.
>
> My guess is that this goes for many (most? all?) users of
> soii-addresses, so it should be safe to not have those in the
> installer during upgrades.

I'm worried that someone runs a completely locked down network.
They installed a server, figured out the random but stable IP address
and added it to their firewall and only allow communication from known
IP addresses.

They will be fine if they use sysupgrade(8). The installed base is
probably smaller than 6.

>
> Paul
>

-- 
In my defence, I have been left unsupervised.



Re: remove net.inet6.ip6.soiikey from userland

2023-05-20 Thread Paul de Weerd
On Sat, May 20, 2023 at 05:33:11PM +0200, Florian Obser wrote:
| In case this turns out to be useful for unlocking work in the kernel.
| 
| It's a minimum diff, if we want to go this way we probably want to move
| init_soiikey() to the engine process and stop bouncing through the main
| process when an interface changes.
| 
| This changes behaviour: in -current we can change the sysctl and down/up
| an interface to read the new value, with this diff that no longer
| works. slaacd will (and can) only read the file on startup.
| 
| This has consequences for the installer: slaacd starts before
| /mnt/etc/soii.key is available in the upgrade case. Which in turn means
| that we get a different IPv6 address in the installer than in the
| running system. I don't know how big of an issue that is.

Can't speak for others, but my use case for soii-addresses is for
incoming connections - outgoing ones should use the temporary privacy
addressed.  So for the installer it doesn't matter.

My guess is that this goes for many (most? all?) users of
soii-addresses, so it should be safe to not have those in the
installer during upgrades.

Paul

| diff --git distrib/miniroot/install.sub distrib/miniroot/install.sub
| index d3d944bf2ca..aa84e4808f2 100644
| --- distrib/miniroot/install.sub
| +++ distrib/miniroot/install.sub
| @@ -2620,9 +2620,6 @@ enable_network() {
|   echo "127.0.0.1\tlocalhost" >/tmp/i/hosts
|   echo "::1\t\tlocalhost" >>/tmp/i/hosts
|  
| - _f=/mnt/etc/soii.key
| - [[ -f $_f ]] && sysctl "net.inet6.ip6.soiikey=$(<$_f)"
| -
|   enable_ifs
|  }
|  
| diff --git distrib/special/sysctl/sysctl.c distrib/special/sysctl/sysctl.c
| index 9156d5f455c..7aedb6dd55b 100644
| --- distrib/special/sysctl/sysctl.c
| +++ distrib/special/sysctl/sysctl.c
| @@ -99,39 +99,6 @@ pstring(struct var *v)
|   return (1);
|  }
|  
| -int
| -parse_hex_char(char ch)
| -{
| - if (ch >= '0' && ch <= '9')
| - return (ch - '0');
| -
| - ch = tolower((unsigned char)ch);
| - if (ch >= 'a' && ch <= 'f')
| - return (ch - 'a' + 10);
| -
| - return (-1);
| -}
| -
| -int
| -set_soii_key(char *src)
| -{
| - uint8_t key[SOIIKEY_LEN];
| - int mib[4] = {CTL_NET, PF_INET6, IPPROTO_IPV6, IPV6CTL_SOIIKEY};
| - int i, c;
| -
| - for(i = 0; i < SOIIKEY_LEN; i++) {
| - if ((c = parse_hex_char(src[2 * i])) == -1)
| - return (-1);
| - key[i] = c << 4;
| - if ((c = parse_hex_char(src[2 * i + 1])) == -1)
| - return (-1);
| - key[i] |= c;
| - }
| -
| - return sysctl(mib, sizeof(mib) / sizeof(mib[0]), NULL, NULL, key,
| - SOIIKEY_LEN);
| -}
| -
|  int
|  main(int argc, char *argv[])
|  {
| @@ -159,17 +126,6 @@ main(int argc, char *argv[])
|  
|   while (argc--) {
|   name = *argv++;
| - /*
| -  * strlen("net.inet6.ip6.soiikey="
| -  * "") == 54
| -  * strlen("net.inet6.ip6.soiikey=") == 22
| -  */
| - if (strlen(name) == 54 && strncmp(name,
| - "net.inet6.ip6.soiikey=", 22) == 0) {
| - set_soii_key(name + 22);
| - continue;
| - }
| -
|   for (i = 0; i < sizeof(vars)/sizeof(vars[0]); i++) {
|   if (strcmp(name, vars[i].name) == 0) {
|   (vars[i].print)(&vars[i]);
| diff --git etc/netstart etc/netstart
| index af4866f909e..105d5a977cf 100644
| --- etc/netstart
| +++ etc/netstart
| @@ -360,13 +360,6 @@ if ifconfig lo0 inet6 >/dev/null 2>&1; then
|   IP6KERNEL=true
|  fi
|  
| -# Load key material for the generation of IPv6 Semantically Opaque Interface
| -# Identifiers (SOII) used for SLAAC addresses.
| -if $IP6KERNEL && ! $PRINT_ONLY; then
| - [[ -f /etc/soii.key ]] &&
| - sysctl -q "net.inet6.ip6.soiikey=$( /etc/soii.key &&
| - chmod 600 /etc/soii.key && sysctl -q \
| - "net.inet6.ip6.soiikey=$(
|  #include 
|  #include 
| +#include 
|  #include 
| -#include 
|  #include 
|  #include 
|  
| @@ -34,6 +34,7 @@
|  #include 
|  #include 
|  
| +#include 
|  #include 
|  #include 
|  #include 
| @@ -76,7 +77,8 @@ voidconfigure_gateway(struct imsg_configure_dfr *, 
uint8_t);
|  void add_gateway(struct imsg_configure_dfr *);
|  void delete_gateway(struct imsg_configure_dfr *);
|  void send_rdns_proposal(struct imsg_propose_rdns *);
| -int  get_soiikey(uint8_t *);
| +int  parse_hex_char(char);
| +void init_soiikey(void);
|  
|  static int   main_imsg_send_ipc_sockets(struct imsgbuf *, struct imsgbuf *);
|  int  main_imsg_compose_frontend(int, int, void *, uint16_t);
| @@ -89,6 +91,7 @@ pid_tfrontend_pid;
|  pid_t engine_pid;
|  
|  int   routesock, ioctl_sock, rtm_seq = 0;
| +uint8_t   soiikey[

remove net.inet6.ip6.soiikey from userland

2023-05-20 Thread Florian Obser
In case this turns out to be useful for unlocking work in the kernel.

It's a minimum diff, if we want to go this way we probably want to move
init_soiikey() to the engine process and stop bouncing through the main
process when an interface changes.

This changes behaviour: in -current we can change the sysctl and down/up
an interface to read the new value, with this diff that no longer
works. slaacd will (and can) only read the file on startup.

This has consequences for the installer: slaacd starts before
/mnt/etc/soii.key is available in the upgrade case. Which in turn means
that we get a different IPv6 address in the installer than in the
running system. I don't know how big of an issue that is.

diff --git distrib/miniroot/install.sub distrib/miniroot/install.sub
index d3d944bf2ca..aa84e4808f2 100644
--- distrib/miniroot/install.sub
+++ distrib/miniroot/install.sub
@@ -2620,9 +2620,6 @@ enable_network() {
echo "127.0.0.1\tlocalhost" >/tmp/i/hosts
echo "::1\t\tlocalhost" >>/tmp/i/hosts
 
-   _f=/mnt/etc/soii.key
-   [[ -f $_f ]] && sysctl "net.inet6.ip6.soiikey=$(<$_f)"
-
enable_ifs
 }
 
diff --git distrib/special/sysctl/sysctl.c distrib/special/sysctl/sysctl.c
index 9156d5f455c..7aedb6dd55b 100644
--- distrib/special/sysctl/sysctl.c
+++ distrib/special/sysctl/sysctl.c
@@ -99,39 +99,6 @@ pstring(struct var *v)
return (1);
 }
 
-int
-parse_hex_char(char ch)
-{
-   if (ch >= '0' && ch <= '9')
-   return (ch - '0');
-
-   ch = tolower((unsigned char)ch);
-   if (ch >= 'a' && ch <= 'f')
-   return (ch - 'a' + 10);
-
-   return (-1);
-}
-
-int
-set_soii_key(char *src)
-{
-   uint8_t key[SOIIKEY_LEN];
-   int mib[4] = {CTL_NET, PF_INET6, IPPROTO_IPV6, IPV6CTL_SOIIKEY};
-   int i, c;
-
-   for(i = 0; i < SOIIKEY_LEN; i++) {
-   if ((c = parse_hex_char(src[2 * i])) == -1)
-   return (-1);
-   key[i] = c << 4;
-   if ((c = parse_hex_char(src[2 * i + 1])) == -1)
-   return (-1);
-   key[i] |= c;
-   }
-
-   return sysctl(mib, sizeof(mib) / sizeof(mib[0]), NULL, NULL, key,
-   SOIIKEY_LEN);
-}
-
 int
 main(int argc, char *argv[])
 {
@@ -159,17 +126,6 @@ main(int argc, char *argv[])
 
while (argc--) {
name = *argv++;
-   /*
-* strlen("net.inet6.ip6.soiikey="
-* "") == 54
-* strlen("net.inet6.ip6.soiikey=") == 22
-*/
-   if (strlen(name) == 54 && strncmp(name,
-   "net.inet6.ip6.soiikey=", 22) == 0) {
-   set_soii_key(name + 22);
-   continue;
-   }
-
for (i = 0; i < sizeof(vars)/sizeof(vars[0]); i++) {
if (strcmp(name, vars[i].name) == 0) {
(vars[i].print)(&vars[i]);
diff --git etc/netstart etc/netstart
index af4866f909e..105d5a977cf 100644
--- etc/netstart
+++ etc/netstart
@@ -360,13 +360,6 @@ if ifconfig lo0 inet6 >/dev/null 2>&1; then
IP6KERNEL=true
 fi
 
-# Load key material for the generation of IPv6 Semantically Opaque Interface
-# Identifiers (SOII) used for SLAAC addresses.
-if $IP6KERNEL && ! $PRINT_ONLY; then
-   [[ -f /etc/soii.key ]] &&
-   sysctl -q "net.inet6.ip6.soiikey=$( /etc/soii.key &&
-   chmod 600 /etc/soii.key && sysctl -q \
-   "net.inet6.ip6.soiikey=$(
 #include 
 #include 
+#include 
 #include 
-#include 
 #include 
 #include 
 
@@ -34,6 +34,7 @@
 #include 
 #include 
 
+#include 
 #include 
 #include 
 #include 
@@ -76,7 +77,8 @@ void  configure_gateway(struct imsg_configure_dfr *, uint8_t);
 void   add_gateway(struct imsg_configure_dfr *);
 void   delete_gateway(struct imsg_configure_dfr *);
 void   send_rdns_proposal(struct imsg_propose_rdns *);
-intget_soiikey(uint8_t *);
+intparse_hex_char(char);
+void   init_soiikey(void);
 
 static int main_imsg_send_ipc_sockets(struct imsgbuf *, struct imsgbuf *);
 intmain_imsg_compose_frontend(int, int, void *, uint16_t);
@@ -89,6 +91,7 @@ pid_t  frontend_pid;
 pid_t   engine_pid;
 
 int routesock, ioctl_sock, rtm_seq = 0;
+uint8_t soiikey[SLAACD_SOIIKEY_LEN];
 
 void
 main_sig_handler(int sig, short event, void *arg)
@@ -189,6 +192,10 @@ main(int argc, char *argv[])
if (getpwnam(SLAACD_USER) == NULL)
errx(1, "unknown user %s", SLAACD_USER);
 
+#ifndef SMALL
+   init_soiikey();
+#endif /* SMALL */
+
log_init(debug, LOG_DAEMON);
log_setverbose(verbose);
 
@@ -431,11 +438,9 @@ main_dispatch_frontend(int fd, short event, void *bula)
fatalx("%s: IMSG_UPDATE_IF wrong length: %lu",
__func__, IMSG_DATA_SIZE(imsg)