Skip to content

Issue 6199 - wrong search query during certificate based authentication #6205

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

Merged
merged 1 commit into from
Jun 10, 2024

Conversation

progier389
Copy link
Contributor

@progier389 progier389 commented Jun 6, 2024

Problems:
SubjectDN extracted from the certificate is not escaped when used by certmap.conf
Other extracted value are wrongly escaped and quoted when added in filter

Solution: Ensure that proper escape function is used in these two cases.
Values in filter should not be quoted but * should be escaped.

Note: I considered to reuse the ldap_bv2escaped_filter_value function but it needless realloc the returned data
so I ended up to rewrite something the escape function (which is quite straightforward anyway).

Issue: #6199

Reviewed by: @droideck

@progier389 progier389 self-assigned this Jun 6, 2024
@progier389
Copy link
Contributor Author

Fixed the extra space that cause "Validate tests" to fail
Fixed the missing free(certFilter)

Copy link
Member

@droideck droideck left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it'll be better to wait for CI to be fixed so we can check the PR against tests, but the code looks good to me!

@progier389
Copy link
Contributor Author

I agree about waiting for the CI tests

@progier389 progier389 force-pushed the i6199 branch 2 times, most recently from 6ec2c76 to 4142527 Compare June 10, 2024 13:08
@progier389 progier389 merged commit eedde89 into 389ds:main Jun 10, 2024
193 checks passed
progier389 added a commit that referenced this pull request Oct 25, 2024
…tication (#6205)

Problems:
SubjectDN extracted from the certificate is not escaped when used by certmap.conf
Other extracted value are wrongly escaped and quoted when added in filter

Solution: Ensure that proper escape function is used in these two cases.
Values in filter should not be quoted but * should be escaped.

Note: I considered to reuse the ldap_bv2escaped_filter_value function but it needless realloc the returned data
so I ended up to rewrite something the escape function (which is quite straightforward anyway).

Issue: #6199

Reviewed by: @droideck

(cherry picked from commit eedde89)
progier389 added a commit that referenced this pull request Oct 25, 2024
…tication (#6205)

Problems:
SubjectDN extracted from the certificate is not escaped when used by certmap.conf
Other extracted value are wrongly escaped and quoted when added in filter

Solution: Ensure that proper escape function is used in these two cases.
Values in filter should not be quoted but * should be escaped.

Note: I considered to reuse the ldap_bv2escaped_filter_value function but it needless realloc the returned data
so I ended up to rewrite something the escape function (which is quite straightforward anyway).

Issue: #6199

Reviewed by: @droideck

(cherry picked from commit eedde89)
@tbordaz
Copy link
Contributor

tbordaz commented Oct 25, 2024

Sorry to come late to this thread. Just a question regarding value2filter_sizes.
should not we also escape chars: ',' (comma), '=' (equal), '&' (and), '|' (or), '!'

@progier389
Copy link
Contributor Author

progier389 commented Oct 25, 2024

According to https://datatracker.ietf.org/doc/html/rfc4515:

The <valueencoding> rule ensures that the entire filter string is a
   valid UTF-8 string and provides that the octets that represent the
   ASCII characters "*" (ASCII 0x2a), "(" (ASCII 0x28), ")" (ASCII
   0x29), "\" (ASCII 0x5c), and NUL (ASCII 0x00) are represented as a
   backslash "\" (ASCII 0x5c) followed by the two hexadecimal digits
   representing the value of the encoded octet.

So the answer is no.

FYI: in this code we are escaping filter value (that are already dn string representation (i.e escaped as dn according to RFC4514 ) )
so value2filter_sizes only handles the filter value escape.

@progier389 progier389 deleted the i6199 branch May 20, 2025 13:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

LDAP unprotected search query during certificate based authentication
3 participants