Re: X.509 Certificate parsing Server (ITS#3184)

[ando@sys-net.it]

First of all --- let's keep all the conversation in cc
with the ITS, so we can keep track of it


> I've read you comments and they are really helpful. It's a shame I
> didn't discuss any of it with you before writing the code.
> I'll try to do my best to make all the fixes and improvements, but now I
> have an obligation to my University (to wright a thesis), so I'll do it
> in my free time. It would be nice to have a rough time line (or
> deadline) for making al the changes

I don't think there's need for a deadline, as much of your work (and
mine!) seems to be based on volunteering.  Since the change is not urgent,
a reasonable deadline would be 4Q 2004, if you really want a number.  But
don't feel obliged!

>>
>> - the change could be done thru the overlay (or "slapo") mechanism, thus
>> avoiding any impact on the frontend code.
>
> Yes, BTW I really tried to minimize the XPS code present in OpenLDAP
> fronend code, but due to specific of the XPS it wasn't that easy to do.
> Anyway overlays sound perfect, but I will need at least some
> documentation or pointers to where in the code to look at.

There is very little documentation at present, as the api is not frozen
yet, but I see changes slowed down very much in the past months, and it
seems to have cooled down quite a bit.  Referring to HEAD code, there's
a memorandum by Howard Chu in servers/slapd/overlays/slapover.txt;
the frontend code for overlays is in servers/slapd/backover.c; you don't
need to deal with it, but it might be nice to have a look at the code that
actually calls the overlays handlers.  Of course, declarations are in
servers/slapd/slap.h and servers/slapd/proto-slap.h.

The directory servers/slapd/overlays/ also contains a few examples.

>
>>   To reduce the clobbering
>> of the slapd.conf namespace, it is suggested that all the specific
>> parameters
>> be prefixed with "xps-";
>
>
> No problems there...

:) ... or "xps_", if you prefer the underscore style!

>
>> the idea of having a specific .conf file to be
>> included is reasonable, but it should not be a requirement.
>
>
> I don't believe to is right now. I just tried to separate them from
> usual options to avoid any possible confusions.

OK, that's good for your example, but I'd leave this coding style to those
system administrators that like it.

>
>> Note that overlays currently apply inside database specific portions
>> of slapd, but there are plans to introduce "frontend" overlays that
>> apply
>> before database selection (ITS#3080), which could be of use in case
>> the XPS needs be applied to any database.
>
> So it will be OK to implement it for BDB and then make it a frontend
> overlay if required?

If you implement it as an overlay, it will work for any backend
(in principle :) as soon as the addition/handling of extra entries
is handled in the appropriate manner, in principle it should also
apply to proxy backends, and the changes be propagated, although
I'm not sure I see any use for it at present.

>
>> If help with overlays is required, feel free to ask; I may provide you
>> a template with the fundamental calls you need to fill in.
>
> That would be great, while I was writing the code I really needed this
> kind of stuff :)

A template is available at http://www.sys-net.it/~ando/Download/xps.c
I made the copyright statement compliant to OpenLDAP's, as a suggestion,
if you don't mind.  Feel free to set it as you deem appropriate, including
the original one you used in your first submission.

>
>> - it is also suggested that the helper functions specific to XPS be
>> either
>> delcared as "static" in the xps.c file of the overlay, or prefixed
>> with "slap_".
>
> which way is better?

If it is self-contained (i.e. one file), use static functions; in any
case, a clean namespace is preferable.  In the template I used
slapo_xps_*.

>
>> - the overlay should be put in a specific "xps" directory under
>> "contrib/slapd-modules/"
>>
>> - the specific  schema could be hard-wired into the overlay code,
>> much like it's currently done for, e.g. overlays/ppolicy.c or
>> back-monitor/init.c
>
> I assume it will make things faster, but the thing is - certificates
> have all kind of weird extensions I'm not even familiar with so we tried
> to make the XPS as extensible as possible. In this case, hardcoded
> schema would be a disadvantage, don't you think?

Not a performance issue; if you think it's frozen, you can hardwire it. 
If you think it's not, you can use files.  The point is that it is
advisable to hardwire schema that is required by the code, otherwise its
absence will inhibit the code usage; moreover, if it's hardwired you can
rely on the appropriateness of its definition.  Do as you like.

>
>> - the Write Ahead Log should be intended as a temporary solution, since
>> OpenLDAP's slapd will soon make available a native transaction
>> mechanism;
>> as such, I'd prefer to make it very well isolated to allow its
>> replacement
>> (I havent' looked at the code in detail, so maybe it's OK how it is
>> right now).
>
> It's not that hard to remove it and, as a matter of fact, I suggested
> using the transaction mechanism, but I was told that OpenLDAP may serve
> as a proxy to other LDAP server which doesn't have this mechanism, so we
> decided to implement the Wright Ahead Log

OK.

>
>> I note that fflush() is not really a system call, but rather a part of
>> the stdio
>> library made available by the libc.  As such, I'd rather use the
>> sync() system call
>> (note that even sync() does not guarantee data is flushed to the disk;
>> it only
>> schedules the flush before returning!).
>
> well, since they both don't guarantee data consistency I just went with
> fflush. To be honest I don't know the system level details that well, so
> I can't state the clear advantage of sync()
>
>> - it is suggested that a test be added to the test suite; for
>> directions on how
>> to design a test, please feel free to ask, and use existing tests as
>> guidelines.
>
> I will try to make them.

look at ppolicy, unique and refint tests as guidelines.

Sincerely, p.

-- 
Pierangelo Masarati
mailto:pierangelo.masarati@sys-net.it


    SysNet - via Dossi,8 27100 Pavia Tel: +390382573859 Fax: +390382476497