Hi John,
is that really a question or you think that I should add these test ? ;-)
Also because we are fixing the issue with root having too restrictive
umask - do you think that these tests should run with restrictive umask
? If yes - should the restrictive umask be set just in particular test
case or as part of setUpClass ?
Regards,
Tomas D.
On 12.12.2011 15:31, John Fischer wrote:
Tomas,
These changes look good. My only question now is should the tests be updated
to check the permissions of these files that get created? The only one that
looks
a little challenging to do would be the dhcp.py test.
Thanks,
John
On Dec 12, 2011, at 2:56 AM, Tomas Dzik wrote:
Hi all,
the updated webrev is here:
https://cr.opensolaris.org/action/browse/caiman/t.dzik/7052679-3/
Could you please let me know whether it is OK now ?
Best regards,
Tomas D.
Dne 9.12.11 01:34, Ethan Quach napsal(a):
On 12/08/11 09:42, Tomas Dzik wrote:
Hi John,
it's easy to change that code to use umask instead of chmod. On the
other hand it's very hard to get consensus on this mailing list how to
do such simple thing like changing ownership of a file in a shell script.
So I would like to know definitive answer before I will change the
code (it should take 2 minutes) and retest it (it takes me more then 3
hours to build it on both platforms, run tests, update test ai servers
on both platforms and to create service and install client machines).
And this is already 3rd version of this fix.
Ethan what do you think ?
John's suggestion sounds fine to me.
-ethan
Best regards,
Tomas D.
Dne 8.12.11 18:15, John Fischer napsal(a):
Tomas,
In that case can you please use a umask so that the permissions are
correct
when the file is created.
Thanks,
John
On Dec 8, 2011, at 9:06 AM, Tomas Dzik wrote:
Dne 8.12.11 17:51, John Fischer napsal(a):
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
Hi John,
there is no particular reason for using chmod in sparc-setup.sh
code. Both ways would work. I just picked one.
Best regards,
Tomas D.
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
