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.

I would prefer the hashing version.

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)

Reply via email to