[Freeipa-devel] [PATCH] slapi-nis support for trusted domains

[Nalin Dahyabhai]

On Wed, Jul 31, 2013 at 03:53:21PM +0300, Alexander Bokovoy wrote:
> Authentication is handled for both IPA and trusted domain users. The
> former case requires some specific handling of the SLAPI_BIND_TARGET_SDN
> to rewrite it to the original entry's DN. As result successful bind
> looks like this in the dirsrv logs:
> [31/Jul/2013:15:49:03 +0300] conn=15 fd=79 slot=79 SSL connection from 192.168.111.216 to 192.168.111.216
> [31/Jul/2013:15:49:03 +0300] conn=15 SSL 256-bit AES
> [31/Jul/2013:15:49:03 +0300] conn=15 op=0 BIND dn="uid=admin,cn=users,cn=compat,dc=example,dc=com" method=128 version=3
> [31/Jul/2013:15:49:03 +0300] conn=15 op=1 SRCH base="dc=example,dc=com" scope=2 filter="(uid=foobar)" attrs=ALL
> [31/Jul/2013:15:49:03 +0300] conn=15 op=0 RESULT err=0 tag=97 nentries=0 etime=0 dn="uid=admin,cn=users,cn=accounts,dc=example,dc=com"

The RESULT dn being different from the BIND dn is easy to miss, but it
should prevent possible problems where a given user can be bound to more
than one DN, depending on where they started, so I guess it's fine.

On to specific patches:
HEAD~11 looks fine.
HEAD~10:
* Add internal whitespace when computing the value to pass to
  slapi_ch_malloc().
* Break the declaration and initialization of "str" into two lines.
* Use '\0' to terminate str instead of 0.
* In the new comment in format.h, NULL should be NUL.
HEAD~9 looks fine.
HEAD~8:
* In configure.in, drop the hunk that changes the version number that we
  pass to AC_INIT.
* In configure.in, one test compares x$use_sss_nss_idmap != xno, while
  one shortly after compares $use_sss_nss_idmap = yes.  If
  $use_sss_nss_idmap can be empty, then the second test needs to be
  ready for that.
* The help text still refers to SSSD specifically, when the code doesn't
  enforce or guarantee that SSSD's involved when performing nsswitch
  lookups or PAM authentication.
HEAD~7:
* Fix formatting of wrapped lines when calling backend_shr_get_vattr_str().
* Check for errors when converting the configured sssd_min_id to a number
  by switching from atol() to strtol() or strtoul().
HEAD~6:
* Drop extra whitespace after the type of the "name" member when
  defining struct backend_search_filter_config.
* backend_search_filter_has_cn_uid() allocates config->name using
  slapi_ch_malloc(), but it is later freed by free().
* backend_retrieve_user_entry_from_sssd(): fix line wrapping in
  function signature.
* backend_retrieve_user_entry_from_sssd() needs to handle ERANGE errors.
* backend_retrieve_user_entry_from_sssd(): avoid using
  slapi_entry_attr_set_int(), which will treat an unsigned 32-bit
  with the high bit set as a negative value
* backend_retrieve_user_entry_from_sssd(): check for zero-length
  pw_shell value, and avoid adding it as a loginShell value, since
  that'd be invalid
* backend_retrieve_user_entry_from_sssd() adds "ipaNTUserAttrs" as an
  objectClass in the temporary entry, but the value isn't copied into
  the cache.  What purpose does it serve?
* backend_retrieve_user_entry_from_sssd(): extra whitespace when
  constructing the new entry's DN.
* backend_retrieve_user_entry_from_sssd(): memory leak from not freeing
  the result of slapi_escape_filter_value().
* backend_retrieve_group_entry_from_sssd(): fix line wrapping in
  function signature.
* backend_retrieve_group_entry_from_sssd() needs to handle ERANGE errors.
* backend_retrieve_group_entry_from_sssd(): extra whitespace when
  constructing the new entry's DN.
* backend_retrieve_group_entry_from_sssd(): memory leak from not freeing
  the result of slapi_escape_filter_value().
* backend_retrieve_group_entry_from_sssd() adds "ipaNTGroupAttrs" as an
  objectClass in the temporary entry, but the value isn't copied into
  the cache.  What purpose does it serve?
* backend_retrieve_group_list_from_sssd() needs to handle ERANGE errors.
* backend_retrieve_group_entry_from_sssd(): avoid using
  slapi_entry_attr_set_int(), which will treat an unsigned 32-bit
  with the high bit set as a negative value
* backend_retrieve_group_list_from_sssd(): check for NULL result from
  realloc().
* backend_search_sssd(): fix line wrapping for call to
  slapi_filter_apply().
* backend_search_sssd(): use strtol() or strtoul() to convert a value to
  a number, and check for errors.
* backend_search_sssd(): staged->container_sdn/map_group/map_set are
  allocated with slapi_ch_strdup() but freed with free() later.
* backend_retrieve_from_sssd(): check for NULL result from malloc().
* backend_retrieve_from_sssd(): fix line wrapping for calls to
  backend_retrieve_user/group_entry_from_sssd.
HEAD~5:
* free_pam_response() uses slapi_ch_free() to free memory that was
  probably allocated with malloc().
* pam_conv_func(): remove extra whitespace in call to slapi_pblock_get()
* pam_conv_func(): use malloc() instead of slapi_ch_calloc() to allocate
  replies, because plugins tend to be use free() to free them.
* pam_conv_func(): replace if/else-if/else-if/else-if stacks with a
  switch() statement.
* do_pam_auth(): fix line wrapping in function signature.
* do_pam_auth(): comments suggest that it's just entered or left a
  critical section, when the function's been called in one.
* do_pam_auth(): replace if/else-if/else-if/else-if stacks with a
  switch() statement.
* do_pam_auth(): fix line wrapping when calling PR_smprintf().
HEAD~4:
* Also needs to add back-sch.h to nisserver_plugin_la_SOURCES.
HEAD~3:
* backend_search_find_set_data_in_group_cb(): fix line wrapping in
  function signature.
* backend_search_cb() looks like it'll leak staged->entries[i] if
  another thread has added a similarly-named entry to the cache.
* backend_search_cb() now appears to be freeing the closest-match
  as part of a conditional clause, right before it would unconditionally
  do so.
* backend_search_cb() doesn't need to cast cbdata.sssd_buffer_len before
  comparing it to -1.
HEAD~2:
* backend_bind_cb() allocates strings with slapi_ch_strdup() and
  slapi_entry_attr_get_charptr() but frees them with free().
* backend_bind_cb() should avoid having to cast away a "const" by using
  non-const variables for the saved copies of the group and set names.
HEAD~1:
* Drop the version number change.
HEAD:
* Doesn't mention the PAM service used for authentication, which it
  probably should.  Still, a good start.

Thanks,

Nalin