On Mon, 06 Jun 2016, Sumit Bose wrote:

On Sun, Jun 05, 2016 at 10:36:51PM +0300, Alexander Bokovoy wrote:

...

On Fri, 03 Jun 2016, Sumit Bose wrote:

...

Hi,

this patch fixes an issue in SSSD's implementation of libwbclient. wbcSidsToUnixIds() translates a list of SID to POSIX IDs and it is expected that if one SID cannot be mapped the related output entry should just get type WBC_ID_TYPE_NOT_SPECIFIED. Currently the request fail completely if one SID cannot be mapped.

To test, use 'wbinfo --sids-to-unix-ids' with an invalid SID.

Without fix:

$ wbinfo --sids-to-unix-ids=S-1-5-21-3692237560-1981608775-3610128199-1104,S-1-5-21-3692237560-1981608775-3610128199-5 wbcSidsToUnixIds failed: WBC_ERR_UNKNOWN_FAILURE wbinfo_sids_to_unix_ids failed

With fix:

$ wbinfo --sids-to-unix-ids=S-1-5-21-3692237560-1981608775-3610128199-1104,S-1-5-21-3692237560-1981608775-3610128199-5 S-1-5-21-3692237560-1981608775-3610128199-1104 -> uid 700201104 S-1-5-21-3692237560-1981608775-3610128199-5 -> unmapped

Even with completely random SIDs you should see a proper output:

$ wbinfo --sids-to-unix-ids=S-2-3-4,S-5-6-7 S-2-3-4 -> unmapped S-5-6-7 -> unmapped

bye, Sumit

...

From 52de39e45829ffd1bd18b3f83310066f97a38397 Mon Sep 17 00:00:00 2001 From: Sumit Bose sbose@redhat.com Date: Thu, 2 Jun 2016 21:01:11 +0200 Subject: [PATCH] libwbclient: wbcSidsToUnixIds() don't fail on errors

Resolves: https://fedorahosted.org/sssd/ticket/3028

src/sss_client/libwbclient/wbc_idmap_sssd.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/src/sss_client/libwbclient/wbc_idmap_sssd.c b/src/sss_client/libwbclient/wbc_idmap_sssd.c index 1b0e2e10a5ce1a0c7577d391b740ff988f920903..b3e292217e056dde323c82f1303b49a058933dab 100644 --- a/src/sss_client/libwbclient/wbc_idmap_sssd.c +++ b/src/sss_client/libwbclient/wbc_idmap_sssd.c @@ -173,14 +173,12 @@ wbcErr wbcSidsToUnixIds(const struct wbcDomainSid *sids, uint32_t num_sids,

for (c = 0; c < num_sids; c++) {
    wbc_status = wbcSidToString(&sids[c], &sid_str);

•    if (!WBC_ERROR_IS_OK(wbc_status)) {
  

•        return wbc_status;
  

•    }
  

•    ret = sss_nss_getidbysid(sid_str, &id, &type);
  

•    wbcFreeMemory(sid_str);
  

•    if (ret != 0) {
  

•        return WBC_ERR_UNKNOWN_FAILURE;
  

•    if (WBC_ERROR_IS_OK(wbc_status)) {
  

•        ret = sss_nss_getidbysid(sid_str, &id, &type);
  

•        wbcFreeMemory(sid_str);
  

•        if (ret != 0) {
  

•            type = SSS_ID_TYPE_NOT_SPECIFIED;
  

•        }
    }
  
    switch (type) {
  

With this change 'type' variable will become undefined if wbcSidToString() failed. Perhaps, it could be set to 'type = SSS_ID_TYPE_NOT_SPECIFIED;' at the beginning of the for() loop?

Hi Alexander,

thank you for catching this. I have added your suggestion to the patch. Please note that although not strictly needed I kept the 'if (ret != 0)' block to avoid compiler or Coverity warnings about unchecked return values.

Yes, I was thinking how to merge those two yesterday as well and also decided that the 'if (ret != 0)' should stay.

ACK.