On 05.09.2014 [15:00:45 -0700], Nishanth Aravamudan wrote:
On 02.09.2014 [08:25:12 -0700], Nishanth Aravamudan wrote:
> On 31.08.2014 [10:15:01 +0200], J?rgen Maas wrote:
> > This is definitely a bug, i will take a look at it.
> >
> > FYI, the master branch is slated to become cobbler 3.0, this means
> > functionality and API may (and shall) change or can be removed altogether
> > (eg. s390/itanium).
>
> Yep, that's fine with me, to be honest. But one thing I'd like to see
> improved is the commit logs indicating what is functionally changing
> (even if just the intent). The logs right now are a bit terse and it
> takes looking at the actual commit to know what they do often :)
>
> > One other example is the remote kickstart URL feature; this has been
> > removed on purpose also as a result of multiple CVE reports.
> > Kickstarts *must* reside on the Cobbler server and also they *must* be in
> > the directory /var/lib/cobbler/kickstarts/
>
> I wonder if it would be possible to provide a mirroring? That is, if the
> kickstart referenced is remote, can it not be mirrored over as part of
> the sync and thus be local for the deployment itself? I suppose if
> that's the case, the admin could just mirror them over.
>
> I do think there's one use-case that is no longer supported, but isn't a
> CVE -- users testing custom kickstarts. Presuming a good authorization
> model, regular users may not be allowed to edit the Cobbler-wide
> kickstarts. But they might want to test some custom partitioning, or
> feature flag, and would like to specify that in their kickstart file?
FYI, found another bug introduced by this commit:
remote.write_kickstart_template now calls
file_path = validate.kickstart_file_path(file_path, for_item=False)
but that does the following check:
if not os.path.isfile(kickstart):
raise CX("Invalid kickstart template file location %s, file not found" %
kickstart)
which doesn't make a lot of sense for writing the kickstart from the
webUI? (on creating a new one).
Something like the following? I'll send through a github pull request if
it seems reasonable:
diff --git a/cobbler/remote.py b/cobbler/remote.py
index 835d29f..069e167 100644
--- a/cobbler/remote.py
+++ b/cobbler/remote.py
@@ -1991,7 +1991,7 @@ class CobblerXMLRPCInterface:
what = "write_kickstart_template"
self._log(what, name=file_path, token=token)
self.check_access(token, what, file_path, True)
- file_path = validate.kickstart_file_path(file_path, for_item=False)
+ file_path = validate.kickstart_file_path(file_path, for_item=False, write=True)
try:
utils.mkdir(os.path.dirname(file_path))
diff --git a/cobbler/validate.py b/cobbler/validate.py
index ff63b8a..3727edf 100644
--- a/cobbler/validate.py
+++ b/cobbler/validate.py
@@ -84,7 +84,7 @@ def snippet_file_path(snippet):
return snippet
-def kickstart_file_path(kickstart, for_item=True):
+def kickstart_file_path(kickstart, for_item=True, write=False):
"""
Validate the kickstart file path.
@@ -113,7 +113,7 @@ def kickstart_file_path(kickstart, for_item=True):
if not kickstart.startswith(KICKSTART_TEMPLATE_BASE_DIR):
raise CX("Invalid kickstart template file location %s, it is not inside
- if not os.path.isfile(kickstart):
+ if not os.path.isfile(kickstart) and not write:
raise CX("Invalid kickstart template file location %s, file not found"
return kickstart