Re: ldap(1) add SAFE-INIT-CHAR
On 11/6/18 8:59 AM, Claudio Jeker wrote:
> On Tue, Nov 06, 2018 at 08:21:57AM +0100, Martijn van Duren wrote:
>> ping
>>
>> On 10/24/18 10:27 AM, Martijn van Duren wrote:
>>> In my previous ldap mail I proclaimed that we should encode whitespace.
>>> Reading rfc2849 a bit further, encoding a string with leading space is
>>> mandatory by SAFE-INIT-CHAR. This is needed because of the definition
>>> of value-spec, which allows additional space, colon, and less-than
>>> after the colon separating the AttributeDescription.
>>>
>>> The code below adds these definitions. I also changed the outlen
>>> calculation because it at least fails b64_ntop on inlen == 1.
>>>
>>> OK?
>
> See inline.
>
>>> martijn@
>>>
>>> Index: ldapclient.c
>>> ===
>>> RCS file: /cvs/src/usr.bin/ldap/ldapclient.c,v
>>> retrieving revision 1.5
>>> diff -u -p -r1.5 ldapclient.c
>>> --- ldapclient.c23 Oct 2018 08:28:34 - 1.5
>>> +++ ldapclient.c24 Oct 2018 08:21:27 -
>>> @@ -404,8 +404,13 @@ ldapc_printattr(struct ldapc *ldap, cons
>>> * in SAFE-STRINGs. String value that do not match the
>>> * criteria must be encoded as Base64.
>>> */
>>> - for (cp = (const unsigned char *)value;
>>> - encode == 0 &&*cp != '\0'; cp++) {
>>> + cp = (const unsigned char *)value;
>>> + /* !SAFE-INIT-CHAR: SAFE-CHAR minus %x20 %x3A %x3C */
>>> + if (*cp == ' ' ||
>>> + *cp == ':' ||
>>> + *cp == '<')
>>> + encode = 1;
>>> + for (; encode == 0 &&*cp != '\0'; cp++) {
> ^ missing space
>
>>> /* !SAFE-CHAR %x01-09 / %x0B-0C / %x0E-7F */
>>> if (*cp > 127 ||
>>> *cp == '\0' ||
>
> The *cp == '\0' check here is strange since that is part of the for loop
> termination check.
See my other diff. The check for '\0' in the for-loop is incorrect,
since a attribute is transferred as an OCTETSTRING, but isn't required
to be an LDAPSTRING, AKA an UTF-8 string. (RFC4511 4.1.5)
>
>>> @@ -421,7 +426,7 @@ ldapc_printattr(struct ldapc *ldap, cons
>>> }
>>> } else {
>>> inlen = strlen(value);
>>> - outlen = inlen * 2 + 1;
>>> + outlen = (((inlen + 2) / 3) * 4) + 1;
>
> I'm surprised that there is no macro to calculate the length of a base64
> string. Anyway math is right.
>
>>>
>>> if ((out = calloc(1, outlen)) == NULL ||
>>> b64_ntop(value, inlen, out, outlen) == -1) {
>>>
>>
>
> OK claudio
>
Re: ldap(1) add SAFE-INIT-CHAR
On Tue, Nov 06, 2018 at 08:21:57AM +0100, Martijn van Duren wrote:
> ping
>
> On 10/24/18 10:27 AM, Martijn van Duren wrote:
> > In my previous ldap mail I proclaimed that we should encode whitespace.
> > Reading rfc2849 a bit further, encoding a string with leading space is
> > mandatory by SAFE-INIT-CHAR. This is needed because of the definition
> > of value-spec, which allows additional space, colon, and less-than
> > after the colon separating the AttributeDescription.
> >
> > The code below adds these definitions. I also changed the outlen
> > calculation because it at least fails b64_ntop on inlen == 1.
> >
> > OK?
See inline.
> > martijn@
> >
> > Index: ldapclient.c
> > ===
> > RCS file: /cvs/src/usr.bin/ldap/ldapclient.c,v
> > retrieving revision 1.5
> > diff -u -p -r1.5 ldapclient.c
> > --- ldapclient.c23 Oct 2018 08:28:34 - 1.5
> > +++ ldapclient.c24 Oct 2018 08:21:27 -
> > @@ -404,8 +404,13 @@ ldapc_printattr(struct ldapc *ldap, cons
> > * in SAFE-STRINGs. String value that do not match the
> > * criteria must be encoded as Base64.
> > */
> > - for (cp = (const unsigned char *)value;
> > - encode == 0 &&*cp != '\0'; cp++) {
> > + cp = (const unsigned char *)value;
> > + /* !SAFE-INIT-CHAR: SAFE-CHAR minus %x20 %x3A %x3C */
> > + if (*cp == ' ' ||
> > + *cp == ':' ||
> > + *cp == '<')
> > + encode = 1;
> > + for (; encode == 0 &&*cp != '\0'; cp++) {
^ missing space
> > /* !SAFE-CHAR %x01-09 / %x0B-0C / %x0E-7F */
> > if (*cp > 127 ||
> > *cp == '\0' ||
The *cp == '\0' check here is strange since that is part of the for loop
termination check.
> > @@ -421,7 +426,7 @@ ldapc_printattr(struct ldapc *ldap, cons
> > }
> > } else {
> > inlen = strlen(value);
> > - outlen = inlen * 2 + 1;
> > + outlen = (((inlen + 2) / 3) * 4) + 1;
I'm surprised that there is no macro to calculate the length of a base64
string. Anyway math is right.
> >
> > if ((out = calloc(1, outlen)) == NULL ||
> > b64_ntop(value, inlen, out, outlen) == -1) {
> >
>
OK claudio
--
:wq Claudio
Re: ldap(1) add SAFE-INIT-CHAR
ping
On 10/24/18 10:27 AM, Martijn van Duren wrote:
> In my previous ldap mail I proclaimed that we should encode whitespace.
> Reading rfc2849 a bit further, encoding a string with leading space is
> mandatory by SAFE-INIT-CHAR. This is needed because of the definition
> of value-spec, which allows additional space, colon, and less-than
> after the colon separating the AttributeDescription.
>
> The code below adds these definitions. I also changed the outlen
> calculation because it at least fails b64_ntop on inlen == 1.
>
> OK?
>
> martijn@
>
> Index: ldapclient.c
> ===
> RCS file: /cvs/src/usr.bin/ldap/ldapclient.c,v
> retrieving revision 1.5
> diff -u -p -r1.5 ldapclient.c
> --- ldapclient.c 23 Oct 2018 08:28:34 - 1.5
> +++ ldapclient.c 24 Oct 2018 08:21:27 -
> @@ -404,8 +404,13 @@ ldapc_printattr(struct ldapc *ldap, cons
>* in SAFE-STRINGs. String value that do not match the
>* criteria must be encoded as Base64.
>*/
> - for (cp = (const unsigned char *)value;
> - encode == 0 &&*cp != '\0'; cp++) {
> + cp = (const unsigned char *)value;
> + /* !SAFE-INIT-CHAR: SAFE-CHAR minus %x20 %x3A %x3C */
> + if (*cp == ' ' ||
> + *cp == ':' ||
> + *cp == '<')
> + encode = 1;
> + for (; encode == 0 &&*cp != '\0'; cp++) {
> /* !SAFE-CHAR %x01-09 / %x0B-0C / %x0E-7F */
> if (*cp > 127 ||
> *cp == '\0' ||
> @@ -421,7 +426,7 @@ ldapc_printattr(struct ldapc *ldap, cons
> }
> } else {
> inlen = strlen(value);
> - outlen = inlen * 2 + 1;
> + outlen = (((inlen + 2) / 3) * 4) + 1;
>
> if ((out = calloc(1, outlen)) == NULL ||
> b64_ntop(value, inlen, out, outlen) == -1) {
>
ldap(1) add SAFE-INIT-CHAR
In my previous ldap mail I proclaimed that we should encode whitespace.
Reading rfc2849 a bit further, encoding a string with leading space is
mandatory by SAFE-INIT-CHAR. This is needed because of the definition
of value-spec, which allows additional space, colon, and less-than
after the colon separating the AttributeDescription.
The code below adds these definitions. I also changed the outlen
calculation because it at least fails b64_ntop on inlen == 1.
OK?
martijn@
Index: ldapclient.c
===
RCS file: /cvs/src/usr.bin/ldap/ldapclient.c,v
retrieving revision 1.5
diff -u -p -r1.5 ldapclient.c
--- ldapclient.c23 Oct 2018 08:28:34 - 1.5
+++ ldapclient.c24 Oct 2018 08:21:27 -
@@ -404,8 +404,13 @@ ldapc_printattr(struct ldapc *ldap, cons
* in SAFE-STRINGs. String value that do not match the
* criteria must be encoded as Base64.
*/
- for (cp = (const unsigned char *)value;
- encode == 0 &&*cp != '\0'; cp++) {
+ cp = (const unsigned char *)value;
+ /* !SAFE-INIT-CHAR: SAFE-CHAR minus %x20 %x3A %x3C */
+ if (*cp == ' ' ||
+ *cp == ':' ||
+ *cp == '<')
+ encode = 1;
+ for (; encode == 0 &&*cp != '\0'; cp++) {
/* !SAFE-CHAR %x01-09 / %x0B-0C / %x0E-7F */
if (*cp > 127 ||
*cp == '\0' ||
@@ -421,7 +426,7 @@ ldapc_printattr(struct ldapc *ldap, cons
}
} else {
inlen = strlen(value);
- outlen = inlen * 2 + 1;
+ outlen = (((inlen + 2) / 3) * 4) + 1;
if ((out = calloc(1, outlen)) == NULL ||
b64_ntop(value, inlen, out, outlen) == -1) {
