Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

certmap fails when Issuer DN has comma in name #2602

Closed
389-ds-bot opened this issue Sep 13, 2020 · 6 comments
Closed

certmap fails when Issuer DN has comma in name #2602

389-ds-bot opened this issue Sep 13, 2020 · 6 comments
Labels
closed: fixed Migration flag - Issue
Milestone

Comments

@389-ds-bot
Copy link

Cloned from Pagure issue: https://pagure.io/389-ds-base/issue/49543

  • Created at 2018-01-19 06:44:57 by ftweedal
  • Closed at 2018-11-26 18:34:42 as fixed
  • Assigned to nobody

Issue Description

When issuer Dn has a comma in it (or presumably other characters that reqiure escaping),
certmap fails because a comparison between the stringified Issuer DN from the certificate,
and the issuer DN from the certmap file, fails. For example, if the Issuer DN is:

CN=Certificate Authority,O="Acme, Inc.",ST=Massachusetts,C=US

This gets read from the certificate via the ldapu_get_cert_issuer_dn, which uses
NSS' CERT_NameToAscii to return a stringified version of the DN. CERT_NameToAscii uses the RFC 1485 rules to serialise the DN, so the string looks like:

CN=Certificate Authority,O="Acme, Inc.",ST=Massachusetts,C=US

This string then gets processed by ldapu_dn_normalize which turns it into:

CN=Certificate Authority,O="Acme, Inc.",ST=Massachusetts,C=US

, which is wrong.

The comparison then fails when compared with the DN in the certmap.conf, which
is properly escaped (a basic strcasecmp):

CN=Certificate Authority,O=Acme\, Inc.,ST=Massachusetts,C=US

Package Version and Platform

v1.3.7

Steps to reproduce

  1. configure a certmap with an issuer that has comma in a name.
  2. attempt certificate bind with valid certificate issued by the certmap issuer.

Actual results

Bind fails, and doesn't even reach the internal search op for a user
matching the certificate. (failure to match the issuer caused fallback to default certmap).

Expected results

Bind succeeds, or at least uses the correct certmap such that internal search ops to look
up a matching user are executed.

Notes on proposed fix

NSS stringifies DNs in the obsolete RFC 1485 format (https://bugzilla.mozilla.org/show_bug.cgi?id=355096).

We should avoid string DNs entirely. The issuer DN from certmap.conf should
be parsed into 389's internal DN representation. The X.509 Issuer DN from the certificate
should be directly converted into the same internal representation, without becoming
a string. It may be possible to directly decode the DER-encoded Issuer DN from the cert.

Then these two internal representations should be compared by the appropriate function.

Related issues

It is likely that the treatment of the Subject DN is also incorrect in the presence of commas, etc, because the certmap code also uses CERT_NameToAscii to read the Subject DN from the
certificate (which is then used e.g. in the construction of a user search filter).

@389-ds-bot 389-ds-bot added the closed: fixed Migration flag - Issue label Sep 13, 2020
@389-ds-bot 389-ds-bot added this to the 1.3.7.0 milestone Sep 13, 2020
@389-ds-bot
Copy link
Author

Comment from mreynolds (@mreynolds389) at 2018-02-13 18:32:48

Metadata Update from @mreynolds389:

  • Custom field component adjusted to None
  • Custom field origin adjusted to None
  • Custom field reviewstatus adjusted to None
  • Custom field type adjusted to None
  • Custom field version adjusted to None
  • Issue set to the milestone: 1.3.7.0

@389-ds-bot
Copy link
Author

389-ds-bot commented Sep 13, 2020

Comment from ftweedal at 2018-03-19 04:43:49

PR: #2670

@389-ds-bot
Copy link
Author

@389-ds-bot
Copy link
Author

Comment from mreynolds (@mreynolds389) at 2018-11-26 18:34:21

Metadata Update from @mreynolds389:

  • Issue assigned to mreynolds389

@389-ds-bot
Copy link
Author

Comment from mreynolds (@mreynolds389) at 2018-11-26 18:34:43

Metadata Update from @mreynolds389:

  • Assignee reset
  • Issue close_status updated to: fixed
  • Issue status updated to: Closed (was: Open)

@389-ds-bot
Copy link
Author

Comment from mreynolds (@mreynolds389) at 2018-11-26 18:35:11

3800388..818807d 389-ds-base-1.3.8 -> 389-ds-base-1.3.8

9740c20..1a93d63 389-ds-base-1.3.9 -> 389-ds-base-1.3.9

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed: fixed Migration flag - Issue
Projects
None yet
Development

No branches or pull requests

1 participant