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
