Tomas, Why are you using umask for everything except the sparc-setup.sh code? The chmod still concerns me. Is there a reason why an appropriate umask can not be used in the sparc-setup.sh script?
Thanks, John On Dec 8, 2011, at 8:45 AM, Ethan Quach wrote: > Tomas, > > It looks fine to me. > > thanks, > -ethan > > > On 12/08/11 08:26, Tomas Dzik wrote: >> Hi Ethan, >> I changed chmod to absolute 0644 and the latest webrev is here: >> >> https://cr.opensolaris.org/action/browse/caiman/t.dzik/7052679-2/ >> >> Could you please look at it and let me know whether is it OK now ? >> >> Thanks a lot in advance, >> >> Tomas D. >> >> Dne 7.12.11 09:24, Ethan Quach napsal(a): >>> Tomas, >>> >>> On 12/06/11 04:43, Tomas Dzik wrote: >>>> Hi Ethan, >>>> I was considering all three ways and deciding which way to use and I >>>> choose setting umask to be cleanest and safest option. >>>> >>>> Combination of os.fdopen(os.open()) does not work because according to >>>> python docs: >>>> http://docs.python.org/library/os.html#os.open >>>> >>>> "Open the file file and set various flags according to flags and >>>> possibly its mode according to mode. The default mode is 0777 (octal), >>>> and the current umask value is first masked out." >>>> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ >>> >>> Ok. I didn't realize it masked against the umask for cases where a mode >>> is passed in ... >>> >>>> >>>> Chmoding these files after creation is possible - however you never >>>> know what might be security critical. >>>> >>>> For the changes in setup-sparc.sh I intentionally just added the 'r' >>>> permission because I know that these files must be readable for >>>> everyone and I didn't want to anticipate anything else - it's up to >>>> root to set his umask according to his needs/security policy. However >>>> if you feel that application should pretend to be smarter then admin I >>>> can easily change it. >>> >>> But we're effectively doing this for the files being created where umask >>> is used, so why do it differently with these two files? >>> >>>> >>>> So do you want me to change os.umask to os.chmod(00644) in python code >>>> and >>> >>> Using umask is fine with me here. >>> >>>> chmod ugo+r to chmod 0644 in setup-sparc.sh ? >>> >>> I would prefer to set them to 0644 explicitly. >>> >>> >>> thanks, >>> -ethan >>> >>> >>>> >>>> Regards, >>>> >>>> Tomas D. >>>> >>>> >>>> >>>> Dne 6.12.11 03:07, Ethan Quach napsal(a): >>>>> Tomas, >>>>> >>>>> Rather than dancing with setting/re-setting the umask, why shouldn't we >>>>> just explicitly set the perms of the file to exactly what we want at >>>>> time of creation? >>>>> >>>>> I noticed that the built-in open() function does not allow specification >>>>> of perms for cases where it creates the file, but the os.open() function >>>>> does, and that can be wrapped around an os.fdopen() call to be used with >>>>> 'with'. >>>>> >>>>> For example, the 'with' line in dhcp.py could be changed to the >>>>> following to achieve what we need: >>>>> >>>>> with os.fdopen(os.open(tmp_cfgfle, os.O_WRONLY | os.O_CREAT, 0644), 'w') >>>>> as tmp_cfg: >>>>> .... >>>>> ... >>>>> >>>>> >>>>> I'm no python expert so I don't know if there are any serious drawbacks >>>>> to using this versus using the built-in open(), but to me temporarily >>>>> setting something in the env (the umask in this case) across a chunk of >>>>> code seems susceptible to future bugs. >>>>> >>>>> >>>>> Alternatively, if the file in question is not a security-critical file >>>>> (which seem to be true for the files in this fix), what's wrong with >>>>> simply chmod'ing the file to exactly what we want right after its >>>>> creation? >>>>> >>>>> >>>>> For the changes in setup-sparc.sh, just adding the 'r' perm would still >>>>> leave some bits susceptible to the umask; so at a minimum, we should >>>>> just set it entirely to what we want, 0644 in this case. >>>>> >>>>> >>>>> -ethan >>>>> >>>>> >>>>> On 12/05/11 02:23, Tomas Dzik wrote: >>>>>> This is just a reminder that I would appreciate if someone could >>>>>> review these changes. >>>>>> >>>>>> Thanks a lot in advance, >>>>>> >>>>>> Tomas D. >>>>>> >>>>>> Dne 30.11.11 13:14, Tomas Dzik napsal(a): >>>>>>> Hi all, >>>>>>> I would like to ask you for a code review for bug: >>>>>>> >>>>>>> 7052679 AI Sparc client gets wantboot.conf error when booting due to >>>>>>> restrictive umask >>>>>>> Webrev: >>>>>>> >>>>>>> https://cr.opensolaris.org/action/browse/caiman/t.dzik/7052679/ >>>>>>> >>>>>>> Testing: >>>>>>> >>>>>>> 1) I built the gate with fix on x86 and on sparc. >>>>>>> 2) Using distro constructor I created AI iso image on both platforms >>>>>>> 3) I created virtual machines on x86 and on sparc and updated these >>>>>>> machines to the built gate >>>>>>> 4) I set umask for root to 0066 (as reported in bug) >>>>>>> 5) On x86 I created install service. For sparc I created it from >>>>>>> publisher and for x86 I created it from built AI iso image >>>>>>> 6) I verified permissions of wanboot.conf and system.conf >>>>>>> 7) I modified manifest, added it as a new default manifest and >>>>>>> installed >>>>>>> x86 client (also on virtual machine) from this AI server. >>>>>>> I observed that client installed correctly. >>>>>>> 8) On sparc I created install service for sparc from built iso, added >>>>>>> new default manifest and also added new profile. (All command were run >>>>>>> with umask set to 0066.) >>>>>>> 9) I installed sparc client from this AI server and verified that >>>>>>> client >>>>>>> installed correctly and used the right manifest and profile. >>>>>>> 10) Sources are pep8 clean. >>>>>>> >>>>>>> Best regards, >>>>>>> >>>>>>> Tomas D. >>>>>>> >>>>>>> _______________________________________________ >>>>>>> caiman-discuss mailing list >>>>>>> [email protected] >>>>>>> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss >>>>>> >>>>>> _______________________________________________ >>>>>> caiman-discuss mailing list >>>>>> [email protected] >>>>>> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss >>>>> >>>>> >>>>> _______________________________________________ >>>>> caiman-discuss mailing list >>>>> [email protected] >>>>> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss >>>> >> > _______________________________________________ > caiman-discuss mailing list > [email protected] > http://mail.opensolaris.org/mailman/listinfo/caiman-discuss _______________________________________________ caiman-discuss mailing list [email protected] http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
