[RFC][PATCH] extending the libsepol API

[RFC][PATCH] extending the libsepol API

[Kevin Carr]

[-- Attachment #1: Type: text/plain, Size: 2162 bytes --]

Hello All,

Some of us at Tresys have been working on changing setools to use libsepol
as our underlying policy representation.  Our basic goals are the following:

1. Provide setools (and other analysis tools) with some of the added
features of libsepol such as modular policy, the new language features, etc 
2. Reduce the redundancy and maintenance required to maintain our own policy
representation 

To make our changes we have been working on some API additions for libsepol.
The attached diff adds some header files that represent how we plan to
export a new shared API for accessing types/attribs.  The patch is the first
of many aimed at fleshing out the shared API.  The diff contains just the
headers as we would like to get some feedback from the community at this
point.

The patch adds two files: "include/iterator.h" and "include/type_query.h".  

We are concerned about consistency in the API and so we intend that the
remaining component accessors will look very similar to the types API.
Ultimately we will need to add access to all other components in the policy
as well as rules and interfaces.  We feel that the iterator concept will
scale well to those functions.

As for the limited number of accessor functions that currently exist,
(users, booleans, etc.) we feel that the current design metaphors don't fit
well with our requirements and we are not sure what to do about them long
term as we are presenting something very different, and as I said we are
concerned about consistency.  There are several long-term options: 1.
Deprecate them in source and keep them for ABI stability, 2. Don't do number
1 ;)

We are looking for constructive feedback, so any comments will be helpful.
We have documented our headers with the doxygen format as we think this is
helpful, we are more than happy to continue or discontinue that format as
work continues, as well as make changes to fit our needs with the rest of
the community.

Again, our goal here is to build something that is more complete in scope
than the current functions, and ultimately found beneficial to the whole
SELinux community.

Kevin Carr
Tresys Technology
410.290.1411 x137


[-- Attachment #2: types-api.patch --]
[-- Type: application/octet-stream, Size: 12055 bytes --]

diff -purN --exclude=.svn trunk/libsepol/include/sepol/iterator.h branch/setools_export-types/libsepol/include/sepol/iterator.h
--- trunk/libsepol/include/sepol/iterator.h	1969-12-31 19:00:00.000000000 -0500
+++ branch/setools_export-types/libsepol/include/sepol/iterator.h	2006-03-12 04:31:26.000000000 -0500
@@ -0,0 +1,86 @@
+/**
+ * @file iterator.h
+ * Defines the public API for sepol_iterator; this structure
+ * is used when requesting lists of components from the policy
+ * database.
+ * 
+ * @author Kevin Carr kcarr@tresys.com
+ * @author Jeremy A. Mowery jmowery@tresys.com
+ * @author Jason Tang jtang@tresys.com
+ *
+ * Copyright (C) 2006 Tresys Technology, LLC
+ *
+ *  This library is free software; you can redistribute it and/or
+ *  modify it under the terms of the GNU Lesser General Public
+ *  License as published by the Free Software Foundation; either
+ *  version 2.1 of the License, or (at your option) any later version.
+ *
+ *  This library is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ *  Lesser General Public License for more details.
+ *
+ *  You should have received a copy of the GNU Lesser General Public
+ *  License along with this library; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ */
+
+#ifndef SEPOL_ITERATOR_H
+#define SEPOL_ITERATOR_H
+
+#include <stddef.h>
+
+struct sepol_iterator;
+typedef struct sepol_iterator sepol_iterator_t;
+
+/**
+ *  Free memory used by the iterator.
+ *  @param iter Pointer to the iterator to be freed; frees all
+ *  memory used by the iterator and the iterator itself. On returning
+ *  *iter will be NULL.
+ */
+extern void sepol_iterator_destroy(sepol_iterator_t **iter);
+
+/**
+ *  Get the item at the current position of the iterator.
+ *  @param iter The iterator from which to get the item.
+ *  @param item Pointer in which to store the current item; the caller is 
+ *  responsible for safely casting this pointer. Unless specifically
+ *  noted by the function creating the iterator, the item set 
+ *  by this function should not be freed. If the iterator is at 
+ *  the end (i.e. all items have been traversed) *item will be NULL.
+ *  @return Returns 0 on success and < 0 on failure; if the call fails,
+ *  errno will be set and *item will be NULL.
+ */
+extern int sepol_iterator_get_item(sepol_iterator_t *iter, void **item);
+
+/**
+ *  Advance the iterator to the next item.
+ *  @param iter The iterator to advance; internal state data will change.
+ *  @return Returns 0 on success and < 0 on failure; advancing an
+ *  iterator that is at the end fails (and returns < 0). If the call fails,
+ *  errno will be set.
+ */
+extern int sepol_iterator_next(sepol_iterator_t *iter);
+
+/**
+ *  Determine if an iterator is at the end.
+ *  @param iter The iterator to check.
+ *  @return Returns non-zero if the current position of the iterator
+ *  is at the end of the list (i.e. past the last valid item) and
+ *  zero in any other case. If there is an error determining if
+ *  the iterator is at the end then non-zero will be returned.
+ */
+extern int sepol_iterator_is_end(sepol_iterator_t *iter);
+
+/**
+ *  Get the total number of items in the list stored in the iterator.
+ *  @param iter The iterator from which to get the number of items.
+ *  @param size Pointer in which to store the number of items. 
+ *  Must be non-NULL.
+ *  @return Returns 0 on success and < 0 on failure; if the call fails,
+ *  errno will be set and *size will be 0.
+ */
+extern int sepol_iterator_get_size(sepol_iterator_t *iter, size_t *size);
+
+#endif /* SEPOL_ITERATOR */
diff -purN --exclude=.svn trunk/libsepol/include/sepol/type_query.h branch/setools_export-types/libsepol/include/sepol/type_query.h
--- trunk/libsepol/include/sepol/type_query.h	1969-12-31 19:00:00.000000000 -0500
+++ branch/setools_export-types/libsepol/include/sepol/type_query.h	2006-03-12 04:31:39.000000000 -0500
@@ -0,0 +1,159 @@
+ /**
+ *  @file type_query.h
+ *  Defines the public interface for searching and iterating over types. 
+ *
+ *  @author Kevin Carr kcarr@tresys.com
+ *  @author Jeremy A. Mowery jmowery@tresys.com
+ *  @author Jason Tang jtang@tresys.com
+ *
+ *  Copyright (C) 2006 Tresys Technology, LLC
+ *
+ *  This library is free software; you can redistribute it and/or
+ *  modify it under the terms of the GNU Lesser General Public
+ *  License as published by the Free Software Foundation; either
+ *  version 2.1 of the License, or (at your option) any later version.
+ *
+ *  This library is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ *  Lesser General Public License for more details.
+ *
+ *  You should have received a copy of the GNU Lesser General Public
+ *  License along with this library; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ */
+
+#ifndef SEPOL_TYPE_QUERY_H
+#define SEPOL_TYPE_QUERY_H
+
+#include <stddef.h>
+#include <stdint.h>
+#include <sepol/handle.h>
+#include <sepol/policydb.h>
+#include <sepol/iterator.h>
+
+typedef struct sepol_type_datum sepol_type_datum_t;
+
+/**
+ *  Get the datum for a type by name.
+ *  @param handle Error handler for the policy database.
+ *  @param policy The policy from which to get the type.
+ *  @param name The name of the type; searching is case sensitive.
+ *  @param datum Pointer in which to store the type datum; the caller
+ *  should not free this pointer.
+ *  @return Returns 0 on success and < 0 on failure; if the call fails,
+ *  errno will be set and *datum will be NULL.
+ */
+extern int sepol_policydb_get_type_by_name(sepol_handle_t *handle, sepol_policydb_t *policy, const char *name, sepol_type_datum_t **datum);
+
+/**
+ *  Get an iterator for types declared in the policy.
+ *  @param handle Error handler for the policy database.
+ *  @param policy The policy from which to create the iterator.
+ *  @param iter Iterator of type sepol_type_datum_t* returned; 
+ *  the caller is responsible for calling sepol_iterator_destroy() to 
+ *  free memory used; it is important to note that the iterator is 
+ *  valid only as long as the policy is unchanged.
+ *  @return Returns 0 on success and < 0 on failure; if the call fails,
+ *  errno will be set and *iter will be NULL.
+ */
+extern int sepol_policydb_get_type_iter(sepol_handle_t *handle, sepol_policydb_t *policy, sepol_iterator_t **iter);
+
+/**
+ *  Get the integer value associated with a type. Values range from 1 
+ *  to the number of types declared in the policy.
+ *  @param handle Error handler for the policy database.
+ *  @param policy The policy associated with the type.
+ *  @param datum The type from which to get the value.
+ *  @param value Pointer to the integer in which to store value.
+ *  @return Returns 0 on success and < 0 on failure; if the call fails,
+ *  errno will be set and value will be 0.
+ */
+extern int sepol_type_datum_get_value(sepol_handle_t *handle, sepol_policydb_t *policy, sepol_type_datum_t *datum, uint32_t *value);
+
+/**
+ *  Determine whether a given type is an alias for another type.
+ *  @param handle Error handler for the policy database.
+ *  @param policy The policy associated with the type.
+ *  @param datum The type to check.
+ *  @param isalias Pointer to be set to 1 (true) if the type is an alias
+ *  and 0 (false) otherwise.
+ *  @return Returns 0 on success and < 0 on failure; if the call fails,
+ *  errno will be set and *isalias will be 0 (false).
+ */
+extern int sepol_type_datum_get_isalias(sepol_handle_t *handle, sepol_policydb_t *policy, sepol_type_datum_t *datum, unsigned char *isalias);
+
+/**
+ *  Determine whether a given type is an attribute.
+ *  @param handle Error handler for the policy database.
+ *  @param policy The policy associated with the type.
+ *  @param datum The type to check.
+ *  @param isattr Pointer to be set to 1 (true) if the type is an
+ *  attribute and 0 (false) otherwise.
+ *  @return Returns 0 on success and < 0 on failure; if the call fails,
+ *  errno will be set and *isattr will be 0 (false).
+ */
+extern int sepol_type_datum_get_isattr(sepol_handle_t *handle, sepol_policydb_t *policy, sepol_type_datum_t *datum, unsigned char *isattr);
+
+/**
+ *  Get an iterator for the list of types in an attribute.
+ *  @param handle Error handler for the policy database.
+ *  @param policy The policy associated with the attribute.
+ *  @param datum The attribute from which to get the types.
+ *  @param types Iterator of type sepol_type_datum_t* returned; 
+ *  the caller is responsible for calling sepol_iterator_destroy() to 
+ *  free memory used; it is important to note that the iterator is 
+ *  valid only as long as the policy is unchanged.
+ *  @return Returns 0 on success, > 0 if the type is not an attribute
+ *  and < 0 on failure; if the call fails, errno will be set and
+ *  *types will be NULL. If the type is not an attribute *types will
+ *  be NUll.
+ */
+extern int sepol_type_datum_get_type_iter(sepol_handle_t *handle, sepol_policydb_t *policy, sepol_type_datum_t *datum, sepol_iterator_t **types);
+
+
+/**
+ *  Get an iterator for the list of attributes given to a type.
+ *  @param handle Error handler for the policy database.
+ *  @param policy The policy associated with the type.
+ *  @param datum The type for which to get the attributes.
+ *  @param attrs Iterator of type sepol_type_datum_t* returned; 
+ *  the caller is responsible for calling sepol_iterator_destroy() to 
+ *  free memory used; it is important to note that the iterator is 
+ *  valid only as long as the policy is unchanged.
+ *  @return Returns 0 on success, > 0 if the type is an attribute
+ *  and < 0 on failure; if the call fails, errno will be set and
+ *  *types will be NULL. If the type is an attribute *types will
+ *  be NUll.
+ */
+extern int sepol_type_datum_get_attr_iter(sepol_handle_t *handle, sepol_policydb_t *policy, sepol_type_datum_t *datum, sepol_iterator_t **attrs);
+
+/**
+ *  Get the name by which a type is identified from its datum.
+ *  @param handle Error handler for the policy database.
+ *  @param policy The policy associated with the type.
+ *  @param datum The type for which to get the name.
+ *  @param name Pointer in which to store the name; the caller 
+ *  should not free the string. If the type is an alias then the
+ *  primary name will be returned.
+ *  @return Returns 0 on success and < 0 on failure; if the call fails,
+ *  errno will be set and *name will be NULL.
+ */
+extern int sepol_type_datum_get_name(sepol_handle_t *handle, sepol_policydb_t *policy, sepol_type_datum_t *datum, char **name);
+
+/**
+ *  Get an iterator for the list of aliases for a type.
+ *  @param handle Error handler for the policy database.
+ *  @param policy The policy associated with the type.
+ *  @param datum The type for which to get aliases.
+ *  @param aliases Iterator of type char* returned; the caller is 
+ *  responsible for calling sepol_iterator_destroy() to free
+ *  memory used; it is important to note that the iterator is valid
+ *  only as long as the policy is unchanged. If a type has no aliases,
+ *  the iterator will be at end and have size 0.
+ *  @return Returns 0 on success and < 0 on failure; if the call fails,
+ *  errno will be set and *aliases will be NULL.
+ */
+extern int sepol_type_datum_get_alias_iter(sepol_handle_t *handle, sepol_policydb_t *policy, sepol_type_datum_t *datum, sepol_iterator_t **aliases);
+
+#endif /* SEPOL_TYPE_QUERY_H */

Re: [RFC][PATCH] extending the libsepol API

[Stephen Smalley]

On Tue, 2006-03-21 at 17:46 -0500, Kevin Carr wrote:
> Hello All,
> 
> Some of us at Tresys have been working on changing setools to use libsepol
> as our underlying policy representation.  Our basic goals are the following:
> 
> 1. Provide setools (and other analysis tools) with some of the added
> features of libsepol such as modular policy, the new language features, etc 
> 2. Reduce the redundancy and maintenance required to maintain our own policy
> representation 
> 
> To make our changes we have been working on some API additions for libsepol.
> The attached diff adds some header files that represent how we plan to
> export a new shared API for accessing types/attribs.  The patch is the first
> of many aimed at fleshing out the shared API.  The diff contains just the
> headers as we would like to get some feedback from the community at this
> point.
> 
> The patch adds two files: "include/iterator.h" and "include/type_query.h".  
> 
> We are concerned about consistency in the API and so we intend that the
> remaining component accessors will look very similar to the types API.
> Ultimately we will need to add access to all other components in the policy
> as well as rules and interfaces.  We feel that the iterator concept will
> scale well to those functions.
> 
> As for the limited number of accessor functions that currently exist,
> (users, booleans, etc.) we feel that the current design metaphors don't fit
> well with our requirements and we are not sure what to do about them long
> term as we are presenting something very different, and as I said we are
> concerned about consistency.  There are several long-term options: 1.
> Deprecate them in source and keep them for ABI stability, 2. Don't do number
> 1 ;)

Can you clarify why you feel that the current APIs don't meet your
requirements?

-- 
Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

RE: [RFC][PATCH] extending the libsepol API

[Kevin Carr]

> > As for the limited number of accessor functions that currently exist,
> > (users, booleans, etc.) we feel that the current design metaphors don't
> fit
> > well with our requirements and we are not sure what to do about them
> long
> > term as we are presenting something very different, and as I said we are
> > concerned about consistency.  There are several long-term options: 1.
> > Deprecate them in source and keep them for ABI stability, 2. Don't do
> number
> > 1 ;)
> 
> Can you clarify why you feel that the current APIs don't meet your
> requirements?

We originally started with the current way of creating opaque keys from
string values and found that the key concept did not scale very well when we
get to accessing rules.  For example, if a type is subtracted in a rule,
does that type appear in the key?  One of our main goals is to make the API
as consistent as possible so we didn't want to try and force the key concept
across the rest of the API as it didn't seem to fit.

In the current implementation there is an issue of who owns the memory in a
key.  The key_create() functions just copy the pointer and not the contents
of the string, so a key 'object' can live longer than the string used to
create it and thus is easily misused by users of the library.  Additionally,
we thought the memory and programming overhead involved in creating, and
destroying the keys was undesirable and not necessary.  For example when we
start iterating over all the types in all the rules of the policy, using
keys can get expensive.  

Kevin Carr
Tresys Technology
410.290.1411 x137



--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

RE: [RFC][PATCH] extending the libsepol API

[Kevin Carr]

I am bringing this discussion back on list so Ivan can join in when he
returns.

> On Thu, 2006-03-23 at 08:52 -0500, Kevin Carr wrote:
> > We can't use the key model elsewhere in the library, so moving forward
> it
> > can no longer be something that is consistent across the entire API.
> 
> It isn't clear to me that the key model can't be used, just that you
> don't want to use it.  In any event, that discussion should go back on
> list.  I took my reply off list since I don't think we'll make much
> headway here until Ivan returns.

Keys are not sufficient to help us with rule searches.  Imagine that we want
to find all the rules that have a certain subset of permissions.  The key
would have the source, target, and class right?  Thus we cannot construct a
key for what we want.  What we really need is a way to access all the rules
independent of some search criteria so that we can do complex rule matching
outside of libsepol.

> 
> > Also iterators can be used consistently across the
> > entire API and every usage is the same as far as the user is concerned.
> 
> You still have to know what kind of object you are processing, so what
> does this buy you?

Yes, of course the user will have to know what type of object he's
accessing, this is the only thing that's ever different when using
iterators.

> > I understand this has been discussed in the past but I think it needs to
> be
> > discussed again as we are talking about building a much more extensive
> API.
> > We need to add a lot in order to integrate with setools and this is the
> best
> > time to fix things that were done incorrectly in the past especially
> because
> > we are talking about a relatively small amount of code to replace.  We
> are
> > prepared to make the appropriate changes.
> 
> It is much easier to invent your own API than to try to work within the
> constraints of the existing one - I understand that.  But it isn't good
> practice, and it creates a not-inaccurate perception that SELinux is
> never stabilizing, always in flux, never presenting a stable interface
> for users and application developers.  Constant churn.

I agree that SELinux needs to become more stable and I believe that is what
we are trying to do right now.

> > We agree 100% that tools compiled for the old functions need to continue
> > working correctly.  We want to replace the current functions with
> something
> > better but leave the old ones around for compatibility and leave them in
> the
> > ABI.
> 
> Which becomes a) a maintenance nightmare - keeping the old functions
> working forever even as we rework the internals over time and stop using
> them in our own code, b) a source of bloat in the library, and c) a
> source of confusion to users who are presented with a confusing array of
> different APIs.  Much better if we can just avoid the need to deprecate
> the existing functions in the first place.

a) I think the maintenance will be minimal.  An option might be to
re-implement the existing functions, by using the new ones.  Then just
maintaining the new ones would suffice indefinitely.  b) I don't think that
there is ever a reason to keep the functions in the library if no one is
using them, in that long-term case, they should be removed.  c) I don't
think this will cause any confusion to developers as anything that is
deprecated should be clearly marked as such, either in the documentation or
with attributes in the source.

Kevin Carr
Tresys Technology
410.290.1411 x137



--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

RE: [RFC][PATCH] extending the libsepol API

[Stephen Smalley]

On Thu, 2006-03-23 at 14:23 -0500, Kevin Carr wrote:
> Keys are not sufficient to help us with rule searches.  Imagine that we want
> to find all the rules that have a certain subset of permissions.  The key
> would have the source, target, and class right?  Thus we cannot construct a
> key for what we want.  What we really need is a way to access all the rules
> independent of some search criteria so that we can do complex rule matching
> outside of libsepol.

For which you can use the sepol_<record>_iterate() interfaces, right?
But that doesn't mean that no user of the library will want to use
key-based interfaces for lookups, ala an encapsulated form of the
avtab_search interface, or that we shouldn't provide such interfaces.

> Yes, of course the user will have to know what type of object he's
> accessing, this is the only thing that's ever different when using
> iterators.

The actual processing that is occurring during the iteration is
object-specific, so why is it a problem to have object-specific iterate
APIs?

> I agree that SELinux needs to become more stable and I believe that is what
> we are trying to do right now.

No, you don't understand.  There is a stable ABI for libsepol right now.
You are suggesting deprecating a large chunk of it in favor of your own
model.  The burden of proof that such deprecation is justified is on
you, and it needs to be more than just "our proposed API is better
suited to our particular needs".

> a) I think the maintenance will be minimal.  An option might be to
> re-implement the existing functions, by using the new ones.  Then just
> maintaining the new ones would suffice indefinitely.  b) I don't think that
> there is ever a reason to keep the functions in the library if no one is
> using them, in that long-term case, they should be removed.  c) I don't
> think this will cause any confusion to developers as anything that is
> deprecated should be clearly marked as such, either in the documentation or
> with attributes in the source.

Removing the interface once it has gone into a stable OS release (e.g.
FC5) is very bad form.  And we have to plan that people developing for
the next generation of distribution releases are looking at the current
APIs as their basis.  

-- 
Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: [RFC][PATCH] extending the libsepol API

[Stephen Smalley]

On Tue, 2006-03-21 at 17:46 -0500, Kevin Carr wrote:
> To make our changes we have been working on some API additions for libsepol.
> The attached diff adds some header files that represent how we plan to
> export a new shared API for accessing types/attribs.  The patch is the first
> of many aimed at fleshing out the shared API.  The diff contains just the
> headers as we would like to get some feedback from the community at this
> point.
> 
> The patch adds two files: "include/iterator.h" and "include/type_query.h".  

Aside from concerns about not introducing an orthogonal set of APIs into
libsepol, other comments:

diff -purN --exclude=.svn trunk/libsepol/include/sepol/type_query.h branch/setools_export-types/libsepol/include/sepol/type_query.h
--- trunk/libsepol/include/sepol/type_query.h	1969-12-31 19:00:00.000000000 -0500
+++ branch/setools_export-types/libsepol/include/sepol/type_query.h	2006-03-12 04:31:39.000000000 -0500
@@ -0,0 +1,159 @@
<snip>
+#include <stddef.h>
+#include <stdint.h>
+#include <sepol/handle.h>
+#include <sepol/policydb.h>
+#include <sepol/iterator.h>
+
+typedef struct sepol_type_datum sepol_type_datum_t;

Just struct sepol_type / sepol_type_t would seem sufficient.
datum doesn't add anything here; an unfortunate choice in the internal
headers that we don't to export to the stable interface.

+extern int sepol_policydb_get_type_by_name(sepol_handle_t *handle, sepol_policydb_t *policy, const char *name, sepol_type_datum_t **datum);

Rather verbose.  How about sepol_type_query() instead, like the existing
APIs.  And mark the policydb arguments as const for the read-only
interfaces.

+extern int sepol_type_datum_get_value(sepol_handle_t *handle, sepol_policydb_t *policy, sepol_type_datum_t *datum, uint32_t *value);

Likewise => sepol_type_get_value().  Ditto for other interfaces.

+extern int sepol_type_datum_get_name(sepol_handle_t *handle, sepol_policydb_t *policy, sepol_type_datum_t *datum, char **name);

Seems strange to query the datum for the name.

-- 
Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

RE: [RFC][PATCH] extending the libsepol API

[Stephen Smalley]

On Thu, 2006-03-23 at 14:58 -0500, Stephen Smalley wrote:
> No, you don't understand.  There is a stable ABI for libsepol right now.
> You are suggesting deprecating a large chunk of it in favor of your own
> model.  The burden of proof that such deprecation is justified is on
> you, and it needs to be more than just "our proposed API is better
> suited to our particular needs".
<snip>
> Removing the interface once it has gone into a stable OS release (e.g.
> FC5) is very bad form.  And we have to plan that people developing for
> the next generation of distribution releases are looking at the current
> APIs as their basis.  

And to make this a bit stronger:  Even if you can justify the need for
additional interfaces for your specialized needs, that doesn't justify a
wholesale departure from the existing APIs, and your goal should be to
minimize the delta to only what is truly required, making your new
interfaces complement and extend the existing ones rather than replacing
them altogether.  If there are defects in the current interface, then
work on incrementally improving it.  No rewrites, please.

-- 
Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

RE: [RFC][PATCH] extending the libsepol API

[Kevin Carr]

> On Thu, 2006-03-23 at 14:23 -0500, Kevin Carr wrote:
> > Keys are not sufficient to help us with rule searches.  Imagine that we
> want
> > to find all the rules that have a certain subset of permissions.  The
> key
> > would have the source, target, and class right?  Thus we cannot
> construct a
> > key for what we want.  What we really need is a way to access all the
> rules
> > independent of some search criteria so that we can do complex rule
> matching
> > outside of libsepol.
> 
> For which you can use the sepol_<record>_iterate() interfaces, right?

We are concerned about short-term stability of API as you are, but we feel
that the long-term viability of the API is even more important.  Besides the
fact that the iterate callbacks are simply annoying to use, imagine a user
down the line wants a swig wrapper for libsepol.  Function pointers are a
bad idea to have in the API. 

> But that doesn't mean that no user of the library will want to use
> key-based interfaces for lookups, ala an encapsulated form of the
> avtab_search interface, or that we shouldn't provide such interfaces.

A key cannot be constructed for a rule that will return a single record.
The key would contain all parts of the rule.  This breaks the metaphor of
keys and records.  This is not a policy "database" with generic keys and
records, it is a concrete policy representation.

> > Yes, of course the user will have to know what type of object he's
> > accessing, this is the only thing that's ever different when using
> > iterators.
> 
> The actual processing that is occurring during the iteration is
> object-specific, so why is it a problem to have object-specific iterate
> APIs?
> 
> > I agree that SELinux needs to become more stable and I believe that is
> what
> > we are trying to do right now.
> 
> No, you don't understand.  There is a stable ABI for libsepol right now.
> You are suggesting deprecating a large chunk of it in favor of your own
> model.  The burden of proof that such deprecation is justified is on
> you, and it needs to be more than just "our proposed API is better
> suited to our particular needs".

We are concerned about the needs of a general purpose policy representation
library for SELinux.  This should represent the interests of the community.
We are in a unique position because we write analysis tools that inspect the
entire policy representation. In doing so, we have a very good idea about
what this API should contain.

> > a) I think the maintenance will be minimal.  An option might be to
> > re-implement the existing functions, by using the new ones.  Then just
> > maintaining the new ones would suffice indefinitely.  b) I don't think
> that
> > there is ever a reason to keep the functions in the library if no one is
> > using them, in that long-term case, they should be removed.  c) I don't
> > think this will cause any confusion to developers as anything that is
> > deprecated should be clearly marked as such, either in the documentation
> or
> > with attributes in the source.
> 
> Removing the interface once it has gone into a stable OS release (e.g.
> FC5) is very bad form.  And we have to plan that people developing for
> the next generation of distribution releases are looking at the current
> APIs as their basis.

As I said, reimplementing these functions with new ones will allow these
functions to continue working.  I understand that the ultimate decision
resides with the maintainer of libsepol, however it is important to express
that the issues with the current API will continue to plaque developers of
all tools that need a good representation library for SELinux.  Long-term
viability has to be considered.

Kevin Carr
Tresys Technology
410.290.1411 x137



--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: [RFC][PATCH] extending the libsepol API

[Ivan Gyurdiev]

> the caller is 
> + *  responsible for safely casting this pointer. 

Do you plan to iterate over opaque objects, such as the current record 
implementations?

 > + *  errno will be set and *item will be NULL.

Elsewhere we use a callback to communicate an error condition with the 
caller.
What's the advantage of errno over pre-defined return codes?

+extern int sepol_iterator_get_size(sepol_iterator_t *iter, size_t *size);

Shouldn't this be "unsigned int" instead?


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: [RFC][PATCH] extending the libsepol API

[Ivan Gyurdiev]

> imagine a user
> down the line wants a swig wrapper for libsepol.  Function pointers are a
> bad idea to have in the API.
Is it fundamentally impossible to write a swig wrapper for a function 
pointer?
I'm not so sure about that...

>> But that doesn't mean that no user of the library will want to use
>> key-based interfaces for lookups, ala an encapsulated form of the
>> avtab_search interface, or that we shouldn't provide such interfaces.
>>     
>
> A key cannot be constructed for a rule that will return a single record.
> The key would contain all parts of the rule. 
That depends on your record representation and what you're trying to 
accomplish. Why can't a key be a (src_type, target_type, target_class) 
triple, with the different access vectors being part of the data? What 
do you imagine a "rule record" will look like?

The point of the key is to uniquely identify an object, for the purpose 
of add/remove/modify -type management interface. Maybe such an interface 
doesn't make sense in the case of rules - I'm not sure of what 
higher-level goals you have. When I was thinking about writing a "rule 
record" my goal was to support small changes to policy that users might 
want to do - that was later superceded by loadable modules, so I didn't 
have to worry about rule records - the module became the record instead, 
as the unit of management.

> As I said, reimplementing these functions with new ones will allow these
> functions to continue working.
Which functions exactly are being discussed for deprecation? You want to 
replace the _iterate functions with an explicit iterator object?
Does it really matter which one is chosen - they do pretty much the same 
thing, don't they?


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: [RFC][PATCH] extending the libsepol API

[Ivan Gyurdiev]

>>> Also iterators can be used consistently across the
>>> entire API and every usage is the same as far as the user is concerned.
>>>       
So, that's essentially an "Iterable" interface that you want to create, 
which is implemented by all records.
I like OOP ideas, and inheritance/interfaces... otoh I don't like void* 
and casting from it.

I think OOP ideas help when you want to put multiple object types in the 
same framework - is this necessary here?
I don't think it increases readability otherwise.

By the way, this argument extends to the dbase interface in libsemanage.

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: [RFC][PATCH] extending the libsepol API

[Ivan Gyurdiev]

> In the current implementation there is an issue of who owns the memory in a
> key.  The key_create() functions just copy the pointer and not the contents
> of the string, so a key 'object' can live longer than the string used to
> create it and thus is easily misused by users of the library.  
Yes, that was probably a design mistake... anyway, the keys are 
record-specific, so I'm not sure it's necessary to follow the same 
pattern for future keys [ except maybe for consistency ].

> Additionally,
> we thought the memory and programming overhead involved in creating, and
> destroying the keys was undesirable and not necessary.  For example when we
> start iterating over all the types in all the rules of the policy, using
> keys can get expensive.  
>   
A key is used to locate data. Iteration does not involve locating data [ 
at least the current implementation loops over all the data in order ], 
so no keys are used anywhere. There is overhead involved in converting 
from base representation to an opaque record, and that's the price for 
encapsulation / policy independence. You can probably get rid of the 
overhead while keeping encapsulation [ with smarter record 
implementation]... but independence of the record from policy seems to 
require copying/expanding the data.

I think the existing records would best stay independent from policy, 
esp. if they'll be serialized to go over the wire for the policy server. 
New records could work differently...

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.