Issue 7863 - Invalid DN syntax (34) not written by slapo-accesslog
Summary: Invalid DN syntax (34) not written by slapo-accesslog
Status: UNCONFIRMED
Alias: None
Product: OpenLDAP
Classification: Unclassified
Component: overlays (show other issues)
Version: unspecified
Hardware: All All
: --- enhancement
Target Milestone: ---
Assignee: OpenLDAP project
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-05-28 16:25 UTC by Michael Ströder
Modified: 2020-03-20 20:48 UTC (History)
0 users

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this issue.
Description Michael Ströder 2014-05-28 16:25:38 UTC
Full_Name: Michael Str�der
Version: HEAD and RE24
OS: Linux
URL: 
Submission from: (NULL) (212.227.35.93)


It seems that modify requests which failed due to Invalid DN syntax (34) are not
written to accesslog-DB. I guess that those requests get abandoned by the
frontend and never reach the backend at all.

It would be handy to see the invalid modify request in the accesslog-DB though.

Any chance to achieve this?
Comment 1 Howard Chu 2014-05-29 07:22:58 UTC
michael@stroeder.com wrote:
> Full_Name: Michael Ströder
> Version: HEAD and RE24
> OS: Linux
> URL:
> Submission from: (NULL) (212.227.35.93)
>
>
> It seems that modify requests which failed due to Invalid DN syntax (34) are not
> written to accesslog-DB. I guess that those requests get abandoned by the
> frontend and never reach the backend at all.

Correct.

> It would be handy to see the invalid modify request in the accesslog-DB though.
>
> Any chance to achieve this?

Not likely. The frontend must call select_backend() based on the incoming DN 
to determine which backend to invoke, and thus which stack of overlays are 
involved. If the DN is invalid, no selection can occur.

-- 
   -- Howard Chu
   CTO, Symas Corp.           http://www.symas.com
   Director, Highland Sun     http://highlandsun.com/hyc/
   Chief Architect, OpenLDAP  http://www.openldap.org/project/

Comment 2 Michael Ströder 2014-05-29 16:13:13 UTC
Howard Chu wrote:
> michael@stroeder.com wrote:
>> It seems that modify requests which failed due to Invalid DN syntax (34) are
>> not
>> written to accesslog-DB. I guess that those requests get abandoned by the
>> frontend and never reach the backend at all.
> 
> Correct.
> 
>> It would be handy to see the invalid modify request in the accesslog-DB though.
>>
>> Any chance to achieve this?
> 
> Not likely. The frontend must call select_backend() based on the incoming DN
> to determine which backend to invoke, and thus which stack of overlays are
> involved. If the DN is invalid, no selection can occur.

Hmm, I've done some more tests. Invalid syntax (21) also does not make it
beyond the frontend into accesslog-DB.

I have no clear opinion on this. Of course the current behaviour is good for
performance. But sometimes one would like to observe what broken LDAP clients
sent in a modify request in the past.

Also running with BER loglevel or breaking up the TLS connection with stunnel
and sniff with Wireshark is not always an option.

Having this configurable would be great.

What's your opinion on this?

Ciao, Michael.

Comment 3 ando@openldap.org 2014-05-29 19:30:42 UTC
On 05/29/2014 06:13 PM, michael@stroeder.com wrote:
> Howard Chu wrote:
>> michael@stroeder.com wrote:
>>> It seems that modify requests which failed due to Invalid DN syntax (34) are
>>> not
>>> written to accesslog-DB. I guess that those requests get abandoned by the
>>> frontend and never reach the backend at all.
>>
>> Correct.
>>
>>> It would be handy to see the invalid modify request in the accesslog-DB though.
>>>
>>> Any chance to achieve this?
>>
>> Not likely. The frontend must call select_backend() based on the incoming DN
>> to determine which backend to invoke, and thus which stack of overlays are
>> involved. If the DN is invalid, no selection can occur.
>
> Hmm, I've done some more tests. Invalid syntax (21) also does not make it
> beyond the frontend into accesslog-DB.
>
> I have no clear opinion on this. Of course the current behaviour is good for
> performance. But sometimes one would like to observe what broken LDAP clients
> sent in a modify request in the past.
>
> Also running with BER loglevel or breaking up the TLS connection with stunnel
> and sniff with Wireshark is not always an option.
>
> Having this configurable would be great.
>
> What's your opinion on this?

In terms of code architecture, overlays have been designed to work on 
validated data (the request must be well formed).  To this end, the 
notion of "frontend" database was added to make it possible to layer an 
overlay *before* backend selection.

To have a custom piece of code muck with a request before it is 
validated (or even before it is parsed) you need to split the code into:

- parsing, without doing any sanity check

- allow custom code to step in

- do validation afterwards

This does not necessarily imply performance penalty (when no custom code 
steps in, the amount of operations would be the same for sane requests; 
in case of malformed requests, you would miss the opportunity to bail 
out on the first inconsistency).

However, it would imply significant refactorization of the current code, 
without (IMHO) a strong motivation for it: malformed requests are 
already handled in terms of response to clients, and somehow logged.

An (expensive) alternative would be to register a handler at connection 
level which duplicates the parsing to anticipate checking for errors 
that would be detected before passing control to the frontend.  Not the 
way I'd go, unless strictly necessary.

A paranoid but intriguing alternative would be to implement an operation 
mode that keeps parsing after the first error is detected, turning the 
operation into a no-op.  Again, lots of refactorization and error 
handling and so on (for example, what if a syntax, an attribute 
description or a matching rule is not found?  You'd always have to check 
that the corresponding pointer is not NULL before using it, whereas 
right now we often assume that if execution got there, data must be 
consistent.  Of course, we could introduce dummy syntaxes 
"NonExistingSyntax", ..., but we'd probably need to define their 
semantics in such a way that they apply to every case we need to handle 
and so).

My 2c.

p.

-- 
Pierangelo Masarati
Associate Professor
Dipartimento di Scienze e Tecnologie Aerospaziali
Politecnico di Milano

Comment 4 ando@openldap.org 2014-05-29 19:49:54 UTC
On 05/29/2014 09:31 PM, pierangelo.masarati@polimi.it wrote:
> On 05/29/2014 06:13 PM, michael@stroeder.com wrote:
>> Howard Chu wrote:
>>> michael@stroeder.com wrote:
>>>> It seems that modify requests which failed due to Invalid DN syntax (34) are
>>>> not
>>>> written to accesslog-DB. I guess that those requests get abandoned by the
>>>> frontend and never reach the backend at all.
>>>
>>> Correct.
>>>
>>>> It would be handy to see the invalid modify request in the accesslog-DB though.
>>>>
>>>> Any chance to achieve this?
>>>
>>> Not likely. The frontend must call select_backend() based on the incoming DN
>>> to determine which backend to invoke, and thus which stack of overlays are
>>> involved. If the DN is invalid, no selection can occur.
>>
>> Hmm, I've done some more tests. Invalid syntax (21) also does not make it
>> beyond the frontend into accesslog-DB.
>>
>> I have no clear opinion on this. Of course the current behaviour is good for
>> performance. But sometimes one would like to observe what broken LDAP clients
>> sent in a modify request in the past.
>>
>> Also running with BER loglevel or breaking up the TLS connection with stunnel
>> and sniff with Wireshark is not always an option.
>>
>> Having this configurable would be great.
>>
>> What's your opinion on this?
>
> In terms of code architecture, overlays have been designed to work on
> validated data (the request must be well formed).  To this end, the
> notion of "frontend" database was added to make it possible to layer an
> overlay *before* backend selection.
>
> To have a custom piece of code muck with a request before it is
> validated (or even before it is parsed) you need to split the code into:
>
> - parsing, without doing any sanity check
>
> - allow custom code to step in
>
> - do validation afterwards
>
> This does not necessarily imply performance penalty (when no custom code
> steps in, the amount of operations would be the same for sane requests;
> in case of malformed requests, you would miss the opportunity to bail
> out on the first inconsistency).
>
> However, it would imply significant refactorization of the current code,
> without (IMHO) a strong motivation for it: malformed requests are
> already handled in terms of response to clients, and somehow logged.
>
> An (expensive) alternative would be to register a handler at connection
> level which duplicates the parsing to anticipate checking for errors
> that would be detected before passing control to the frontend.  Not the
> way I'd go, unless strictly necessary.
>
> A paranoid but intriguing alternative would be to implement an operation
> mode that keeps parsing after the first error is detected, turning the
> operation into a no-op.  Again, lots of refactorization and error
> handling and so on (for example, what if a syntax, an attribute
> description or a matching rule is not found?  You'd always have to check
> that the corresponding pointer is not NULL before using it, whereas
> right now we often assume that if execution got there, data must be
> consistent.  Of course, we could introduce dummy syntaxes
> "NonExistingSyntax", ..., but we'd probably need to define their
> semantics in such a way that they apply to every case we need to handle
> and so).

It now comes to my mind that another perhaps less intrusive chance of 
intervention could be to act at the *response* level.  Think of:

- the possibility to stack code in the response chain (not necessarily 
overlays)

- have a way to understand if the response originates *before* the 
frontend was called

In this case, the custom code could re-parse the request to re-detect 
why it failed and handle it (e.g. log custom information on failure 
reason).  Not a piece of cake, but probably less intrusive than other 
options.  (I'm not sure the raw request buffer is still available at 
this stage, though.)

p.

-- 
Pierangelo Masarati
Associate Professor
Dipartimento di Scienze e Tecnologie Aerospaziali
Politecnico di Milano

Comment 5 Howard Chu 2014-05-29 23:33:25 UTC
pierangelo.masarati@polimi.it wrote:
> It now comes to my mind that another perhaps less intrusive chance of
> intervention could be to act at the *response* level.  Think of:
>
> - the possibility to stack code in the response chain (not necessarily
> overlays)
>
> - have a way to understand if the response originates *before* the
> frontend was called
>
> In this case, the custom code could re-parse the request to re-detect
> why it failed and handle it (e.g. log custom information on failure
> reason).  Not a piece of cake, but probably less intrusive than other
> options.  (I'm not sure the raw request buffer is still available at
> this stage, though.)

Still not something useful for slapo-accesslog - we're storing log info as 
LDAP attributes. We can't store reqDN if the DN has invalid syntax. We can't 
store reqMod values if the attributetype is unknown or the values are invalid. 
When a request fails validation it's literally garbage, and toxic - cannot be 
considered safe to preserve in a DB.

-- 
   -- Howard Chu
   CTO, Symas Corp.           http://www.symas.com
   Director, Highland Sun     http://highlandsun.com/hyc/
   Chief Architect, OpenLDAP  http://www.openldap.org/project/

Comment 6 ando@openldap.org 2014-05-30 07:31:02 UTC
On 05/30/2014 01:33 AM, hyc@symas.com wrote:
> pierangelo.masarati@polimi.it wrote:
>> It now comes to my mind that another perhaps less intrusive chance of
>> intervention could be to act at the *response* level.  Think of:
>>
>> - the possibility to stack code in the response chain (not necessarily
>> overlays)
>>
>> - have a way to understand if the response originates *before* the
>> frontend was called
>>
>> In this case, the custom code could re-parse the request to re-detect
>> why it failed and handle it (e.g. log custom information on failure
>> reason).  Not a piece of cake, but probably less intrusive than other
>> options.  (I'm not sure the raw request buffer is still available at
>> this stage, though.)
>
> Still not something useful for slapo-accesslog - we're storing log info as
> LDAP attributes. We can't store reqDN if the DN has invalid syntax. We can't
> store reqMod values if the attributetype is unknown or the values are invalid.
> When a request fails validation it's literally garbage, and toxic - cannot be
> considered safe to preserve in a DB.

Sure.  Indeed, it could not be associated with accesslog (invalid DN 
needs to be detected way before a database can be selected).  Such a 
case would belong to some "debugging" layer (e.g. client-server 
interaction troubleshooting), which BTW I'm convinced the current 
logging is more than enough for.

p.


-- 
Pierangelo Masarati
Associate Professor
Dipartimento di Scienze e Tecnologie Aerospaziali
Politecnico di Milano