ping
On 10/23/18 9:50 AM, Martijn van Duren wrote:
> The following diff is a half decent solution to the problems and
> (together with my earlier patches) makes our output (minus the comments)
> identical to ldapsearch output (for my particular usecase).
>
> Another issue I addressed in this patch is that a base64 encoded value
> must be prepended with an additional colon according to rfc2849:
> value-spec = ":" ( FILL 0*1(SAFE-STRING) /
> ":" FILL (BASE64-STRING) /
> "<" FILL url)
> ; See notes 7 and 8, below
>
> The aldap return strings where I'm certain that the OCTETSTRINGS must
> be an LDAPString I've retained the utoa. Functions that aren't used
> in ldap(1) I've transformed because it was easier for now and should
> be revisited when backporting to ypldap.
>
> And although the SAFE-CHARS are defined as
> SAFE-CHAR %x01-09 / %x0B-0C / %x0E-7F
> I think it would be wise to follow ldapsearch's lead, where they also
> encode non-printable and strings containing only whitespace. This way
> the output is more human readable when the output is just a whitespace
> and we don't have the risk of sending escape-sequences to the console.
>
> Thoughts on both the diff and b64 encoding some of the SAFE-STRINGs?
>
> martijn@
>
> On 10/22/18 9:01 AM, Martijn van Duren wrote:
>> I found another bug while testing ldap(1) to M$ AD.
>> Attributes like objectGUID are encoded as OCTET STRING, but are binary
>> data, which are not UTF-8 strings.
>> When looking at RFC4511 I found the following:
>> 4.1.5. Attribute Value
>> A field of type AttributeValue is an OCTET STRING containing an
>> encoded attribute value. The attribute value is encoded according to
>> the LDAP-specific encoding definition of its corresponding syntax.
>>
>> So we should not blindly utoa on an OCTET STRING.
>>
>> Diff below alleviates the problems when comparing the output of
>> ldapsearch from OpenLDAP, but is probably horribly wrong.
>>
>> Note that because of the current assumptions some values aren't printed
>> at all, because they contain NUL-characters in the string. The patch
>> below does not address this issue.
>>
>> martijn@
>>
> Index: aldap.c
> ===================================================================
> RCS file: /cvs/src/usr.bin/ldap/aldap.c,v
> retrieving revision 1.5
> diff -u -p -r1.5 aldap.c
> --- aldap.c 12 Aug 2018 22:04:09 -0000 1.5
> +++ aldap.c 23 Oct 2018 07:35:54 -0000
> @@ -39,7 +39,7 @@ static struct ber_element *ldap_parse_se
> char *);
> static struct ber_element *ldap_do_parse_search_filter(
> struct ber_element *, char **);
> -char **aldap_get_stringset(struct ber_element *);
> +struct ber_octetstring *aldap_get_stringset(struct ber_element
> *);
> char *utoa(char *);
> static int isu8cont(unsigned char);
> char *parseval(char *, size_t);
> @@ -511,7 +511,7 @@ aldap_get_resultcode(struct aldap_messag
> char *
> aldap_get_dn(struct aldap_message *msg)
> {
> - char *dn;
> + struct ber_octetstring dn;
>
> if (msg->dn == NULL)
> return NULL;
> @@ -519,10 +519,10 @@ aldap_get_dn(struct aldap_message *msg)
> if (ber_get_string(msg->dn, &dn) == -1)
> return NULL;
>
> - return utoa(dn);
> + return utoa(dn.bv_val);
> }
>
> -char **
> +struct ber_octetstring *
> aldap_get_references(struct aldap_message *msg)
> {
> if (msg->references == NULL)
> @@ -547,7 +547,7 @@ aldap_free_references(char **values)
> char *
> aldap_get_diagmsg(struct aldap_message *msg)
> {
> - char *s;
> + struct ber_octetstring s;
>
> if (msg->body.res.diagmsg == NULL)
> return NULL;
> @@ -555,7 +555,7 @@ aldap_get_diagmsg(struct aldap_message *
> if (ber_get_string(msg->body.res.diagmsg, &s) == -1)
> return NULL;
>
> - return utoa(s);
> + return utoa(s.bv_val);
> }
>
> int
> @@ -576,11 +576,12 @@ aldap_count_attrs(struct aldap_message *
> }
>
> int
> -aldap_first_attr(struct aldap_message *msg, char **outkey, char ***outvalues)
> +aldap_first_attr(struct aldap_message *msg, char **outkey,
> + struct ber_octetstring **outvalues)
> {
> struct ber_element *b, *c;
> char *key;
> - char **ret;
> + struct ber_octetstring *ret;
>
> if (msg->body.search.attrs == NULL)
> goto fail;
> @@ -605,11 +606,12 @@ fail:
> }
>
> int
> -aldap_next_attr(struct aldap_message *msg, char **outkey, char ***outvalues)
> +aldap_next_attr(struct aldap_message *msg, char **outkey,
> + struct ber_octetstring **outvalues)
> {
> struct ber_element *a, *b;
> char *key;
> - char **ret;
> + struct ber_octetstring *ret;
>
> if (msg->body.search.iter == NULL)
> goto notfound;
> @@ -640,11 +642,12 @@ notfound:
> }
>
> int
> -aldap_match_attr(struct aldap_message *msg, char *inkey, char ***outvalues)
> +aldap_match_attr(struct aldap_message *msg, char *inkey,
> + struct ber_octetstring **outvalues)
> {
> struct ber_element *a, *b;
> char *descr = NULL;
> - char **ret;
> + struct ber_octetstring *ret;
>
> if (msg->body.search.attrs == NULL)
> goto fail;
> @@ -677,16 +680,11 @@ notfound:
> }
>
> int
> -aldap_free_attr(char **values)
> +aldap_free_attr(struct ber_octetstring *values)
> {
> - int i;
> -
> if (values == NULL)
> return -1;
>
> - for (i = 0; values[i] != NULL; i++)
> - free(values[i]);
> -
> free(values);
>
> return (1);
> @@ -836,13 +834,12 @@ fail:
> * internal functions
> */
>
> -char **
> +struct ber_octetstring *
> aldap_get_stringset(struct ber_element *elm)
> {
> struct ber_element *a;
> int i;
> - char **ret;
> - char *s;
> + struct ber_octetstring *ret;
>
> if (elm->be_type != BER_TYPE_OCTETSTRING)
> return NULL;
> @@ -853,16 +850,13 @@ aldap_get_stringset(struct ber_element *
> if (i == 1)
> return NULL;
>
> - if ((ret = calloc(i + 1, sizeof(char *))) == NULL)
> + if ((ret = calloc(i + 1, sizeof(*ret))) == NULL)
> return NULL;
>
> for (a = elm, i = 0; a != NULL && a->be_type == BER_TYPE_OCTETSTRING;
> a = a->be_next, i++) {
> -
> - ber_get_string(a, &s);
> - ret[i] = utoa(s);
> + ber_get_string(a, &(ret[i]));
> }
> - ret[i + 1] = NULL;
>
> return ret;
> }
> Index: aldap.h
> ===================================================================
> RCS file: /cvs/src/usr.bin/ldap/aldap.h,v
> retrieving revision 1.1.1.1
> diff -u -p -r1.1.1.1 aldap.h
> --- aldap.h 13 Jun 2018 15:45:57 -0000 1.1.1.1
> +++ aldap.h 23 Oct 2018 07:35:54 -0000
> @@ -226,7 +226,7 @@ int aldap_get_errno(struct aldap *, con
> int aldap_get_resultcode(struct aldap_message *);
> char *aldap_get_dn(struct aldap_message *);
> char *aldap_get_diagmsg(struct aldap_message *);
> -char **aldap_get_references(struct aldap_message *);
> +struct ber_octetstring *aldap_get_references(struct aldap_message *);
> void aldap_free_references(char **values);
> int aldap_parse_url(const char *, struct aldap_url *);
> void aldap_free_url(struct aldap_url *);
> @@ -234,10 +234,13 @@ int aldap_search_url(struct aldap *, ch
> struct aldap_page_control *);
>
> int aldap_count_attrs(struct aldap_message *);
> -int aldap_match_attr(struct aldap_message *, char *, char ***);
> -int aldap_first_attr(struct aldap_message *, char **, char ***);
> -int aldap_next_attr(struct aldap_message *, char **, char ***);
> -int aldap_free_attr(char **);
> +int aldap_match_attr(struct aldap_message *, char *,
> + struct ber_octetstring **);
> +int aldap_first_attr(struct aldap_message *, char **,
> + struct ber_octetstring **);
> +int aldap_next_attr(struct aldap_message *, char **,
> + struct ber_octetstring **);
> +int aldap_free_attr(struct ber_octetstring *);
>
> struct aldap_page_control *aldap_parse_page_control(struct ber_element *,
> size_t len);
> void aldap_freepage(struct aldap_page_control *);
> Index: ber.c
> ===================================================================
> RCS file: /cvs/src/usr.bin/ldap/ber.c,v
> retrieving revision 1.19
> diff -u -p -r1.19 ber.c
> --- ber.c 12 Aug 2018 22:04:09 -0000 1.19
> +++ ber.c 23 Oct 2018 07:35:54 -0000
> @@ -283,12 +283,13 @@ ber_add_nstring(struct ber_element *prev
> }
>
> int
> -ber_get_string(struct ber_element *elm, char **s)
> +ber_get_string(struct ber_element *elm, struct ber_octetstring *s)
> {
> if (elm->be_encoding != BER_TYPE_OCTETSTRING)
> return -1;
>
> - *s = elm->be_val;
> + s->bv_val = elm->be_val;
> + s->be_len = elm->be_len;
> return 0;
> }
>
> @@ -660,6 +661,7 @@ ber_scanf_elements(struct ber_element *b
> off_t *pos;
> struct ber_oid *o;
> struct ber_element *parent[_MAX_SEQ], **e;
> + struct ber_octetstring octstr;
>
> memset(parent, 0, sizeof(struct ber_element *) * _MAX_SEQ);
>
> @@ -714,8 +716,9 @@ ber_scanf_elements(struct ber_element *b
> break;
> case 's':
> s = va_arg(ap, char **);
> - if (ber_get_string(ber, s) == -1)
> + if (ber_get_string(ber, &octstr) == -1)
> goto fail;
> + *s = octstr.bv_val;
> ret++;
> break;
> case 't':
> Index: ber.h
> ===================================================================
> RCS file: /cvs/src/usr.bin/ldap/ber.h,v
> retrieving revision 1.7
> diff -u -p -r1.7 ber.h
> --- ber.h 12 Aug 2018 22:04:09 -0000 1.7
> +++ ber.h 23 Oct 2018 07:35:54 -0000
> @@ -52,6 +52,11 @@ struct ber {
> unsigned int (*br_application)(struct ber_element *);
> };
>
> +struct ber_octetstring {
> + void *bv_val;
> + size_t be_len;
> +};
> +
> /* well-known ber_element types */
> #define BER_TYPE_DEFAULT ((unsigned int)-1)
> #define BER_TYPE_EOC 0
> @@ -104,7 +109,8 @@ int ber_get_boolean(struct
> ber_elemen
> struct ber_element *ber_add_string(struct ber_element *, const char *);
> struct ber_element *ber_add_nstring(struct ber_element *, const char *,
> size_t);
> -int ber_get_string(struct ber_element *, char **);
> +int ber_get_string(struct ber_element *, struct
> + ber_octetstring *);
> int ber_get_nstring(struct ber_element *, void **,
> size_t *);
> struct ber_element *ber_add_bitstring(struct ber_element *, const void *,
> Index: ldapclient.c
> ===================================================================
> RCS file: /cvs/src/usr.bin/ldap/ldapclient.c,v
> retrieving revision 1.3
> diff -u -p -r1.3 ldapclient.c
> --- ldapclient.c 3 Jul 2018 10:10:09 -0000 1.3
> +++ ldapclient.c 23 Oct 2018 07:35:54 -0000
> @@ -83,7 +83,8 @@ struct ldapc_search {
> __dead void usage(void);
> int ldapc_connect(struct ldapc *);
> int ldapc_search(struct ldapc *, struct ldapc_search *);
> -int ldapc_printattr(struct ldapc *, const char *, const char *);
> +int ldapc_printattr(struct ldapc *, const char *,
> + struct ber_octetstring *);
> void ldapc_disconnect(struct ldapc *);
> int ldapc_parseurl(struct ldapc *, struct ldapc_search *,
> const char *);
> @@ -298,7 +299,7 @@ ldapc_search(struct ldapc *ldap, struct
> const char *errstr;
> const char *searchdn, *dn = NULL;
> char *outkey;
> - char **outvalues;
> + struct ber_octetstring *outvalues;
> int ret, i, code, fail = 0;
>
> do {
> @@ -359,9 +360,9 @@ ldapc_search(struct ldapc *ldap, struct
> ret != -1;
> ret = aldap_next_attr(m, &outkey, &outvalues)) {
> for (i = 0; outvalues != NULL &&
> - outvalues[i] != NULL; i++) {
> + outvalues[i].bv_val != NULL; i++) {
> if (ldapc_printattr(ldap, outkey,
> - outvalues[i]) == -1) {
> + &(outvalues[i])) == -1) {
> fail = 1;
> break;
> }
> @@ -383,11 +384,13 @@ ldapc_search(struct ldapc *ldap, struct
> }
>
> int
> -ldapc_printattr(struct ldapc *ldap, const char *key, const char *value)
> +ldapc_printattr(struct ldapc *ldap, const char *key,
> + struct ber_octetstring *value)
> {
> char *p = NULL, *out;
> const unsigned char *cp;
> int encode;
> + size_t i;
> size_t inlen, outlen, left;
>
> if (ldap->ldap_flags & F_LDIF) {
> @@ -402,34 +405,34 @@ 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++) {
> + for (cp = value->bv_val, i = 0;
> + encode == 0 && i <= value->be_len - 1; i++) {
> /* !SAFE-CHAR %x01-09 / %x0B-0C / %x0E-7F */
> - if (*cp > 127 ||
> - *cp == '\0' ||
> - *cp == '\n' ||
> - *cp == '\r')
> + if (cp[i] > 127 ||
> + cp[i] == '\0' ||
> + cp[i] == '\n' ||
> + cp[i] == '\r')
> encode = 1;
> }
>
> if (!encode) {
> - if (asprintf(&p, "%s: %s", key, value) == -1) {
> + if (asprintf(&p, "%s: %s", key, value->bv_val) == -1) {
> log_warnx("asprintf");
> return (-1);
> }
> } else {
> - inlen = strlen(value);
> - outlen = inlen * 2 + 1;
> + outlen = value->be_len * 2 + 1;
>
> if ((out = calloc(1, outlen)) == NULL ||
> - b64_ntop(value, inlen, out, outlen) == -1) {
> + b64_ntop(value->bv_val, value->be_len, out, outlen)
> + == -1) {
> log_warnx("Base64 encoding failed");
> free(p);
> return (-1);
> }
>
> /* Base64 is indicated with a double-colon */
> - if (asprintf(&p, "%s: %s", key, out) == -1) {
> + if (asprintf(&p, "%s:: %s", key, out) == -1) {
> log_warnx("asprintf");
> free(out);
> return (-1);
> @@ -458,7 +461,9 @@ ldapc_printattr(struct ldapc *ldap, cons
> * on all values no matter if they include non-printable
> * characters.
> */
> - if (stravis(&p, value, VIS_SAFE|VIS_NL) == -1) {
> + p = calloc(1, 4 * value->be_len + 1);
> + if (strvisx(p, value->bv_val, value->be_len, VIS_SAFE|VIS_NL)
> + == -1) {
> log_warn("visual encoding failed");
> return (-1);
> }
>
