Skip to content

Issue 6367 - RFE support of Session Tracking Control internet draft #6403

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
Nov 29, 2024

Conversation

tbordaz
Copy link
Contributor

@tbordaz tbordaz commented Nov 13, 2024

Bug description:
This RFE is to support https://datatracker.ietf.org/doc/html/draft-wahl-ldap-session-03
In short, it allows a client to send strings in a control.
Those strings are added to the operation result logged in the
access logs.
Those strings are meaningful for the client (debug,
kmonitoring,...).

Fix description:
The design is https://www.port389.org/docs/389ds/design/session-identifier-in-logs.html

fixes: #6367

Reviewed by:

@Firstyear
Copy link
Contributor

This is potentially an RCE due to ascii control characters when viewed by an admin.

@tbordaz
Copy link
Contributor Author

tbordaz commented Nov 13, 2024

Thank @Firstyear for the rapid review. I assume RCE means remote code execution. I agree that taking client data requires careful checking. This data is ascii printable, less than 64K long. It is truncated to 15 first chars that are print in the log. Those chars (15bytes) are stored in the pblock. How can it be a RCE ?

@progier389
Copy link
Contributor

progier389 commented Nov 13, 2024

Not sure if it is really a RCE but anyway I think that @Firstyear concern is valid:
Logging directly the tag could be tricky for admin reading or running script on the log files and generates problems.
We should better escape the tag when logging it in access log to avoid bad surprise.

@Firstyear
Copy link
Contributor

@tbordaz ascii control characters - there is a method of adding them into a log such than an admin who views the log will trigger code to exec in their shell.

You need to filter our ascii control chars is generally the easiest fix.

@tbordaz tbordaz force-pushed the issue_6367 branch 2 times, most recently from 8f04829 to 07aaac9 Compare November 18, 2024 14:20
@progier389
Copy link
Contributor

Looks good except a few minor points:
code simplification (ber parsing and redundant size testing)
and more importantly an error message that IMHO should be changed

Bug description:
	This RFE is to support https://datatracker.ietf.org/doc/html/draft-wahl-ldap-session-03
	In short, it allows a client to send strings in a control.
	Those strings are added to the operation result logged in the
	access logs.
	Those strings are meaningful for the client (debug,
	kmonitoring,...).

Fix description:
	The design is https://www.port389.org/docs/389ds/design/session-identifier-in-logs.html

fixes: 389ds#6367

Reviewed by: William Brown, Pierre Rogier (Thanks !!!)
Copy link
Contributor

@progier389 progier389 left a comment

Choose a reason for hiding this comment

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

LGTM

@tbordaz tbordaz merged commit fd62700 into 389ds:main Nov 29, 2024
199 checks passed
tbordaz added a commit that referenced this pull request Feb 11, 2025
…6403)

Bug description:
	This RFE is to support https://datatracker.ietf.org/doc/html/draft-wahl-ldap-session-03
	In short, it allows a client to send strings in a control.
	Those strings are added to the operation result logged in the
	access logs.
	Those strings are meaningful for the client (debug,
	kmonitoring,...).

Fix description:
	The design is https://www.port389.org/docs/389ds/design/session-identifier-in-logs.html

fixes: #6367

Reviewed by: William Brown, Pierre Rogier (Thanks !!!)
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.

RFE support of Session Tracking Control internet draft
3 participants