[Drbd-dev] Only 63 characters maximum allowed for shared secret (and other string values)

[Drbd-dev] Only 63 characters maximum allowed for shared secret (and other string values)

[Tijs Van Buggenhout]

Hi,

In the online users-guide manual (and man 8 drbdsetup) I can read the 
following for shared-secret keyword: "The shared secret used in peer 
authentication. May be up to 64 characters."

This seems to be inaccurate, as only 63 characters can be defined as valid 
value for the keyword, otherwise an error is raised. 64 bytes is the buffer 
size for the value of the keyword (drbd/linux/drbd.h:#define SHARED_SECRET_MAX 
64) but it needs to be null terminated, hence one character is lost..

(master.domain.be):~# drbdsetup connect drbd0 172.16.1.179 172.16.1.180 --
protocol C --cram-hmac-alg sha1 --shared-secret 
'012345678901234567890123456789012345678901234567890123456789012'
(master.domain.be):~# drbdsetup disconnect 172.16.1.179 172.16.1.180
(master.domain.be):~# drbdsetup connect drbd0 172.16.1.179 172.16.1.180 --
protocol C --cram-hmac-alg sha1 --shared-secret 
'0123456789012345678901234567890123456789012345678901234567890123'
drbd0: Failure: (126) UnknownMandatoryTag
additional info from kernel:
invalid attribute value
(master.domain.be):~# drbdsetup connect drbd0 172.16.1.179 172.16.1.180 --
protocol C --cram-hmac-alg sha1 --shared-secret 
'01234567890123456789012345678901234567890123456789012345678901234'
drbd0: Failure: (126) UnknownMandatoryTag
additional info from kernel:
invalid attribute value

A bit confusing though, as...

struct net_conf is defined in drbd/linux/drbd_genl.h

GENL_struct(DRBD_NLA_NET_CONF, 5, net_conf,
    __str_field_def(1,DRBD_GENLA_F_MANDATORY | DRBD_F_SENSITIVE,                                    
                    shared_secret,SHARED_SECRET_MAX)

but __str_field_def as defined in drbd/linux/genl_magic_struct.h reads :

#define __str_field_def(attr_nr, attr_flag, name, maxlen) \
        __str_field(attr_nr, attr_flag, name, maxlen)

which would make one believe SHARED_SECRET_MAX is actually the maximum length 
allowed for shared secret (SHARED_SECRET_MAX correspons with maxlen parameter 
of __str_field_def macro).

Also in the same file, __str_field macro is defined as:

#define __str_field(attr_nr, attr_flag, name, maxlen) \
        __array(attr_nr, attr_flag, name, NLA_NUL_STRING, char, maxlen, \
                        nla_strlcpy, nla_put, false)

where NLA_NUL_STRING is introduced as nla type for the field, meaning..

user/libgenl.h: *    NLA_NUL_STRING       Maximum length of string (excluding 
NUL)

.. which again implies that SHARED_SECRET_MAX would be the maximum length.

However in drbd/linux/genl_magic_struct.h the __array macro is defined as:

#define __array(attr_nr, attr_flag, name, nla_type, _type, maxlen,      \
                __get, __put, __is_signed)                              \
        [attr_nr] = { .type = nla_type,                                 \
                      .len = maxlen - (nla_type == NLA_NUL_STRING) },

the (max) length for the value of the field is decreased to (maxlen - 1) when 
nla_type equals NLA_NUL_STRING.

Also..

drbd/drbd_receiver.c:   char secret[SHARED_SECRET_MAX]; /* 64 byte */
drbd/drbd_receiver.c:   key_len = strlen(nc->shared_secret);
drbd/drbd_receiver.c:   memcpy(secret, nc->shared_secret, key_len);

and 

drbd/drbd_nl.c: ((char *)new_conf->shared_secret)[SHARED_SECRET_MAX-1] = 0;

require shared_secret to be null terminated, and max 63 bytes in length... 
Other fields like cram_hmac_alg, integrity_alg, verify_alg and csums_alg use 
the same boundary.

Did I misinterprete the manual? What is the intended behaviour?

Best regards,
Tijs

[Drbd-dev] Only 63 characters maximum allowed for shared secret (and other string values)

[Tijs Van Buggenhout]

Hi,

In the online users-guide manual (and man 8 drbdsetup) I can read the 
following for shared-secret keyword: "The shared secret used in peer 
authentication. May be up to 64 characters."

This seems to be inaccurate, as only 63 characters can be defined as valid 
value for the keyword, otherwise an error is raised. 64 bytes is the buffer 
size for the value of the keyword (drbd/linux/drbd.h:#define SHARED_SECRET_MAX 
64) but it needs to be null terminated, hence one character is lost..

(master.domain.be):~# drbdsetup connect drbd0 172.16.1.179 172.16.1.180 --
protocol C --cram-hmac-alg sha1 --shared-secret 
'012345678901234567890123456789012345678901234567890123456789012'
(master.domain.be):~# drbdsetup disconnect 172.16.1.179 172.16.1.180
(master.domain.be):~# drbdsetup connect drbd0 172.16.1.179 172.16.1.180 --
protocol C --cram-hmac-alg sha1 --shared-secret 
'0123456789012345678901234567890123456789012345678901234567890123'
drbd0: Failure: (126) UnknownMandatoryTag
additional info from kernel:
invalid attribute value
(master.domain.be):~# drbdsetup connect drbd0 172.16.1.179 172.16.1.180 --
protocol C --cram-hmac-alg sha1 --shared-secret 
'01234567890123456789012345678901234567890123456789012345678901234'
drbd0: Failure: (126) UnknownMandatoryTag
additional info from kernel:
invalid attribute value

A bit confusing though, as...

struct net_conf is defined in drbd/linux/drbd_genl.h

GENL_struct(DRBD_NLA_NET_CONF, 5, net_conf,
    __str_field_def(1,DRBD_GENLA_F_MANDATORY | DRBD_F_SENSITIVE,                                    
                    shared_secret,SHARED_SECRET_MAX)

but __str_field_def as defined in drbd/linux/genl_magic_struct.h reads :

#define __str_field_def(attr_nr, attr_flag, name, maxlen) \
        __str_field(attr_nr, attr_flag, name, maxlen)

which would make one believe SHARED_SECRET_MAX is actually the maximum length 
allowed for shared secret (SHARED_SECRET_MAX correspons with maxlen parameter 
of __str_field_def macro).

Also in the same file, __str_field macro is defined as:

#define __str_field(attr_nr, attr_flag, name, maxlen) \
        __array(attr_nr, attr_flag, name, NLA_NUL_STRING, char, maxlen, \
                        nla_strlcpy, nla_put, false)

where NLA_NUL_STRING is introduced as nla type for the field, meaning..

user/libgenl.h: *    NLA_NUL_STRING       Maximum length of string (excluding 
NUL)

.. which again implies that SHARED_SECRET_MAX would be the maximum length.

However in drbd/linux/genl_magic_struct.h the __array macro is defined as:

#define __array(attr_nr, attr_flag, name, nla_type, _type, maxlen,      \
                __get, __put, __is_signed)                              \
        [attr_nr] = { .type = nla_type,                                 \
                      .len = maxlen - (nla_type == NLA_NUL_STRING) },

the (max) length for the value of the field is decreased to (maxlen - 1) when 
nla_type equals NLA_NUL_STRING.

Also..

drbd/drbd_receiver.c:   char secret[SHARED_SECRET_MAX]; /* 64 byte */
drbd/drbd_receiver.c:   key_len = strlen(nc->shared_secret);
drbd/drbd_receiver.c:   memcpy(secret, nc->shared_secret, key_len);

and 

drbd/drbd_nl.c: ((char *)new_conf->shared_secret)[SHARED_SECRET_MAX-1] = 0;

require shared_secret to be null terminated, and max 63 bytes in length... 
Other fields like cram_hmac_alg, integrity_alg, verify_alg and csums_alg use 
the same boundary.

Did I misinterprete the manual? What is the intended behaviour?

Best regards,
Tijs

Re: [Drbd-dev] Only 63 characters maximum allowed for shared secret (and other string values)

[Lars Ellenberg]

On Tue, Mar 12, 2013 at 11:29:10AM +0100, Tijs Van Buggenhout wrote:
> Hi,
> 
> In the online users-guide manual (and man 8 drbdsetup) I can read the 
> following for shared-secret keyword: "The shared secret used in peer 
> authentication. May be up to 64 characters."
> 
> This seems to be inaccurate, as only 63 characters can be defined as valid 
> value for the keyword, otherwise an error is raised. 64 bytes is the buffer 
> size for the value of the keyword (drbd/linux/drbd.h:#define SHARED_SECRET_MAX 
> 64) but it needs to be null terminated, hence one character is lost..

Right.  So the documentation is off by one.

> which would make one believe SHARED_SECRET_MAX is actually the maximum length 
> allowed for shared secret (SHARED_SECRET_MAX correspons with maxlen parameter 
> of __str_field_def macro).

Maximum payload including terminating NUL.

> Also in the same file, __str_field macro is defined as:
> where NLA_NUL_STRING is introduced as nla type for the field, meaning..
> 
> user/libgenl.h: *    NLA_NUL_STRING       Maximum length of string (excluding 
> NUL)

exactly, but what is described there is the .len member of the policy struct.

> #define __array(attr_nr, attr_flag, name, nla_type, _type, maxlen,      \
>                 __get, __put, __is_signed)                              \
>         [attr_nr] = { .type = nla_type,                                 \
>                       .len = maxlen - (nla_type == NLA_NUL_STRING) },
> 
> the (max) length for the value of the field is decreased to (maxlen - 1) when 
> nla_type equals NLA_NUL_STRING.

Yep, to make the value of the .len attribute of the policy struct match,
so validate_nla() will validate it to be <= that *including* the
terminating NUL.

> Did I misinterprete the manual? What is the intended behaviour?

See linux kernel source tree,
lib/nlattr.c, validate_nla, case NLA_NUL_STRING.

It validates the payload of that nla to contain a terminating NUL,
and contain that within the first pt->len + 1 byte
in case the attrlen should happen to be larger, even,
possibly due to padding.

That "+ 1" is why there is the "- (nla_type == NLA_NUL_STRING)"
in our macro.


-- 
: Lars Ellenberg
: LINBIT | Your Way to High Availability
: DRBD/HA support and consulting http://www.linbit.com

DRBD® and LINBIT® are registered trademarks of LINBIT, Austria.