[PATCH phosphor-networkd] Validating user & group names prior to system call and wait() on proc…

[PATCH phosphor-networkd] Validating user & group names prior to system call and wait() on proc…

[OpenBMC Patches]

… spawned with pexpect.

https://github.com/openbmc/phosphor-networkd/pull/10

Hariharasubramanian R (1):
  Validating user & group names prior to system call and wait() on proc
    spawned with pexpect.

 userman.py | 106 ++++++++++++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 76 insertions(+), 30 deletions(-)

-- 
2.7.1

[PATCH phosphor-networkd] Validating user & group names prior to system call and wait() on proc spawned with pexpect.

[OpenBMC Patches]

From: Hariharasubramanian R <hramasub@in.ibm.com>

---
 userman.py | 106 ++++++++++++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 76 insertions(+), 30 deletions(-)

diff --git a/userman.py b/userman.py
index 6109582..033d3d1 100755
--- a/userman.py
+++ b/userman.py
@@ -25,7 +25,8 @@ OBJ_NAME_USER = '/org/openbmc/UserManager/User'
     Object Path > /org/openbmc/UserManager/Groups
         Interface:Method > org.openbmc.Enrol.GroupAddSys string:"groupname"
         Interface:Method > org.openbmc.Enrol.GroupAddUsr string:"groupname"
-        Interface:Method > org.openbmc.Enrol.GroupList
+        Interface:Method > org.openbmc.Enrol.GroupListUsr
+        Interface:Method > org.openbmc.Enrol.GroupListSys
     Object Path > /org/openbmc/UserManager/Group
         Interface:Method > org.openbmc.Enrol.GroupDel string:"groupname"
     Object Path > /org/openbmc/UserManager/Users
@@ -60,16 +61,26 @@ class UserManGroups (dbus.service.Object):
 
     @dbus.service.method(INTF_NAME, "s", "x")
     def GroupAddUsr (self, groupname):
+        if not groupname : return 1
+
+        groups = self.GroupListAll ()
+        if groupname in groups: return 1
+
         r = call (["addgroup", groupname])
         return r
 
     @dbus.service.method(INTF_NAME, "s", "x")
     def GroupAddSys (self, groupname):
+        if not groupname : return 1
+
+        groups = self.GroupListAll ()
+        if groupname in groups: return 1
+
         r = call (["addgroup", "-S", groupname])
         return 0
 
     @dbus.service.method(INTF_NAME, "", "as")
-    def GroupList (self):
+    def GroupListUsr (self):
         groupList = []
         with open("/etc/group", "r") as f:
             for grent in f:
@@ -78,6 +89,23 @@ class UserManGroups (dbus.service.Object):
                     groupList.append(groupParams[0])
         return groupList
 
+    @dbus.service.method(INTF_NAME, "", "as")
+    def GroupListSys (self):
+        groupList = []
+        with open("/etc/group", "r") as f:
+            for grent in f:
+                groupParams = grent.split (":")
+                if (int(groupParams[2]) > 100 and int(groupParams[2]) < 1000): groupList.append(groupParams[0])
+        return groupList
+
+    def GroupListAll (self):
+        groupList = []
+        with open("/etc/group", "r") as f:
+            for grent in f:
+                groupParams = grent.split (":")
+                groupList.append(groupParams[0])
+        return groupList
+
 class UserManGroup (dbus.service.Object):
     def __init__(self, bus, name):
         self.bus = bus
@@ -93,6 +121,11 @@ class UserManGroup (dbus.service.Object):
 
     @dbus.service.method(INTF_NAME, "", "x")
     def GroupDel (self, groupname):
+        if not groupname : return 1
+
+        groups = Groupsobj.GroupListAll ()
+        if groupname not in groups: return 1
+
         r = call (["delgroup", groupname])
         return r
 
@@ -111,39 +144,31 @@ class UserManUsers (dbus.service.Object):
 
     @dbus.service.method(INTF_NAME, "ssss", "x")
     def UserAdd (self, gecos, username, groupname, passwd):
+        if not username: return 1
+
+        users = self.UserList ()
+        if username in users : return 1
+
         if groupname:
-            cmd = "adduser "  + " -g "  + gecos + " -G ", groupname + " " + username
+            groups = Groupsobj.GroupListAll ()
+            if groupname not in groups: return 1
+
+        opts = ""
+        if gecos: opts = " -g " + '"' + gecos + '"'
+
+        if groupname:
+            cmd = "adduser "  + opts + " " + " -G " + groupname + " " + username
         else:
-            cmd = "adduser "  + " -g "  + gecos + username
+            cmd = "adduser "  + opts + " " + username
 
         proc = pexpect.spawn (cmd)
-        proc.expect ("[New password: ]")
+        proc.expect (['New password: ', 'Retype password: '])
         proc.sendline (passwd)
-        proc.expect ("[Retype password: ]")
+        proc.expect (['New password: ', 'Retype password: '])
         proc.sendline (passwd)
-        return 0
 
-
-#        if groupname:
-#            proc = subprocess.Popen(['adduser', "-g", gecos, "-G", groupname, username], shell=False, stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=subprocess.PIPE, bufsize=1)
-#        else:
-#            proc = subprocess.Popen(['adduser', "-g", gecos, username], shell=False, stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=subprocess.PIPE, bufsize=1)
-#
-#        with proc.stdout:
-#            for prompt in iter(proc.stdout.readline, b''):
-#                proc.stdin.write(passwd)
-#
-#        return 0
-
-#        proc = subprocess.Popen(['passwd', username], shell=False, stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
-#        out,err = proc.communicate(passwd)
-#        out,err = proc.communicate(passwd)
-#        proc.stdin.write(passwd)
-#        proc.stdin.write(passwd)
-#        if (not err): return 0
-#        print out
-#        print err
-#        return 0
+        proc.wait()
+        return 0
 
     @dbus.service.method(INTF_NAME, "", "as")
     def UserList (self):
@@ -170,20 +195,41 @@ class UserManUser (dbus.service.Object):
 
     @dbus.service.method(INTF_NAME, "s", "x")
     def UserDel (self, username):
+        if not username : return 1
+
+        users = Usersobj.UserList ()
+        if username not in users : return 1
+
         r = call (["deluser", username])
         return r
 
     @dbus.service.method(INTF_NAME, "ss", "x")
     def Passwd (self, username, passwd):
-        r = call (["echo", "-e", passwd, "passwd", username])
-        return r
+        if not username : return 1
+        
+        users = self.UserList ()
+        if username not in users : return 1
 
+        cmd = "passwd" + " " + username
+        proc = pexpect.spawn (cmd)
+        proc.expect (['New password: ', 'Retype password: '])
+        proc.sendline (passwd)
+        proc.expect (['New password: ', 'Retype password: '])
+        proc.sendline (passwd)
+
+        proc.wait()
+        return r
 
 def main():
     dbus.mainloop.glib.DBusGMainLoop(set_as_default=True)
     bus = dbus.SystemBus()
     name = dbus.service.BusName(DBUS_NAME, bus)
 
+    global Groupsobj
+    global Groupobj
+    global Usersobj
+    global Userobj
+
     Groupsobj   = UserManGroups (bus, OBJ_NAME_GROUPS)
     Groupobj    = UserManGroup  (bus, OBJ_NAME_GROUP)
     Usersobj    = UserManUsers  (bus, OBJ_NAME_USERS)
-- 
2.7.1

Re: [PATCH phosphor-networkd] Validating user & group names prior to system call and wait() on proc spawned with pexpect.

[Stewart Smith]

OpenBMC Patches <openbmc-patches@stwcx.xyz> writes:
> From: Hariharasubramanian R <hramasub@in.ibm.com>
>
> ---
>  userman.py | 106 ++++++++++++++++++++++++++++++++++++++++++++-----------------
>  1 file changed, 76 insertions(+), 30 deletions(-)
>
> diff --git a/userman.py b/userman.py
> index 6109582..033d3d1 100755
> --- a/userman.py
> +++ b/userman.py
> @@ -25,7 +25,8 @@ OBJ_NAME_USER = '/org/openbmc/UserManager/User'
>      Object Path > /org/openbmc/UserManager/Groups
>          Interface:Method > org.openbmc.Enrol.GroupAddSys string:"groupname"
>          Interface:Method > org.openbmc.Enrol.GroupAddUsr string:"groupname"
> -        Interface:Method > org.openbmc.Enrol.GroupList
> +        Interface:Method > org.openbmc.Enrol.GroupListUsr
> +        Interface:Method > org.openbmc.Enrol.GroupListSys
>      Object Path > /org/openbmc/UserManager/Group
>          Interface:Method > org.openbmc.Enrol.GroupDel string:"groupname"
>      Object Path > /org/openbmc/UserManager/Users
> @@ -60,16 +61,26 @@ class UserManGroups (dbus.service.Object):
>  
>      @dbus.service.method(INTF_NAME, "s", "x")
>      def GroupAddUsr (self, groupname):
> +        if not groupname : return 1
> +
> +        groups = self.GroupListAll ()
> +        if groupname in groups: return 1
> +
>          r = call (["addgroup", groupname])
>          return r
>  
>      @dbus.service.method(INTF_NAME, "s", "x")
>      def GroupAddSys (self, groupname):
> +        if not groupname : return 1
> +
> +        groups = self.GroupListAll ()
> +        if groupname in groups: return 1
> +
>          r = call (["addgroup", "-S", groupname])
>          return 0
>  
>      @dbus.service.method(INTF_NAME, "", "as")
> -    def GroupList (self):
> +    def GroupListUsr (self):
>          groupList = []
>          with open("/etc/group", "r") as f:
>              for grent in f:
> @@ -78,6 +89,23 @@ class UserManGroups (dbus.service.Object):
>                      groupList.append(groupParams[0])
>          return groupList
>  
> +    @dbus.service.method(INTF_NAME, "", "as")
> +    def GroupListSys (self):
> +        groupList = []
> +        with open("/etc/group", "r") as f:
> +            for grent in f:
> +                groupParams = grent.split (":")
> +                if (int(groupParams[2]) > 100 and int(groupParams[2]) < 1000): groupList.append(groupParams[0])
> +        return groupList

Why aren't you using an existing python module such as grp rather than
writing your own (likely buggy) parser?

It seems to have existed since at least python 2.6...
https://docs.python.org/2.6/library/grp.html

> @@ -111,39 +144,31 @@ class UserManUsers (dbus.service.Object):
>  
>      @dbus.service.method(INTF_NAME, "ssss", "x")
>      def UserAdd (self, gecos, username, groupname, passwd):
> +        if not username: return 1
> +
> +        users = self.UserList ()
> +        if username in users : return 1
> +
>          if groupname:
> -            cmd = "adduser "  + " -g "  + gecos + " -G ", groupname + " " + username
> +            groups = Groupsobj.GroupListAll ()
> +            if groupname not in groups: return 1
> +
> +        opts = ""
> +        if gecos: opts = " -g " + '"' + gecos + '"'
> +
> +        if groupname:
> +            cmd = "adduser "  + opts + " " + " -G " + groupname + " " + username
>          else:
> -            cmd = "adduser "  + " -g "  + gecos + username
> +            cmd = "adduser "  + opts + " " + username

I note there's a python-libuser package on ubuntu, is that a library
that could be used instead?

-- 
Stewart Smith
OPAL Architect, IBM.

Re: [PATCH phosphor-networkd] Validating user & group names prior to system call and wait() on proc spawned with pexpect.

[Brad Bishop]

[-- Attachment #1: Type: text/plain, Size: 4362 bytes --]

Sorry about the dup Stewart…

grp just parses the file and returns a dictionary.  It doesn’t do anything mutable.

The only thing I could find was libuser….

-brad

> On Feb 11, 2016, at 5:18 PM, Stewart Smith <stewart@linux.vnet.ibm.com> wrote:
> 
> OpenBMC Patches <openbmc-patches@stwcx.xyz <mailto:openbmc-patches@stwcx.xyz>> writes:
>> From: Hariharasubramanian R <hramasub@in.ibm.com>
>> 
>> ---
>> userman.py | 106 ++++++++++++++++++++++++++++++++++++++++++++-----------------
>> 1 file changed, 76 insertions(+), 30 deletions(-)
>> 
>> diff --git a/userman.py b/userman.py
>> index 6109582..033d3d1 100755
>> --- a/userman.py
>> +++ b/userman.py
>> @@ -25,7 +25,8 @@ OBJ_NAME_USER = '/org/openbmc/UserManager/User'
>>     Object Path > /org/openbmc/UserManager/Groups
>>         Interface:Method > org.openbmc.Enrol.GroupAddSys string:"groupname"
>>         Interface:Method > org.openbmc.Enrol.GroupAddUsr string:"groupname"
>> -        Interface:Method > org.openbmc.Enrol.GroupList
>> +        Interface:Method > org.openbmc.Enrol.GroupListUsr
>> +        Interface:Method > org.openbmc.Enrol.GroupListSys
>>     Object Path > /org/openbmc/UserManager/Group
>>         Interface:Method > org.openbmc.Enrol.GroupDel string:"groupname"
>>     Object Path > /org/openbmc/UserManager/Users
>> @@ -60,16 +61,26 @@ class UserManGroups (dbus.service.Object):
>> 
>>     @dbus.service.method(INTF_NAME, "s", "x")
>>     def GroupAddUsr (self, groupname):
>> +        if not groupname : return 1
>> +
>> +        groups = self.GroupListAll ()
>> +        if groupname in groups: return 1
>> +
>>         r = call (["addgroup", groupname])
>>         return r
>> 
>>     @dbus.service.method(INTF_NAME, "s", "x")
>>     def GroupAddSys (self, groupname):
>> +        if not groupname : return 1
>> +
>> +        groups = self.GroupListAll ()
>> +        if groupname in groups: return 1
>> +
>>         r = call (["addgroup", "-S", groupname])
>>         return 0
>> 
>>     @dbus.service.method(INTF_NAME, "", "as")
>> -    def GroupList (self):
>> +    def GroupListUsr (self):
>>         groupList = []
>>         with open("/etc/group", "r") as f:
>>             for grent in f:
>> @@ -78,6 +89,23 @@ class UserManGroups (dbus.service.Object):
>>                     groupList.append(groupParams[0])
>>         return groupList
>> 
>> +    @dbus.service.method(INTF_NAME, "", "as")
>> +    def GroupListSys (self):
>> +        groupList = []
>> +        with open("/etc/group", "r") as f:
>> +            for grent in f:
>> +                groupParams = grent.split (":")
>> +                if (int(groupParams[2]) > 100 and int(groupParams[2]) < 1000): groupList.append(groupParams[0])
>> +        return groupList
> 
> Why aren't you using an existing python module such as grp rather than
> writing your own (likely buggy) parser?
> 
> It seems to have existed since at least python 2.6...
> https://docs.python.org/2.6/library/grp.html <https://docs.python.org/2.6/library/grp.html>
> 
>> @@ -111,39 +144,31 @@ class UserManUsers (dbus.service.Object):
>> 
>>     @dbus.service.method(INTF_NAME, "ssss", "x")
>>     def UserAdd (self, gecos, username, groupname, passwd):
>> +        if not username: return 1
>> +
>> +        users = self.UserList ()
>> +        if username in users : return 1
>> +
>>         if groupname:
>> -            cmd = "adduser "  + " -g "  + gecos + " -G ", groupname + " " + username
>> +            groups = Groupsobj.GroupListAll ()
>> +            if groupname not in groups: return 1
>> +
>> +        opts = ""
>> +        if gecos: opts = " -g " + '"' + gecos + '"'
>> +
>> +        if groupname:
>> +            cmd = "adduser "  + opts + " " + " -G " + groupname + " " + username
>>         else:
>> -            cmd = "adduser "  + " -g "  + gecos + username
>> +            cmd = "adduser "  + opts + " " + username
> 
> I note there's a python-libuser package on ubuntu, is that a library
> that could be used instead?
> 
> -- 
> Stewart Smith
> OPAL Architect, IBM.
> 
> _______________________________________________
> openbmc mailing list
> openbmc@lists.ozlabs.org <mailto:openbmc@lists.ozlabs.org>
> https://lists.ozlabs.org/listinfo/openbmc <https://lists.ozlabs.org/listinfo/openbmc>

[-- Attachment #2: Type: text/html, Size: 19921 bytes --]

Re: [PATCH phosphor-networkd] Validating user & group names prior to system call and wait() on proc spawned with pexpect.

[Stewart Smith]

Brad Bishop <bradleyb@fuzziesquirrel.com> writes:
> Sorry about the dup Stewart…

(np)

> grp just parses the file and returns a dictionary.  It doesn’t do anything mutable.
>
> The only thing I could find was libuser….

Ahh... okay, I wasn't looking toooo closely.

Possibly worth bringing in libuser? Maybe in the near future at least
rather than immediately.

-- 
Stewart Smith
OPAL Architect, IBM.