[PATCHES] New DEBUG facility
by Pavel Březina
https://fedorahosted.org/sssd/ticket/925
[PATCH 1/2]
There are several new macros in util/util.h:
- SSSDBG_* to reflect a debug level (same names as in the ticket) - please,
don't use magic numbers anymore
- DEBUG_MSG(level, function, message) which will format the debug message
like "(time) [prg_name] [function] (level): message\n"
- DEBUG_IS_SET(level) that you should use to check if the level is allowed
to be logged
You can use it like: if (DEBUG_IS_SET(SSSDBG_TRACE_LIBS)) {...}
[PATCH 2/2]
Changes unresolved debug level value (SSSDBG_UNRESOLVED) from -1 to 0
so DEBUG macro could be reduced by one condition. Anyway, it has a minor
effect, every time you want to load debug_level from command line parameters,
you have to use following pattern:
/* Set debug level to invalid value so we can deside if -d 0 was used. */
debug_level = SSSDBG_INVALID;
pc = poptGetContext(argv[0], argc, argv, long_options, 0);
while((opt = poptGetNextOpt(pc)) != -1) { ... }
debug_level = debug_level != SSSDBG_INVALID
? debug_convert_old_level(debug_level)
: SSSDBG_UNRESOLVED; /* Debug level should be loaded from config file. */
12 years, 8 months
[PATCH] Improve password policy error code and message
by Sumit Bose
Hi,
if we request e.g. the shadow password policy but a user does not have the
corresponding attributes the following log message is shown:
(Thu Aug 25 06:22:59 2011) [sssd[be[LDAP]]] [find_password_expiration_attributes] (1): No shadow password attributes found, but shadow password policy was requested.
(Thu Aug 25 06:22:59 2011) [sssd[be[LDAP]]] [get_user_dn] (1): find_password_expiration_attributes failed.
(Thu Aug 25 06:22:59 2011) [sssd[be[LDAP]]] [sdap_handle_release] (8): Trace: sh[0x241b610], connected[1], ops[(nil)], ldap[0x241e270], destructor_lock[0], release_memory[0]
(Thu Aug 25 06:22:59 2011) [sssd[be[LDAP]]] [remove_connection_callback] (9): Successfully removed connection callback.
(Thu Aug 25 06:22:59 2011) [sssd[be[LDAP]]] [be_pam_handler_callback] (4): Backend returned: (3, 4, <NULL>) [Internal Error (System error)]
Kaushik mentioned that "Backend returned: (3, 4, <NULL>) [Internal Error
(System error)]" might irritate the admin who tries to find out why the access
was denied. The attached patch changes this to:
(Thu Aug 25 13:06:15 2011) [sssd[be[ldap]]] [find_password_expiration_attributes] (1): No shadow password attributes found, but shadow password policy was requested. Access will be denied.
(Thu Aug 25 13:06:15 2011) [sssd[be[ldap]]] [get_user_dn] (1): find_password_expiration_attributes failed.
(Thu Aug 25 13:06:15 2011) [sssd[be[ldap]]] [sdap_handle_release] (8): Trace: sh[0x87d3b28], connected[1], ops[(nil)], ldap[0x87d4a58], destructor_lock[0], release_memory[0]
(Thu Aug 25 13:06:15 2011) [sssd[be[ldap]]] [remove_connection_callback] (9): Successfully removed connection callback.
(Thu Aug 25 13:06:15 2011) [sssd[be[ldap]]] [be_pam_handler_callback] (4): Backend returned: (0, 6, <NULL>) [Success]
bye,
Sumit
12 years, 8 months
sssd & autofs
by Ondrej Valousek
Hi list,
Quick question - is anyone working on the automounter support yet? I am just curious how it would be done.
I see only one ticket (#900) which suggests support via executable maps, but this solution seems to be quite nasty.
I believe that ideal solution would be via plugin library (something like /usr/lib/autofs/lookup_sssd.so) to the automounter - I am sure Ian
Kent (the autofs maintainer) would help as well.
What do you think?
Thanks,
Ondrej
The information contained in this e-mail and in any attachments is confidential and is designated solely for the attention of the intended recipient(s). If you are not an intended recipient, you must not use, disclose, copy, distribute or retain this e-mail or any part thereof. If you have received this e-mail in error, please notify the sender by return e-mail and delete all copies of this e-mail from your computer system(s).
Please direct any additional queries to: communications(a)s3group.com.
Thank You.
Silicon and Software Systems Limited (S3 Group). Registered in Ireland no. 378073.
Registered Office: South County Business Park, Leopardstown, Dublin 18
12 years, 8 months
better handling of DNS timeouts
by Jakub Hrozek
Hi,
In light of https://bugzilla.redhat.com/show_bug.cgi?id=726467 I have been
thinking about improving how we handle DNS timeouts in general.
Currently there is one timeout option we pass to c-ares. However, that option
is per-nameserver. I guess that makes sense from a resolver library POV - as
a resolver library you want to control how long you talk to each name server.
We have been mostly OK with this because in most situations the resolver
can't connect the socket to the name server at all at times out immediatelly.
Problems arise when the server is very slow to respond, drops packets.
To solve this in SSSD we need to have control over how long a name resolution
takes regardless of the number of name servers and also regardless of the
number of servers in failover.
The failover in SSSD has the concept of "services". LDAP is a service,
Kerberos is a service etc. From back end you don't care and don't know
how many servers there are in a service. From fail over we don't know and
don't care how many name servers there are.
My proposal is to:
1) change the current "dns_resolver_timeout" to be per-service, so the
semantics would be "How long to wait until we get an LDAP server IP address"
for example
2) introduce a new option, something like "dns_resolver_server_timeout",
that would control a per-server timeout. This option could maybe be
undocumented, it seems quite low-level.
3) hide the per-nameserver resolver timeout (only #define it). It is too
low level.
Thoughts, comments and ideas are welcome.
12 years, 8 months
sssd issues
by Ondrej Valousek
Hi list,
Ok here is the list of issues I discovered while configuring sssd against Win2008 AD & rfc2307bis schema:
1. If I specify both dns_discovery_domain and ldap_uri parameters then what happens is that dns srv discovery returns a list of ldap
servers. Now if the first one found is not working, others are not tried. I have to comment out the 'ldap_uri' parameter to make it working
correctly.
2. SSSD is unable to detect default Kerberos realm as per /etc/krb5.conf - I have to configure it manually
3. Why do we actually need to specify Kerberos realm and KDC? Isn't /etc/krb5.conf supposed to record these kind of parameters?
4. authconfig is unable to configure sssd to use IPA backend provider
Thanks,
Ondrej
The information contained in this e-mail and in any attachments is confidential and is designated solely for the attention of the intended recipient(s). If you are not an intended recipient, you must not use, disclose, copy, distribute or retain this e-mail or any part thereof. If you have received this e-mail in error, please notify the sender by return e-mail and delete all copies of this e-mail from your computer system(s).
Please direct any additional queries to: communications(a)s3group.com.
Thank You.
Silicon and Software Systems Limited (S3 Group). Registered in Ireland no. 378073.
Registered Office: South County Business Park, Leopardstown, Dublin 18
12 years, 8 months
Re: [SSSD] THE SUDO rule elimination is over.
by Stephen Gallagher
For those following along, this is a review of the patches in Arun's
public git repository located at:
https://github.com/arunscaria91/sssd.git
On Tue, 2011-08-16 at 22:05 +0530, arun scaria wrote:
>
> Dear sgallagh,
> I'm happy to inform you that the sudo rule elimination is over, Now I
> need to evaluate left out rules as we planned. Check my repo and
> comment through mail. If you can, inform the next meeting time. :)
I have a few review comments to make. Sorry that some of these probably
should have happened earlier, but this is the first chance I've had to
go through this in-depth.
sudo_query_validation():
1) This function is getting far too large. Please separate the D-BUS
message parsing into a separate function that allocates and returns
'struct sss_sudo_msg_contents'.
It should look like:
sudo_query_parse(TALLOC_CTX *mem_ctx,
struct DBusMessage *message,
struct sss_sudo_msg_contents **contents);
and would be invoked like: sudo_query_parse(sudo_cli, message, &msg);
Please also separate the reply message creation into its own function,
in such a way that it can be called for success or failure.
Another point: do not explicitly cast to (TALLOC_CTX *). Internally,
it's just an alias for 'void *' and is safe to implicitly cast
everywhere. Adding the explicit cast just makes the code harder to read.
Please don't use GOTO except to an exit condition. Your use of
'free_ctx' is completely unacceptable. We only ever use GOTO if we're
short-circuiting to exit on failure. As mentioned above, please create a
function to send the reply and call it explicitly for either success or
user not found.
Please always test the returned values from ldb_msg_find_attr_*(),
because if the message did not contain the value, we need to consider it
an error and return a failure reply.
You still have "tom" passed into search_sudo_rules(), where it should be
user_name.
Please don't use malloc() or strdup() directly (as with "PASS"). Use
talloc() and talloc_strdup() instead.
search_sudo_rules():
Do not allocate tmp_ctx on sudocli. In a synchronous function, tmp_ctx
contexts should be allocated on NULL and guaranteed to be maintained
internally. This means that you must always talloc_free(tmp_ctx) before
any return from the function. This provides an easy way to clean up any
internal-only memory, but it should NOT be attached to an external
context. If it's attached to an external context and we forget to free
it before returning in some situations, not only is it a leak, but it's
a leak we cannot detect with tools like valgrind.
Furthermore, functions should not attach to an external context (such as
sudocli) directly. If they are going to allocate memory that is to be
returned, the function MUST take a mem_ctx argument (then the caller of
the function will decide whether sudocli is the appropriate parent for
it).
Don't allocate part of 'filter' first and then call prepare_filter().
You should just pass in tmpctx and user_name to prepare_filter and
create it all in that function.
Don't allocate listctx on NULL. You have a tmpctx already; this is
exactly what it's supposed to be for (so you only have to worry about
calling talloc_free(tmpctx) before returning).
Please don't write your own linked list method. We have our own
implementation already in the SSSD sources. See src/util/dlinklist.h.
You probably want to be using DLIST_FOREACH() in place of your while()
loop.
Your functions initList(), appendNode() etc violate our naming
standards.
There are a bunch of things wrong with list_sss. 1) Do not use typedefs.
2) Don't try to create generic lists. Just create a 'struct
sss_sudo_list' and use the real 'struct ldb_message *" instead of
casting back and forth to (void *). 3) sss_sudo_list should have the
prev and next pointers so it can be used with the DLIST macros.
The comment /* see if this is a user */ is in the wrong place. You're
comparing the command here.
Please make each of the individual comparisons (command, user, host)
into their own functions. search_sudo_rules() is too large and hard to
follow. (Also, add comments)
We need to figure out an alternative to the _SSS_LOOPS issue here (but
this isn't your problem, Arun). We have no choice but to call into glibc
if we want innetgr() to resolve properly. This comment is added so that
if anyone besides Arun has read this far, they should feel free to
comment :)
For users, commands and hosts you need to check for negation "!" rules.
As you're walking through each, if you hit an explicit denial at any
time, you should short-circuit to returning PERMISSION DENIED.
On a related note, this function needs to return the results necessary
to reply to the client. You probably need to construct this as a struct
that you'll pass back to sudo_query_validation() which will in turn pass
it into the reply creation routine I mentioned above.
get_host_name():
Please use "mem_ctx" to make things clear. 'ctx' is too ambiguous and
we're trying to phase it out elsewhere in the code.
Do not call exit(). EVER.
It is much better to have the function return an errno_t and instead
return the hostname as a "char **hostname" parameter to the function.
This function needs comments *desperately*. I really don't have any idea
what you're trying to do, but I'm sure you aren't doing it. Right now,
you're returning a copy of a member of the "p" variable that has never
been initialized. Also, this should just be talloc_strdup(mem_ctx,
p->ai_canonname);
I don't really know why you're calling getaddrinfo() for the "http"
service either.
prepare_filter():
See comment above about passing in user_name.
You also need to handle looking up the explicit negations. So your
search filter needs to include "sudoUser=!username", "sudoUser=!#gid"
and "sudoUser=!+*" (same for hosts).
We need to make sure that if any rule specifically denies this user or
host, it is honored.
Change the name of the variable "res" once it's passed in. It would be
more clear if it was named "groups_res".
As I said above, you can't assume that ldb_msg_find_* will return
non-NULL. You need to check this before you can pass it into
talloc_asprintf_append(). If it's NULL, log an error with DEBUG() and
skip it.
You use a lot of repeated code when checking the filter pieces. Might be
a good idea to just create a macro like FILTER_APPEND_CHECK(). (This is
just a suggestion; there's nothing wrong with the repetition other than
it takes up more space).
However, you're checking whether filter is NULL and then attempting to
print it in the DEBUG statements. That's not going to work :)
sysdb_search_sudo_rules():
As described above, tmpctx should be allocated on NULL, not mem_ctx.
Only the values being returned should ever be allocated or stolen with
talloc_steal() onto the passed-in mem_ctx.
compare_sudo_order():
Correct me if I'm wrong, but won't this be sorting from lowest to
highest? Doesn't sudo require us to process the higher values first?
12 years, 8 months
[PATCH] Handle timeout during sss_ldap_init_send
by Jakub Hrozek
In some cases, where there would be no response from the LDAP server,
there would be no R/W events on the LDAP fd, so
sdap_async_sys_connect_done would never be called.
This patch adds a tevent timer that cancels the connection after
SDAP_NETWORK_TIMEOUT seconds.
A simple testcase is enabling DROP iptables rules on the LDAP server.
12 years, 8 months
[PATCH] Do not delete requests inside hash_iterate loop
by Jakub Hrozek
This patch should solve #932/rhbz #725472, although I have not been able
to reproduce the original crash, so I'm not sure. But it's definitely
something we want to fix as deleting a hash table item from a
hash_iterate visitor callback yields potentially undefined results.
12 years, 8 months