11618 Use exec_attr instead of suid for smbfs mount and unmount

Review Request #2262 — Created Aug. 24, 2019 and submitted

Information
gwr
illumos-gate
11618
Reviewers
general 
11618 Use exec_attr instead of suid for smbfs mount and unmount


I have to say, the old "SUID, add priv, drop SUID" seemed simpler.

I guess the RBAC way is "better", right? :)
Works now.




gwr@gwr-oi2$ mkdir H
gwr@gwr-oi2$ mount -F smbfs //gwr@sm20lab/home_gwr H
gwr@gwr-oi2$ ls H
[...]
gwr@gwr-oi2$ umount H
gwr
jbk
jbk 
  1. Ship It!
  2. 
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 




  

 
















 



 

  

   




gwr




   

   

  

 



 















 



 

  

   




gwr




   

   

  

 



 















 



 

  

   




gwr




   

   

  

 



 















 





 

  

   




jclulow




   

   





  

 



 

  


   

   

    
 
jclulow


    
   


  

  

  



    




  1. 
 
    
  
    
   
    I can't immediately see how the prof_attr and exec_attr bits are expected to take effect?  I think for those to apply, you have to first assign the profile you've created to a user.  Then you have to run the program using pfexec (to elevate privileges) based on that profile.  You can also, I believe, use one of the "profile shells" which (as I recall) does the pfexec bits sort of implicitly.  It seems like it would be possible to have mount re-exec itself with the special flags that pfexec uses to trigger the kernel to elevate privileges, but I don't think this just happens by populating those files per se...
    
  
    
 
    

 
    
  


    
 
    
 
      

  


    1. 
 
 
      
 
      
  
 
      
 
      
  
      Hmm.  I was trying to follow what I saw in oi-userland components/library/libfuse which appears to do what I'm doing here, though maybe I missed something?
      
 
      

      2. 



    2. 
 
 
      
 
      
  
 
      
 
      
  
      Pretty sure what you're missing is that the intent is that a user then gets given that profile in /etc/user_attr, likely via usermod -P, to be able to make use of it.
      

      I think that what for example, cdrecord does, is to put the entry into a profile everyone has.  Like Basic Solaris User.
      
 
      

      3. 



    3. 
 
 
      
 
      
  
 
      
 
      
  
      I did that, but then it appears I still need to use "pfexec" to run the smbfs mount and unmount programs.
      
Is that expected?  I don't see libfuse or cdrecord doing that.
      
 
      

      4. 



    4. 
 
 
      
 
      
  
 
      
 
      
  
      I think it's expected.  Doesn't cdrecord have a wrapper?  I don't know about fuse, are you sure it actually does what you want itself?
      
 
      

      5. 



    5. 
 
 
      
 
      
  
 
      
 
      
  
      Yeah, it does pfexec magic.  I think I see a reasonable way to do this.  Have another look.
      
 
      

      6. 


 
    


    

 
    

    2. 















  2. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 




  

 
















 



 

  

   




gwr




   

   

  

 



 















 



 

  

   




gwr




   

   

  

 



 















 



 

  

   




gwr




   

   

  

 



 















 



 

  

   




gwr




   

   

  

 



 















 



 

  

   




gwr




   

   

  

 



 















 



 

  

   




gwr




   

   

  

 



 















 





 

  

   




jbk




   

   







  

 



 

  


   

   

    
 
jbk


    
   


  

  

  



    




  1. 
 
    
  
    
   
    Ship It!
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 















  2. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 




  

 
















 



 

  

   




gwr




   

   

  

 



 















 



 

  

   




gwr




   

   

  

 



 















 





 

  

   




jbk




   

   







  

 



 

  


   

   

    
 
jbk


    
   


  

  

  



    




  1. 
 
    
  
    
   
    Ship It!
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 















  2. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 




  

 
















 



 

  

   




gwr




   

   

  

 



 















 





 

  

   




gwr




   

   





  

 



 

  


   

   

    
 
gwr


    
   


  

  

  



    




  1. 
 
    
  
    
   
    Actually do "least privilege" (again).
    
  
    
 
    

 
    
  


    
 
    
 
      

  


    1. 
 
 
      
 
      
  
 
      
 
      
  
      FYI, I temporarily added a
      

      

          if (priv_ineffect(PRIV_SYS_MOUNT)) {
        (void) fprintf(stderr, "Still have SYS_MOUNT?\n");
    }

      

      after the initial privilege manipulation to verify it's not effective util the later
      

      

          (void) __priv_bracket(PRIV_ON);
    err2 = mount(mnt.mnt_special, mnt.mnt_mountp, ...);
    (void) __priv_bracket(PRIV_OFF);

      
 
      

      2. 


 
    


    

 
    

    2. 

















  2. 
 
    
  
  
    
  
 
    
  
    
   
    
    
     
      usr/src/cmd/fs.d/smbclnt/mount/mount.c
      


       (Diff revisions 8 - 10)


      
     
    
    
       
   
    
     
    

    
     
    

       
  
    
 
    

   
    
    
    




    
    Interesting:  I discovered that while __init_suid_priv  appears to be designed to work OK when a process is not SUID, it does not appear to expect the requested privileges to be already in effect when it runs.  To deal with that, I just added a __priv_bracket(PRIV_OFF) call after it.
    

    The remaining __priv_bracket calls then DTRT.
    
  
    
 
    

 
    
  


    
 
    
 
      

  


    1. 
 
 
      
 
      
  
 
      
 
      
  
      Should I add any comments about why this ... PRIV_OFF call is here?
      
(privs via exec_attr vs privs via setuid)
      
 
      

      2. 


 
    


    

 
    

    3. 




  3. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    4. 




  

 
















 





 

  

   




gwr




   

   





  

 



 

  


   

   

    
 
gwr


    
   


  

  

  



    




  1. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 

















  2. 
 
    
  
  
    
  
 
    
  
    
   
    
    
     
      usr/src/cmd/fs.d/smbclnt/mount/mount.c
      


       (Diff revision 10)


      
     
    
    
       
   
    
     
    

    
     
    

       
  
    
 
    

   
    
    
    




    
    Should I add any comments about why this ... PRIV_OFF call is here?
    

    

    +    * The __init_suid_priv call was designed for SUID programs,
+    * but also works for privileges granted via exec_attr with
+    * one difference: the added privileges are already effective
+    * when the program starts, and remain effective after the call.
+    * To make this work more like the SUID case we'll turn off the
+    * additional privileges with a __priv_bracket() call here.

    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 




  3. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    4. 




  

 
















 



 

  

   




gwr




   

   

  

 



 















 





 

  

   




jbk




   

   







  

 



 

  


   

   

    
 
jbk


    
   


  

  

  



    




  1. 
 
    
  
    
   
    Ship It!
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 















  2. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 




  

 
















 





 

  

   




jclulow




   

   







  

 



 

  


   

   

    
 
jclulow


    
   


  

  

  



    




  1. 
 
    
  
    
   
    Neat!  Glad you were able to get the PRIV_PFEXEC bit to work.
    
  
    
 
    

 
    
  


    
 
    
 
      

  


    1. 
 
 
      
 
      
  
 
      
 
      
  
      Yeah, one unexpected benefit of how this change ended up is:
      
The program will end up using (only) the desired privilege whether that privilege was granted via RBAC (exec_attr) or via SUID execution.  Handy for testing etc.
      
 
      

      2. 


 
    


    

 
    

    2. 















  2. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 




  

 
















 



 

  

   




gwr




   

   

  

 



 

  


   

   

    
 Review request changed

    
   


  

  

  






 

  Status:
  

  Closed (submitted)

  
 







 


 










  

 
















 





 

  

   




gwr




   

   





  

 



 

  


   

   

    
 
gwr


    
   


  

  

  



    




  1. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 

















  2. 
 
    
  
  
    
  
 
    
  
    
   
    
    
     
      usr/src/cmd/fs.d/smbclnt/umount/umount.c
      


       (Diff revision 11)


      
     
    
    
       
   
    
     
    

    
     
    

       
  
    
 
    

   
    
    
    




    
    The sprintf here was a bug, reported by Marco Ivaldi marco.ivaldi@mediaservice.net (thanks).  If someone were to exec this program with a very long argv[0] then data after typename could be smashed.
    

    Note however that the bug was NOT any sort of root exploit because after the __init_suid_priv() call above the program has given up its root privileges and is running as the real user.  That makes this just an ordinary bug, not a security bug.
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 




  3. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    4. 




  

 









     

    

   

  

  

   
   Loading...