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

_______________________________________________
cobbler-devel mailing list
cobbler-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/cobbler-devel

Reply via email to