On Tue, Jul 02, 2013 at 02:20:05PM +0200, Jan Klemkow wrote: > Your change is working. I've test this Bug with following case. > > FIRST: > router-a: cat /etc/hostname.carp0 > 10.0.0.1 vhid 1 carpnodes 1:0,2:100 pass "aaa" balancing ip > router-b: cat /etc/hostname.carp0 > 10.0.0.1 vhid 1 carpnodes 1:0,2:100 pass "bbb" balancing ip > > THAN: > router-a: cat /etc/hostname.carp0 > 10.0.0.1 vhid 1 carpnodes 1:0,2:100 pass "a" balancing ip > router-b: cat /etc/hostname.carp0 > 10.0.0.1 vhid 1 carpnodes 1:0,2:100 pass "a" balancing ip > > After this I get an "carp0: incorrect hash". With our diffs, it is > fixed. But, I would prefer my diff, cause your Diff don't fix the > length problem. In my opinion it is a serious Bug, that password parts > over 20 character are ignored. Even for the Hash-MAC code in carp, it > looks not very secure, if we make shifting with a lot of zeros in cases > of an small password.
Thanks for testing. > I would prefer the hashing version. I don't. If you want to get a secret into the kernel, ifconfig should just store it into the kernel. Your hash would obfuscate the algorithm and brake compatibility. What could be done is to implement a hexkey like in wpakey. This would allow to use the whole 20 byte information of the passphrase. Although I don't think that it is worth it. bluhm > > bye, > Jan > > On Tue, Jul 02, 2013 at 01:04:49PM +0200, Alexander Bluhm wrote: > > On Tue, Jul 02, 2013 at 12:27:54PM +0200, Jan Klemkow wrote: > > > Hi, > > > > > > This diff implements the hashing of the carp password before using it > > > inside of the Kernel. It fix the problem that passwords like > > > "12345678901234567890" and "12345678901234567890XXX" are equal for carp. > > > But It breaks the compatibility with older Versions. Maybe you need to > > > increase the protocol number? > > > > > > bluhm@ have an other idea to solve this problem: ifconfig could XOR > > > every 20 Byte long Chuck of the Passwort. This would not break the > > > compatibility of setups with less than 20 char password. > > > > I would not do anyhing about that as it breaks compatibility without > > gain. Perhaps the XXX comment should be removed. > > > > What is actually wrong is that long passwords cannot be replaced > > completely with short passwords. ioctl(SIOCGVH) fills the carpr_key > > with the old value. strlcpy() overwrites only the beginning of the > > key. > > > > Jan, can you please test this? > > > > ok? > > > > bluhm > > > > Index: sbin/ifconfig/ifconfig.c > > =================================================================== > > RCS file: /data/mirror/openbsd/cvs/src/sbin/ifconfig/ifconfig.c,v > > retrieving revision 1.264 > > diff -u -p -u -p -r1.264 ifconfig.c > > --- sbin/ifconfig/ifconfig.c 31 May 2013 19:56:06 -0000 1.264 > > +++ sbin/ifconfig/ifconfig.c 2 Jul 2013 11:01:38 -0000 > > @@ -3390,7 +3390,7 @@ setcarp_passwd(const char *val, int d) > > if (ioctl(s, SIOCGVH, (caddr_t)&ifr) == -1) > > err(1, "SIOCGVH"); > > > > - /* XXX Should hash the password into the key here, perhaps? */ > > + bzero(carpr.carpr_key, CARP_KEY_LEN); > > strlcpy((char *)carpr.carpr_key, val, CARP_KEY_LEN); > > > > if (ioctl(s, SIOCSVH, (caddr_t)&ifr) == -1)
