[Freeipa-devel] [PATCH] 131-132 extdom: add support for sss_nss_getorigbyname()

[Jakub Hrozek]

On Fri, Oct 17, 2014 at 11:53:44AM +0200, Sumit Bose wrote:
> Hi,
> 
> the first patch replaces sss_nss_getsidbyname() by
> sss_nss_getorigbyname() for the new version of the extdom interface.
> The new call returns more data about the original object and allows the
> IPA client to have the same information about the object in the SSSD
> cache as the IPA servers.
> 
> The second patch just removes an obsolete dependency.
> 
> bye,
> Sumit

Hi,

I was unable to send the patches through Coverity, the RH server seems
to be having issues. I'll wait until tomorrow, if the problems persist,
we'll just skip Coverity and fix any potential problems post-push.

> From 928c04c35601b7bc1c57c1320e4a746abc35e947 Mon Sep 17 00:00:00 2001
> From: Sumit Bose <sbose at redhat.com>
> Date: Fri, 10 Oct 2014 10:56:37 +0200
> Subject: [PATCH 131/132] extdom: add support for sss_nss_getorigbyname()

[...]

> @@ -576,13 +613,14 @@ static int handle_gid_request(enum request_types request_type, gid_t gid,
>      enum sss_id_type id_type;
>      size_t buf_len;
>      char *buf = NULL;
> +    struct sss_nss_kv *kv_list;

Please set kv_list to NULL here, you're freeing the pointer
unconditionally in the done handler, but in some cases (request_type ==
REQ_SIMPLE) kv_list is not set at all.

>  
>      ret = get_buffer(&buf_len, &buf);
>      if (ret != LDAP_SUCCESS) {
>          return ret;
>      }
>  
> -    if (request_type == REQ_SIMPLE || request_type == REQ_FULL_WITH_GROUPS) {
> +    if (request_type == REQ_SIMPLE) {
>          ret = sss_nss_getsidbyid(gid, &sid_str, &id_type);
>          if (ret != 0 || id_type != SSS_ID_TYPE_GID) {
>              if (ret == ENOENT) {
> @@ -592,9 +630,7 @@ static int handle_gid_request(enum request_types request_type, gid_t gid,
>              }
>              goto done;
>          }
> -    }
>  
> -    if (request_type == REQ_SIMPLE) {
>          ret = pack_ber_sid(sid_str, berval);
>      } else {
>          ret = getgrgid_r(gid, &grp, buf, buf_len, &grp_result);
> @@ -607,13 +643,27 @@ static int handle_gid_request(enum request_types request_type, gid_t gid,
>              goto done;
>          }
>  
> +        if (request_type == REQ_FULL_WITH_GROUPS) {
> +            ret = sss_nss_getorigbyname(grp.gr_name, &kv_list, &id_type);
> +            if (ret != 0 || !(id_type == SSS_ID_TYPE_GID
> +                                || id_type == SSS_ID_TYPE_BOTH)) {
> +                if (ret == ENOENT) {
> +                    ret = LDAP_NO_SUCH_OBJECT;
> +                } else {
> +                    ret = LDAP_OPERATIONS_ERROR;
> +                }
> +                goto done;
> +            }
> +        }
> +
>          ret = pack_ber_group((request_type == REQ_FULL ? RESP_GROUP
>                                                         : RESP_GROUP_MEMBERS),
>                               domain_name, grp.gr_name, grp.gr_gid,
> -                             grp.gr_mem, sid_str, berval);
> +                             grp.gr_mem, kv_list, berval);
>      }
>  
>  done:
> +    sss_nss_free_kv(kv_list);
>      free(sid_str);
>      free(buf);
>      return ret;
> @@ -634,6 +684,7 @@ static int handle_sid_request(enum request_types request_type, const char *sid,
>      size_t buf_len;
>      char *buf = NULL;
>      enum sss_id_type id_type;
> +    struct sss_nss_kv *kv_list;

Also please set kv_list to NULL here...

>  
>      ret = sss_nss_getnamebysid(sid, &fq_name, &id_type);
>      if (ret != 0) {

[...]

> @@ -733,6 +811,7 @@ static int handle_name_request(enum request_types request_type,
>      enum sss_id_type id_type;
>      size_t buf_len;
>      char *buf = NULL;
> +    struct sss_nss_kv *kv_list;

...and here.

>  
>      ret = asprintf(&fq_name, "%s%c%s", name, SSSD_DOMAIN_SEPARATOR,
>                                         domain_name);

The rest of the patch looks good to me.

> From f83616c145d5d14d125c663f9ac4e31cff4af81b Mon Sep 17 00:00:00 2001
> From: Sumit Bose <sbose at redhat.com>
> Date: Wed, 15 Oct 2014 16:21:53 +0200
> Subject: [PATCH 132/132] extdom: remove unused dependency to libsss_idmap

ACK