ldap(1) BER_TYPE_OCTETSTRING isn't always UTF-8 string

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

ldap(1) BER_TYPE_OCTETSTRING isn't always UTF-8 string

Martijn van Duren-8
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 22 Oct 2018 06:54:22 -0000
@@ -860,7 +860,9 @@ aldap_get_stringset(struct ber_element *
     a = a->be_next, i++) {
 
  ber_get_string(a, &s);
- ret[i] = utoa(s);
+ if ((ret[i] = calloc(a->be_len + 1, 1)) == NULL)
+ continue;
+ memcpy(ret[i], s, a->be_len);
  }
  ret[i + 1] = NULL;
 

Reply | Threaded
Open this post in threaded view
|

Re: ldap(1) BER_TYPE_OCTETSTRING isn't always UTF-8 string

Reyk Floeter-2
On Mon, Oct 22, 2018 at 09:01:21AM +0200, 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.
>

But it makes sense, if we cannot trust it to be UTF-8 than it
shouldn't be assumed.  aldap comes from ypldap(8), so the same problem
exists there.  Could it eventually affect authentication credentials
that include UTF-8?

> 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.
>

What do you mean with this or could you give an example?  I use vis(1)
to print the attribute values, or base64 in LDIF mode, and it detects
"SAFE-STRINGs" (no \0 etc.) that can be printed without encoding.

Reyk

> 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 22 Oct 2018 06:54:22 -0000
> @@ -860,7 +860,9 @@ aldap_get_stringset(struct ber_element *
>      a = a->be_next, i++) {
>  
>   ber_get_string(a, &s);
> - ret[i] = utoa(s);
> + if ((ret[i] = calloc(a->be_len + 1, 1)) == NULL)
> + continue;
> + memcpy(ret[i], s, a->be_len);
>   }
>   ret[i + 1] = NULL;
>  
>

--

Reply | Threaded
Open this post in threaded view
|

Re: ldap(1) BER_TYPE_OCTETSTRING isn't always UTF-8 string

Martijn van Duren-8
In reply to this post by Martijn van Duren-8
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);
  }

Reply | Threaded
Open this post in threaded view
|

Re: ldap(1) BER_TYPE_OCTETSTRING isn't always UTF-8 string

Martijn van Duren-8
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);
>   }
>