Skip to content

[FIX] RFC4514 compliance#4643

Closed
blaggacao wants to merge 1 commit intopyca:masterfrom
blaggacao:patch-1
Closed

[FIX] RFC4514 compliance#4643
blaggacao wants to merge 1 commit intopyca:masterfrom
blaggacao:patch-1

Conversation

@blaggacao
Copy link
Copy Markdown

That space doesn't seem to be covered by the RFC.
See: https://www.ietf.org/rfc/rfc4514.txt
Specifically, the example: UID=jsmith,DC=example,DC=net

That space doesn't seem to be covered by the RFC.
See: https://www.ietf.org/rfc/rfc4514.txt
Specifically, the example: UID=jsmith,DC=example,DC=net
blaggacao pushed a commit to xoe-labs/facturark that referenced this pull request Dec 16, 2018
@reaperhulk
Copy link
Copy Markdown
Member

@intgr, is this your read of the RFC?

(If this is indeed a change we want to make the tests will need to be updated to pass with the new form)

@intgr
Copy link
Copy Markdown
Contributor

intgr commented Dec 16, 2018

Oh, I could have sworn that whitespace between separators was allowed by RFC 4514 but it's really vague about that. It's merely hinted in a few places, such as the rules for escaping leading/trailing spaces and the excerpt below. As much as I like the spaces for readability, for better compliance, I agree it's best to remove them.

I must have remembered that from RFC 2253 instead (which 4514 is based on), that states:

Implementations MUST allow for space (' ' ASCII 32) characters to be
present between name-component and ',', between attributeTypeAndValue
and '+', between attributeType and '=', and between '=' and
attributeValue. These space characters are ignored when parsing.

The closest thing in RFC 4514 is:

DN strings can be quite long. It is often desirable to line-wrap
overly long DN strings in presentations. Line wrapping should be
done by inserting whitespace after the RDN separator character or, if
necessary, after the AVA separator character. It should be noted to
the user that the inserted whitespace is not part of the DN string
and is to be removed before use in LDAP. For example, the following
DN string is long:

    CN=Kurt D.  Zeilenga,OU=Engineering,L=Redwood Shores,
    O=OpenLDAP Foundation,ST=California,C=US

So it has been line-wrapped for readability. The extra whitespace is
to be removed before the DN string is used in LDAP.

Inserting whitespace is not advised because it may not be obvious to
the user which whitespace is part of the DN string and which
whitespace was added for readability.

@intgr
Copy link
Copy Markdown
Contributor

intgr commented Dec 16, 2018

@blaggacao Do you want to finish this yourself (fixing the tests) or shall I?

@blaggacao
Copy link
Copy Markdown
Author

@intgr I guess I'm not really acquainted with the this library, just spotted this in a debugging journey. It would be best you finish this off.

intgr added a commit to intgr/cryptography that referenced this pull request Dec 17, 2018
RFC 4514 does not explicitly allow whitespace between separators:
https://tools.ietf.org/html/rfc4514

Reported-by: David Arnold <dar@xoe.solutions>
intgr added a commit to intgr/cryptography that referenced this pull request Dec 17, 2018
RFC 4514 does not explicitly allow whitespace between separators:
https://tools.ietf.org/html/rfc4514

Reported-by: David Arnold <dar@xoe.solutions>
@intgr
Copy link
Copy Markdown
Contributor

intgr commented Dec 17, 2018

I had to create a new pull request for this, see: #4646

@blaggacao
Copy link
Copy Markdown
Author

Cool! So I close this one.

@blaggacao blaggacao closed this Dec 17, 2018
intgr added a commit to intgr/cryptography that referenced this pull request Dec 17, 2018
RFC 4514 does not explicitly allow whitespace between separators:
https://tools.ietf.org/html/rfc4514

Reported-by: David Arnold <dar@xoe.solutions>
reaperhulk pushed a commit that referenced this pull request Dec 17, 2018
)

RFC 4514 does not explicitly allow whitespace between separators:
https://tools.ietf.org/html/rfc4514

Reported-by: David Arnold <dar@xoe.solutions>
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 3, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants