Merge lp:~roadmr/ubuntu/oneiric/casper/809885 into lp:ubuntu/oneiric/casper

[Daniel Manrique]

Modifies the regexps used to process resolv.conf entries in scripts/casper-bottom/23networking.

They now strip single quotes from all entries taken from file written by ipconfig, to ensure the generated /etc/resolv.conf is formatted correctly.

Fixes bug 809885.

[Colin Watson (cjwatson)]

To be honest, I think this just highlights the bad way casper was handling this file before you touched it (which is my fault, since apparently I introduced this code). Rather than patching around the problem, could you just make casper read ipconfig's output as shell input using '. /tmp/net-"${DEVICE}".conf', and then we can just use the resulting shell variables? That would be more reliable long-term than trying to keep up with which way ipconfig currently quotes shell variables.

You mention arbitrary code execution in your bug report; but I don't think that's a concern here. All this code is running as root, and if ipconfig wanted to cause casper to run some evil code then it could just do so itself. Furthermore, it's all running in the initramfs, where there is no possibility that a malicious user might have injected data into /tmp. The current code in casper tries to parse ipconfig's output like this because it's wrong, not because it's clever.

review: Needs Fixing

[lp:~roadmr/ubuntu/oneiric/casper/809885]

906. By Daniel Manrique on 2011-07-14

Reworked as per cjwatson's suggestions.
scripts/casper-bottom/23networking: source file written by ipconfig
and use the resulting environment variables to build resolv.conf.
(LP: #809885).

907. By Daniel Manrique on 2011-07-14

merged from trunk

[Daniel Manrique (roadmr)]

Hi Colin,

Thanks for your comments!

> To be honest, I think this just highlights the bad way casper was handling
> this file before you touched it (which is my fault, since apparently I
> introduced this code). Rather than patching around the problem, could you
> just make casper read ipconfig's output as shell input using '.
> /tmp/net-"${DEVICE}".conf', and then we can just use the resulting shell
> variables? That would be more reliable long-term than trying to keep up with
> which way ipconfig currently quotes shell variables.

Agreed, looking at ipconfig's history and bug reports it looks like just sourcing the file and using the variables is the recommended way to work with it.

The updated branch does this, I chose not to put the $DNSDOMAIN and IPV4DNS{0,1} directly in the heredoc for clarity, and also because we still need to strip all but the first entry from the data that will go in the "domain" field of resolv.conf. This, by the way, is the only remaining regex in that section of code, and I simplified it a bit. So preparing the variables beforehand and then just outputting them in the heredoc looked like the clearest way to do it.

I hope I didn't make a mess with the branch, my revision 907 should merge cleanly against trunk but the commit history looks a bit messy, let me know if you'd prefer a new branch with just one commit.

> You mention arbitrary code execution in your bug report; but I don't think
> that's a concern here. All this code is running as root, and if ipconfig
> wanted to cause casper to run some evil code then it could just do so itself.
> Furthermore, it's all running in the initramfs, where there is no possibility
> that a malicious user might have injected data into /tmp. The current code in
> casper tries to parse ipconfig's output like this because it's wrong, not
> because it's clever.

Well, this motivation was mentioned in the klibc changelog, I'm not sure what incident prompted it but I agree that for Ubiquity the risk is nearly nonexistent.

Do let me know if this code looks fine or still needs more work.

Thanks again!

review: Needs Resubmitting

[lp:~roadmr/ubuntu/oneiric/casper/809885]

908. By Daniel Manrique on 2011-07-15

fixed regular expression to process the domain entry in resolv.conf

[Daniel Manrique (roadmr)]

Oops, I hadn't been able to test this with an actual Oneiric install image. I did so today and realized that the new regex to process the domain entry is not working well. I went back to the old one. My sincerest apologies for the slip-up and the merge-spam.

Thanks!

[Jon Peatfield (jp107-e)]

I was just testing a PXE boot setup of 11.10 and ran into this issue, ie resolv.conf ended up with ' (single quotes) round the items making it invalid so DNS name lookup didn't work.

This was for the (current I think) 11.10 .iso images for both i386 and amd64. Unpicking the initrd.lz and applying the above patch to the scripts/casper-bottom/23networking seems to cure the problem for me.

Maybe I'm missing something - won't this affect anyone using the 11.10 livecd versions on a network, or does it only break things when casper is started with an existing network config - as when PXE booting?

Unmerged revisions

908. By Daniel Manrique on 2011-07-15

fixed regular expression to process the domain entry in resolv.conf

907. By Daniel Manrique on 2011-07-14

merged from trunk

906. By Daniel Manrique on 2011-07-14

Reworked as per cjwatson's suggestions.
scripts/casper-bottom/23networking: source file written by ipconfig
and use the resulting environment variables to build resolv.conf.
(LP: #809885).

905. By Daniel Manrique on 2011-07-13

scripts/casper-bottom/23networking: Strip single quotes from all entries
taken from file written by ipconfig, to ensure the generated
/etc/resolv.conf is formatted correctly (LP: #809885).

=== modified file 'debian/changelog'
--- debian/changelog	2011-07-13 18:09:53 +0000
+++ debian/changelog	2011-07-15 14:46:50 +0000
@@ -1,8 +1,13 @@
 casper (1.272) UNRELEASED; urgency=low
 
+  * scripts/casper-bottom/23networking: source file written by ipconfig
+    and use the resulting environment variables to build resolv.conf.
+    (LP: #809885).
+
+  [Andy Whitcroft]
   * scripts/casper: add support for overlayfs for union mounts of the liveCD
 
- -- Andy Whitcroft <[e-mail]>  Mon, 27 Jun 2011 11:49:39 +0100
+ -- Daniel Manrique <[e-mail]>  Wed, 14 Jul 2011 13:53:13 -0400
 
 casper (1.271) oneiric; urgency=low
 

=== modified file 'scripts/casper-bottom/23networking'
--- scripts/casper-bottom/23networking	2011-01-28 21:32:33 +0000
+++ scripts/casper-bottom/23networking	2011-07-15 14:46:50 +0000
@@ -77,17 +77,16 @@
         if [ -n "${DEVICE}" ] && [ -e /tmp/net-"${DEVICE}".conf ]; then
             # create a resolv.conf if it is not present
             cp /tmp/net-"${DEVICE}".conf /root/var/log/netboot.config
-            #ipconfig quotes DNSDOMAIN, quotes need to be removed for a correct resolv.conf
-            rc_search="$(sed -n 's/"//g;s/^DNSDOMAIN=//p' /tmp/net-"${DEVICE}".conf)"
+            #ipconfig-generated file contains sourceable environment variables
+            . /tmp/net-"${DEVICE}".conf
+            rc_search="${DNSDOMAIN}"
             #search might contain multiple entries but domain should only have one.
-            rc_domain="$(sed -n -e 's/"//g;s/^DNSDOMAIN=\([^ ]\+\) *.*/\1/p'  /tmp/net-"${DEVICE}".conf)"
-            rc_server0="$(sed -n 's/^IPV4DNS0=//p' /tmp/net-"${DEVICE}".conf)"
-            rc_server1="$(sed -n 's/^IPV4DNS1=//p' /tmp/net-"${DEVICE}".conf)"
-            rc_server0="nameserver ${rc_server0}"
-            if [ "${rc_server1}" = "0.0.0.0" ]; then
+            rc_domain="$(echo "${DNSDOMAIN}" | sed -n -e 's/^\([^ ]\+\) *.*/\1/p')"
+            rc_server0="nameserver ${IPV4DNS0}"
+            if [ "${IPV4DNS1}" = "0.0.0.0" ]; then
                 rc_server1=""
             else
-                rc_server1="nameserver ${rc_server1}"
+                rc_server1="nameserver ${IPV4DNS1}"
             fi
             cat > /root/etc/resolv.conf <<EOF
 # /etc/resolv.conf