Skip to content

fix: Replace DER with ASN1 BER encoding when parsing distinguishedNames#505

Merged
cpuschma merged 2 commits intogo-ldap:masterfrom
cpuschma:fix/string-ber-encoding
Apr 7, 2024
Merged

fix: Replace DER with ASN1 BER encoding when parsing distinguishedNames#505
cpuschma merged 2 commits intogo-ldap:masterfrom
cpuschma:fix/string-ber-encoding

Conversation

@cpuschma
Copy link
Member

@cpuschma cpuschma commented Apr 7, 2024

This PR replaces the Go asn1 library in the decodeEncodedString function to decode values with ASN1 BER instead. The replacement of the returned error should be ok, as there hasn't been a release with the new implementation of `ParseDN' yet.

See RFC4514 Section 2.4:

If the AttributeType is of the dotted-decimal form, the
AttributeValue is represented by an number sign ('#' U+0023)
character followed by the hexadecimal encoding of each of the octets
of the BER encoding of the X.500 AttributeValue. This form is also
used when the syntax of the AttributeValue does not have an LDAP-
specific ([RFC4517], Section 3.1) string encoding defined for it, or
the LDAP-specific string encoding is not restricted to UTF-8-encoded
Unicode characters. This form may also be used in other cases, such
as when a reversible string representation is desired (see Section
5.2).

@cpuschma cpuschma merged commit 2260012 into go-ldap:master Apr 7, 2024
@cpuschma cpuschma deleted the fix/string-ber-encoding branch April 7, 2024 09:36
@inteon
Copy link
Contributor

inteon commented Apr 10, 2024

@cpuschma FYI: I found a fix for the bug in the ber package that causes the high memory usage: go-asn1-ber/asn1-ber#42

gustavoluvizotto pushed a commit to gustavoluvizotto/ldap-fork that referenced this pull request Apr 11, 2024
…es (go-ldap#505)

* fix: Replace DER with ASN1 BER encoding when parsing distinguishedNames
* Remove leftover comment
@cpuschma
Copy link
Member Author

cpuschma commented Apr 11, 2024

I don't have permissions to review and merge your PR in go-asn1-ber. @johnweldon Can you have a look?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants