[PATCH 2/2] s4-openldap: Optionally compile libcli/security as a public library to be used by openldap

[Nadezhda Ivanova]

Hi Jelmer,
It is not the final version at this point, I just think it will be more
convenient to have it in master so other people (mostly Howard:)) will not
have to work with wip branches and merge. I can't think of a reason why it
cannot be made the default way later, when it is better tested is not
expected to change. Currently as it is not the default, it will not really
affect samba without OpenLDAP.

Regards,
Nadya


On Thu, Dec 12, 2013 at 7:25 PM, Jelmer Vernooij <jelmer at samba.org> wrote:

> Hi Nadezhda,
>
> On Thu, Dec 12, 2013 at 06:57:44PM +0200, Nadezhda Ivanova wrote:
> > A continuation of the previous patch, this one adds an option to the
> > configure script to compile libcli/security as a library that can be used
> > by OpenLdap. This is not the final version of the interface, more
> functions
> > will be added as the access checks are posted to OpenLDAP.
>
> > From 5f789901941f29a1ddfa444bfcdc5f824e21860f Mon Sep 17 00:00:00 2001
> > From: Nadezhda Ivanova <nivanova at symas.com>
> > Date: Thu, 12 Dec 2013 18:40:41 +0200
> > Subject: [PATCH 2/2] s4-openldap: Optionally compile libcli/security as a
> >  public library to be used by openldap
> Mostly looks good to me, some minor comments below:
>
> > diff --git a/libcli/security/samba_security.h
> b/libcli/security/samba_security.h
> > new file mode 100644
> > index 0000000..880cc32
> > --- /dev/null
> > +++ b/libcli/security/samba_security.h
>
> samba_security.h is a very generic name; it looks like this is
> create_descriptor.h?
>
> Perhaps add a separate header file that includes
> <security/create_descriptor.h> and possibly other future public header
> files from libsamba-security.
>
> > @@ -0,0 +1,32 @@
> > +#ifndef _LIBCLI_SAMBA_SECURITY_SECURITY_H_
> > +#define _LIBCLI_SAMBA_SECURITY_SECURITY_H_
> > +#include <stdbool.h>
> > +#include <stdint.h>
> > +#include <talloc.h>
> > +#include "lib/util/data_blob.h"
> > +
> > +typedef enum {
> > +     CR_DESCR_INVALID = 0,
> > +     CR_DESCR_SCHEMA,
> > +     CR_DESCR_CONFIG,
> > +     CR_DESCR_DEFAULT,
> > +     CR_DESCR_OTHER
> > +} CR_DESCR_PARTITION;
> > +
> > +DATA_BLOB *cr_descr_get_descriptor_to_show(TALLOC_CTX *mem_ctx,
> > +                                        const DATA_BLOB *sd,
> > +                                        uint32_t sd_flags);
> > +
> > +DATA_BLOB *cr_descr_get_new_descriptor_as_blob(TALLOC_CTX *mem_ctx,
> > +                                            const DATA_BLOB *blob_token,
> > +                                            const DATA_BLOB
> *blob_domain_sid,
> > +                                            const char
> *defaultSecurityDescriptor,
> > +                                            const DATA_BLOB
> *schemaIDGUID,
> > +                                            const DATA_BLOB *parent,
> > +                                            const DATA_BLOB *object,
> > +                                            const DATA_BLOB *old_sd,
> > +                                            CR_DESCR_PARTITION
> partition,
> > +                                            uint32_t sd_flags);
> > +
> > +
> > +#endif
>
> > diff --git a/libcli/security/security.pc.in b/libcli/security/
> security.pc.in
> > new file mode 100644
> > index 0000000..2c33d67
> > --- /dev/null
> > +++ b/libcli/security/security.pc.in
> ^ This should be samba-security.pc.in, as it's going to be installed in
> the
> system pkg-config path on distributions, where the name 'security' is
> fairly
> ambiguous.
>
> > @@ -0,0 +1,10 @@
> > +prefix=@prefix@
> > +exec_prefix=@exec_prefix@
> > +libdir=@libdir@
> > +includedir=@includedir@
> > +
> > +Name: samba-security
> > +Description: A collection of samba access check functions
> This description doesn't seem to cover everything that is in
> libsamba-security.so,
> only what is in the current header file.
>
> > +Version: @PACKAGE_VERSION@
> > +Libs: @LIB_RPATH@ -L${libdir} -lsamba-security
> > +Cflags: -I${includedir}
> > \ No newline at end of file
> > diff --git a/libcli/security/wscript_build
> b/libcli/security/wscript_build
> > index b529ec8..7f20902 100644
> > --- a/libcli/security/wscript_build
> > +++ b/libcli/security/wscript_build
> > @@ -1,11 +1,28 @@
> >  #!/usr/bin/env python
> >
> > +VERSION = '0.0.1'
> > +if bld.CONFIG_SET('AD_OPENLDAP_DEPS_ENABLED'):
> > +    vnum = VERSION
> > +    private_library=False
> style nit: please add spaces around = in assignments.
>
> > +    pc_files='security.pc'
> > +    includes='include'
> > +    public_headers='samba_security.h'
> > +else:
> > +    vnum = None
> > +    private_library=True
> > +    pc_files=None
> > +    includes=''
> > +    public_headers=None
> Building samba-security as a public library optionally is going to
> increase the number of
> configurations in which Samba can be found. Is there a particular reason
> for not wanting this
> as a public library in some cases?
>
> Cheers,
>
> Jelmer
>