gmane.linux.redhat.sssd.devel

Re: [PATCH] Use uint32_t to copy the service port

Stephen Gallagher — 2012-05-25T17:31:18


Pushed to master and sssd-1-8.
_______________________________________________
sssd-devel mailing list
sssd-devel< at >lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/sssd-devel

Re: [PATCH] Use uint32_t to copy the service port

Jakub Hrozek — 2012-05-25T16:36:27

Ack++ 

I tested this by writing a program that called getservbyport(port,proto) in
a loop and went from unpatched SSSD to my patch and then to Stephen's patch.

After my patch was applied and SSSD restarted, the sss_nss client was
receiving "NULL" instead of "proto" in the protocol field - it was
probably hitting the padding. Then I applied Stephen's patch, restarted
SSSD and everything went back to normal.

Thanks for the explanation Simo.
_______________________________________________
sssd-devel mailing list
sssd-devel< at >lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/sssd-devel

Re: [PATCH] Use uint32_t to copy the service port

Simo Sorce — 2012-05-25T15:54:40

ACK.

Simo.

Re: [PATCH] Use uint32_t to copy the service port

Stephen Gallagher — 2012-05-25T14:17:37

Simo noted that we changed the protocol here. We dropped 32 bits of
padding. In order to remain compatible with existing clients, we need to
put that back in, which the attached patch does. (It also corrects the
comment describing the protocol to have it match reality).

The reason we want to do this is that an upgrade and restart of SSSD
won't change the copy of libnss_sss.so that's already loaded into
running processes. Thus we would be breaking service lookups until those
apps were restarted.
_______________________________________________
sssd-devel mailing list
sssd-devel< at >lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/sssd-devel

Re: Securing remote domains

Simo Sorce — 2012-05-25T13:48:33

If you set id ranges so that 2000 is not valid in the domain it will be
rejected.


These semantics are not soemthing SSSD can influence, they are internal
glibc semantics. On initgroups glibc asks all the group map backends
about group memberships.
However, recently glibc added an option so that you can segregate
initgroups too. In general we try not to use it becaus ein many cases
people do want to have the memberships calculated through all group
backends.
However if you enable "initgroupos: files sss", the getgrouplist call do
not continue past files into sss if entries are found in files.
I am not sure I like this option, as it is rather new, undocumented, and
the semantics may not be really useful, but you may want to experiment
with it if you have a new enough glibc. (Was committed to glibc upstream
repo on may 10 2011)


SSSD does not, I think we could do something as we should have access to
the list of groups previous nsswitch plugins returned, but we don't, as
in many cases users do want the curr

Re: Use variable to control verbosity for things in common directory

Dmitri Pal — 2012-05-25T13:04:50

Yes.

Re: [PATCH] Use uint32_t to copy the service port

Stephen Gallagher — 2012-05-25T12:52:04

Pushed to master and sssd-1-8.
_______________________________________________
sssd-devel mailing list
sssd-devel< at >lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/sssd-devel

Re: [PATCH] Use uint32_t to copy the service port

Stephen Gallagher — 2012-05-25T12:45:41

Ack
_______________________________________________
sssd-devel mailing list
sssd-devel< at >lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/sssd-devel

Re: [PATCH] Use uint32_t to copy the service port

Jakub Hrozek — 2012-05-25T12:22:32

Sure, new patch is attached.

Yes, I also tested on s390x running RHEL6.
From 67f45ccd456cebcf3f036508dce1028bc9d5735a Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek-H+wXaHxf7aLQT0dZR+AlfA< at >public.gmane.org>
Date: Fri, 25 May 2012 13:52:32 +0200
Subject: [PATCH] Send 16bit protocol numbers from the sss_client

https://fedorahosted.org/sssd/ticket/1348
---
 src/responder/nss/nsssrv_services.c |    2 +-
 src/sss_client/nss_services.c       |   13 +++++++------
 2 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/src/responder/nss/nsssrv_services.c b/src/responder/nss/nsssrv_services.c
index 2e539f13576d18c97d8c3bff2ced2fd5ed01290f..db8a2ca132b4f47c4d6cd78ce99280486e22f2a0 100644
--- a/src/responder/nss/nsssrv_services.c
+++ b/src/responder/nss/nsssrv_services.c
< at >< at > -1050,7 +1050,7 < at >< at > errno_t parse_getservbyport(TALLOC_CTX *mem_ctx,
     SAFEALIGN_COPY_UINT16(&c, body, NULL);
     port = ntohs(c);

-    port_and_padding_len = 2 * sizeof(uint16_t) + sizeof(uint32_t);
+    port_and_padding_len =

Re: [PATCH] Use uint32_t to copy the service port

Stephen Gallagher — 2012-05-25T12:12:56

Replying to myself, I can confirm that this is working on x86_64 at
least.
_______________________________________________
sssd-devel mailing list
sssd-devel< at >lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/sssd-devel

Re: [PATCH] Use uint32_t to copy the service port

Stephen Gallagher — 2012-05-25T12:09:06


Nack (minor).

Would you mind using SAFEALIGN_SET_UINT16() for the padding? The macro
expands to exactly the same code you have there.

Has this been tested on a little-endian and big-endian system?
_______________________________________________
sssd-devel mailing list
sssd-devel< at >lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/sssd-devel

Re: Securing remote domains

Stephen Gallagher — 2012-05-25T11:56:57

Right, this is correct (and is one of the headaches with the libc
interface: the behavior of initgroups() and getgrnam() are not
reciprocal).



Yeah, I think I see the issue here now. If there's an LDAP group with ID
10, and they're a member, initgroups() of that user WILL list 10. So
you're right here.


Ok, I get the gist of this. It's pretty hackish though (and will likely
only work with glibc, which is a problem because we're at least *trying*
to maintain compatibility with any libc in SSSD).

Also, as I originally suspected, I'm pretty sure this code is not going
to work with the new initgroups() semantics added to the last two
releases of glibc.

The core problem here is that this is basically a hack. The libc
interface for initgroups() is intentionally-written to be additive. This
is working around that by suppressing some values if they happen to also
exist locally. It's not a bad solution, necessarily, but it's against
the spirit of the interface.

Basically, the expectation of both SSSD and libc

Re: [PATCH] Use uint32_t to copy the service port

Jakub Hrozek — 2012-05-25T11:56:58

Attached.
From dbef520242fd60b234c05323598318df1bf98207 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek-H+wXaHxf7aLQT0dZR+AlfA< at >public.gmane.org>
Date: Fri, 25 May 2012 13:52:32 +0200
Subject: [PATCH] Send 16bit protocol numbers from the sss_client

https://fedorahosted.org/sssd/ticket/1348
---
 src/responder/nss/nsssrv_services.c |    2 +-
 src/sss_client/nss_services.c       |   14 ++++++++------
 2 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/src/responder/nss/nsssrv_services.c b/src/responder/nss/nsssrv_services.c
index 2e539f13576d18c97d8c3bff2ced2fd5ed01290f..db8a2ca132b4f47c4d6cd78ce99280486e22f2a0 100644
--- a/src/responder/nss/nsssrv_services.c
+++ b/src/responder/nss/nsssrv_services.c
< at >< at > -1050,7 +1050,7 < at >< at > errno_t parse_getservbyport(TALLOC_CTX *mem_ctx,
     SAFEALIGN_COPY_UINT16(&c, body, NULL);
     port = ntohs(c);
 
-    port_and_padding_len = 2 * sizeof(uint16_t) + sizeof(uint32_t);
+    port_and_padding_len = 2 * sizeof(uint16_t);
     i = port_and_padding_len;
     j =

Re: Use variable to control verbosity for things in common directory

Stephen Gallagher — 2012-05-25T11:31:35

This is an ancient ticket. I believe it was originally meant to be for
the unit tests in the ding-libs, so that we could set their verbosity
the same way we do for the tests in SSSD.

It mentions the 'common' directory, which was what ding-libs was before
we split it out.

I think this ticket is no longer valid. This was only really useful for
when SSSD and ding-libs were being built together in the same build
system.
_______________________________________________
sssd-devel mailing list
sssd-devel< at >lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/sssd-devel

Re: [PATCH] Use uint32_t to copy the service port

Stephen Gallagher — 2012-05-25T11:28:06

No, you're right. The client should only be sending a 16-bit value.

Nack.
Please change the client to send a uint16_t instead.
_______________________________________________
sssd-devel mailing list
sssd-devel< at >lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/sssd-devel

Re: Use variable to control verbosity for things in common directory

Jakub Hrozek — 2012-05-25T10:50:50

The way I read the ticket (although I'm not exactly sure if verbosity
meant debug there..), the ticket was specifically talking about the sss_
command line tools such as sss_useradd (see src/tools/*.c).

Currently the user must specify the debug level with an undocumented
--debug switch. The ticket proposed to also read some environment variable
and set the debug level according to that environment variable.
_______________________________________________
sssd-devel mailing list
sssd-devel< at >lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/sssd-devel

[PATCH] Use uint32_t to copy the service port

Jakub Hrozek — 2012-05-25T10:46:53

The sss_client was copying 32bit port value, but the NSS responder was
reading 16bit port value. This was breaking on Big-Endian machines where
we read "the other 16bits".

By the way, is there a reason to use 32bits in the client in the first
place? IIRC a port number is a 16 bit value..
From eb8a81adfa05cfa8b62291bac0052c4e15124a8e Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek-H+wXaHxf7aLQT0dZR+AlfA< at >public.gmane.org>
Date: Fri, 25 May 2012 11:51:11 +0200
Subject: [PATCH] Use uint32_t to copy the service port

---
 src/responder/nss/nsssrv_services.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/responder/nss/nsssrv_services.c b/src/responder/nss/nsssrv_services.c
index 2e539f13576d18c97d8c3bff2ced2fd5ed01290f..3a6e1b07866a539b36284446e60b2d507d312275 100644
--- a/src/responder/nss/nsssrv_services.c
+++ b/src/responder/nss/nsssrv_services.c
< at >< at > -1041,13 +1041,13 < at >< at > errno_t parse_getservbyport(TALLOC_CTX *mem_ctx,
     errno_t ret;
     size_t i, j;
     size_t port_and

Use variable to control verbosity for things in commondirectory

Ariel Barria — 2012-05-25T05:22:19

https://fedorahosted.org/sssd/ticket/394

I was reviewing this ticket and talks about a default value to verbosity.

althought i'm not sure if is about the sssd debug level or other case.

If the case of debug level:

Reading theory in
   http://sgallagh.fedorapeople.org/sssd/1.8.91/man/sssd.conf.5.html 
in section debug_level (integer) mentions that:

  "0x0010 is the default value as well as the lowest allowed value"
  "0x0010: Fatal failures. Anything that would prevent SSSD from starting up or causes it to cease running."

If you want to use a higher debug level is changed in sssd.conf-> debug_level = (desired level is placed).

By not specifying on command line flag, is used the indicated in sssd.conf ->debug_level.

If specified in command line debug_level first uses the command line, this was corrected in the ticket https://fedorahosted.org/sssd/ticket/764

In the case concerned from that, the flag already exists.

util.h
[code]
      /** \def DEBUG_IS_SET(level)
         \brief checks whether level

Re: Securing remote domains

Edward Z. Yang — 2012-05-25T04:11:47

Excerpts from Stephen Gallagher's message of Wed May 23 08:12:33 -0400 2012:

Yes, we are in agreement here.


OK, the further case we worry about is as follows: If you call
getpwnam("imposter"), will we get back UID 2000, or will this be rejected?


We do not believe the traditional nsswitch semantics work this way.  If you
look at initgroups(), what will happen is that the lookup for groups of a
remote user will fail on the local groups source (/etc/group), and then NSS
will consult the remote source for groups, and initgroups_dyn will add them as
secondary groups for the user.  These groups are not normally distinguished
from the normal groups.

It is possible that SSSD has different semantics, but this is not
obvious to us.


Yes, and secondary GIDs have the same problem, unless SSSD does something
dramatically different.  I will file this report.


You can access nss_nonlocal from here:

    http://debathena.mit.edu/nss_nonlocal/
    git://andersk.mit.edu/nss_nonlocal
    http://andersk.mit.edu/gitweb/n

[PATCH] sss_idmap: add support for samba struct dom_sid

Sumit Bose — 2012-05-24T13:04:36

Hi,

this patch allows us besides other conversions to convert the dom_sid
structure used by samba to strings and back. This structure is used by
various samba libraries, but there are no public inferfaces for the
conversion. I've seen Simo adding code to the IPA kdb plugin doing these
conversions and I need them for the PAC responder as well. So I thought
it might be useful to put it in a library.

bye,
Sumit
From b33d2e0e6cb18a3c90a9b4fda0d4ae7e60136f97 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose< at >redhat.com>
Date: Thu, 24 May 2012 12:39:56 +0200
Subject: [PATCH] sss_idmap: add support for samba struct dom_sid

The samba ndr libraries use struct dom_sid to handle SIDs. Since there
is no public samba library which offers conversion from other
representations, e.g. as string, this is addded to libsss_idmap. There
is only a compile-time dependency to the samba header files to check if
struct dom_sid has the expected format. There is no run-time dependency
to any samba library.
---
 Makefile.am

Re: [PATCH] NSS: Fix segfault when mmap cache cannot be initialized

Stephen Gallagher — 2012-05-24T12:32:07

Pushed to master.
_______________________________________________
sssd-devel mailing list
sssd-devel< at >lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/sssd-devel