Bug#911380: Adjust server/Debian/share/ltsp/ltsp-build-client-functions

2018-12-16 Thread Wolfgang Schweer 
On Sun, Dec 16, 2018 at 02:30:31PM +0100, Vagrant Cascadian wrote:
> On 2018-12-16, Vagrant Cascadian wrote:
> > On 2018-12-16, Wolfgang Schweer wrote:
> >> On Sun, Dec 16, 2018 at 09:30:21AM +0100, Vagrant Cascadian wrote:
> >>> On 2018-12-16, Vagrant Cascadian wrote:
> >>> > I'm now thinking of passing an option --trust-file-mirrors or something
> >>> > like that. Then the trust is explicit, consistent with apt behavior
> >>> > requiring to explicitly trust it. It wouldn't allow different levels of
> >>> > trust for different file mirror types, but it will at least be simpler
> >>> > to code...
> >>> 
> >>> And implemented, please test:
> >>> 
> >>>   
> >>> https://git.launchpad.net/ltsp/commit/?id=85f4e9585d996bde20620a775185d06a6f41dc46
> >>  
> >> The new option is evaluated too late; if set to "True", the sources.list 
> >> file contains 'deb True file:/// ... '.
> 
> Ah, I see the problem. It was not because it was evaluated too late, but
> because you thought it was a boolean. I should make it a proper boolean,
> rather than the way it's currently configured.
> 
> If using a configuration file, you'd want:
> 
>   TRUST_FILE_MIRROR="[trusted=yes]"
> 
> But I think it would be better as a boolean; I'll change it a bit more.

Good job! The current code is now working like expected.  

Thanks for all your work,

Wolfgang


signature.asc
Description: PGP signature

Bug#911380: Adjust server/Debian/share/ltsp/ltsp-build-client-functions

2018-12-16 Thread Wolfgang Schweer 
On Sun, Dec 16, 2018 at 02:30:31PM +0100, Vagrant Cascadian wrote:
> Ah, I see the problem. It was not because it was evaluated too late, but
> because you thought it was a boolean. I should make it a proper boolean,
> rather than the way it's currently configured.
> 
> If using a configuration file, you'd want:
> 
>   TRUST_FILE_MIRROR="[trusted=yes]"

Yes, I had  TRUST_FILE_MIRROR="True" in the configuration file and 
thought Boolean would be better (more consitent regarding other 
settings).
  
Wolfgang


signature.asc
Description: PGP signature

Bug#911380: Adjust server/Debian/share/ltsp/ltsp-build-client-functions

2018-12-16 Thread Vagrant Cascadian 
On 2018-12-16, Vagrant Cascadian wrote:
> On 2018-12-16, Wolfgang Schweer wrote:
>> On Sun, Dec 16, 2018 at 09:30:21AM +0100, Vagrant Cascadian wrote:
>>> On 2018-12-16, Vagrant Cascadian wrote:
>>> > I'm now thinking of passing an option --trust-file-mirrors or something
>>> > like that. Then the trust is explicit, consistent with apt behavior
>>> > requiring to explicitly trust it. It wouldn't allow different levels of
>>> > trust for different file mirror types, but it will at least be simpler
>>> > to code...
>>> 
>>> And implemented, please test:
>>> 
>>>   
>>> https://git.launchpad.net/ltsp/commit/?id=85f4e9585d996bde20620a775185d06a6f41dc46
>>  
>> The new option is evaluated too late; if set to "True", the sources.list 
>> file contains 'deb True file:/// ... '.

Ah, I see the problem. It was not because it was evaluated too late, but
because you thought it was a boolean. I should make it a proper boolean,
rather than the way it's currently configured.

If using a configuration file, you'd want:

  TRUST_FILE_MIRROR="[trusted=yes]"

But I think it would be better as a boolean; I'll change it a bit more.

live well,
  vagrant


signature.asc
Description: PGP signature

Bug#911380: Adjust server/Debian/share/ltsp/ltsp-build-client-functions

2018-12-16 Thread Vagrant Cascadian 
On 2018-12-16, Wolfgang Schweer wrote:
> On Sun, Dec 16, 2018 at 09:30:21AM +0100, Vagrant Cascadian wrote:
>> On 2018-12-16, Vagrant Cascadian wrote:
>> > I'm now thinking of passing an option --trust-file-mirrors or something
>> > like that. Then the trust is explicit, consistent with apt behavior
>> > requiring to explicitly trust it. It wouldn't allow different levels of
>> > trust for different file mirror types, but it will at least be simpler
>> > to code...
>> 
>> And implemented, please test:
>> 
>>   
>> https://git.launchpad.net/ltsp/commit/?id=85f4e9585d996bde20620a775185d06a6f41dc46
>  
> The new option is evaluated too late; if set to "True", the sources.list 
> file contains 'deb True file:/// ... '.

Thanks for testing!


> The attached patch works for me, please check. 
...
> diff --git a/server/Debian/share/ltsp/ltsp-build-client-functions 
> b/server/Debian/share/ltsp/ltsp-build-client-functions
> index 4fdd1dce..871714da 100644
> --- a/server/Debian/share/ltsp/ltsp-build-client-functions
> +++ b/server/Debian/share/ltsp/ltsp-build-client-functions
> @@ -16,16 +16,18 @@ add_mirrors() {
>  components="$COMPONENTS"
>  fi
>  
> +echo "deb $mirror $dist $components" >> $ROOT/etc/apt/sources.list
>  case $mirror in
>  file:///*)
> -echo "deb $TRUST_FILE_MIRROR $mirror $dist $components" >> 
> $ROOT/etc/apt/sources.list
> +if ! [ -z "$trust-file-mirror" ] ; then
> +# Option to enable trusted file mirrors:
> +# https://bugs.debian.org/911380
> +sed -i 's/deb/deb [trusted=yes]/' 
> $ROOT/etc/apt/sources.list
> +fi

$trust-file-mirror is not a valid variable name in shell, so I'm
guessing this worked for you by always triggering this codepath...

I'd rather not edit all sources.list entries and stick to the way I had
it, and just make sure it evaluates early enough...

Will try again! :)

live well,
  vagrant


signature.asc
Description: PGP signature

Bug#911380: Adjust server/Debian/share/ltsp/ltsp-build-client-functions

2018-12-16 Thread Wolfgang Schweer 
On Sun, Dec 16, 2018 at 09:30:21AM +0100, Vagrant Cascadian wrote:
> On 2018-12-16, Vagrant Cascadian wrote:
> > I'm now thinking of passing an option --trust-file-mirrors or something
> > like that. Then the trust is explicit, consistent with apt behavior
> > requiring to explicitly trust it. It wouldn't allow different levels of
> > trust for different file mirror types, but it will at least be simpler
> > to code...
> 
> And implemented, please test:
> 
>   
> https://git.launchpad.net/ltsp/commit/?id=85f4e9585d996bde20620a775185d06a6f41dc46
 
The new option is evaluated too late; if set to "True", the sources.list 
file contains 'deb True file:/// ... '.

The attached patch works for me, please check. 

Wolfgang
From c9eaca5529c3c8f212491a73c9f053bfd32f69c5 Mon Sep 17 00:00:00 2001
From: Wolfgang Schweer 
Date: Sun, 16 Dec 2018 13:27:31 +0100
Subject: [PATCH] Debian: ltsp-build-client: Evaluate the --trust-file-mirror
 commandline option in ltsp-build-client-functions (doing it in
 010-manage-mirror is too late). https://bugs.debian.org/911380

---
 server/Debian/share/ltsp/ltsp-build-client-functions   | 10 ++
 .../plugins/ltsp-build-client/Debian/010-manage-mirror |  5 -
 2 files changed, 6 insertions(+), 9 deletions(-)

diff --git a/server/Debian/share/ltsp/ltsp-build-client-functions b/server/Debian/share/ltsp/ltsp-build-client-functions
index 4fdd1dce..871714da 100644
--- a/server/Debian/share/ltsp/ltsp-build-client-functions
+++ b/server/Debian/share/ltsp/ltsp-build-client-functions
@@ -16,16 +16,18 @@ add_mirrors() {
 components="$COMPONENTS"
 fi
 
+echo "deb $mirror $dist $components" >> $ROOT/etc/apt/sources.list
 case $mirror in
 file:///*)
-echo "deb $TRUST_FILE_MIRROR $mirror $dist $components" >> $ROOT/etc/apt/sources.list
+if ! [ -z "$trust-file-mirror" ] ; then
+# Option to enable trusted file mirrors:
+# https://bugs.debian.org/911380
+sed -i 's/deb/deb [trusted=yes]/' $ROOT/etc/apt/sources.list
+fi
 dir=$(echo "$mirror" | sed -e 's,^file://,,g')
 mkdir -p $ROOT/$dir
 chroot_mount $dir $dir --bind
 ;;
-*)
-echo "deb $mirror $dist $components" >> $ROOT/etc/apt/sources.list
-;;
 esac
 done
 }
diff --git a/server/share/ltsp/plugins/ltsp-build-client/Debian/010-manage-mirror b/server/share/ltsp/plugins/ltsp-build-client/Debian/010-manage-mirror
index 65f873ae..9e1a8f57 100644
--- a/server/share/ltsp/plugins/ltsp-build-client/Debian/010-manage-mirror
+++ b/server/share/ltsp/plugins/ltsp-build-client/Debian/010-manage-mirror
@@ -15,11 +15,6 @@ case "$MODE" in
 if [ -n "$option_extra_mirror_value" ]; then
 EXTRA_MIRROR="$option_extra_mirror_value"
 fi
-if [ -n "$option_trust_file_mirror_value" ]; then
-# Option to enable trusted file mirrors:
-# https://bugs.debian.org/911380
-TRUST_FILE_MIRROR="[trusted=yes]"
-fi
 ;;
 after-install)
 sources_list="$ROOT/etc/apt/sources.list"
-- 
2.20.0



signature.asc
Description: PGP signature

Bug#911380: Adjust server/Debian/share/ltsp/ltsp-build-client-functions

2018-12-16 Thread Vagrant Cascadian 
On 2018-12-16, Vagrant Cascadian wrote:
> I'm now thinking of passing an option --trust-file-mirrors or something
> like that. Then the trust is explicit, consistent with apt behavior
> requiring to explicitly trust it. It wouldn't allow different levels of
> trust for different file mirror types, but it will at least be simpler
> to code...

And implemented, please test:

  
https://git.launchpad.net/ltsp/commit/?id=85f4e9585d996bde20620a775185d06a6f41dc46

live well,
  vagrant


signature.asc
Description: PGP signature

Bug#911380: Adjust server/Debian/share/ltsp/ltsp-build-client-functions

2018-12-15 Thread Vagrant Cascadian 
On 2018-12-10, Wolfgang Schweer wrote:
> On Sat, Dec 08, 2018 at 03:08:22PM +0100, Vagrant Cascadian wrote:
>> On 2018-12-07, Vagrant Cascadian wrote:
>> But it's also possible to have some mirrors include a file:/// url
>> that's trusted and some that are not... so it needs to be specified on a
>> per-mirror basis.
>
> Just another try w/ conditionally added trust; the attached patch seems 
> to work for me, please check.

> Wolfgang
> From 7106088bcbd7d0a0a49144b66ab10ab2f943e9d6 Mon Sep 17 00:00:00 2001
> From: Wolfgang Schweer 
> Date: Mon, 10 Dec 2018 11:02:26 +0100
> Subject: [PATCH] Debian: ltsp-build-client: Trust file:/ mirrors that point to
>  /media/cdrom to be able to use a CDROM now that file:/// mirrors are no
>  longer trusted by default. https://bugs.debian.org/911380

This seems too fragile; if /media/cdrom changes yet again, it will
break. But the code I committed to ltsp upstream doesn't work either for
a wide variety of reasons... this is turning out to be very messy...

I'm now thinking of passing an option --trust-file-mirrors or something
like that. Then the trust is explicit, consistent with apt behavior
requiring to explicitly trust it. It wouldn't allow different levels of
trust for different file mirror types, but it will at least be simpler
to code...


It's also hard to invest in ltsp-build-client at this point;
ltsp-build-client should be deprecated entirely, as it's already not the
recommended way to install LTSP... but there's a lot of work left to do
to really drop it.


live well,
  vagrant

Bug#911380: Adjust server/Debian/share/ltsp/ltsp-build-client-functions

2018-12-10 Thread Wolfgang Schweer 
On Sat, Dec 08, 2018 at 03:08:22PM +0100, Vagrant Cascadian wrote:
> On 2018-12-07, Vagrant Cascadian wrote:
> But it's also possible to have some mirrors include a file:/// url
> that's trusted and some that are not... so it needs to be specified on a
> per-mirror basis.

Just another try w/ conditionally added trust; the attached patch seems 
to work for me, please check.

Wolfgang
From 7106088bcbd7d0a0a49144b66ab10ab2f943e9d6 Mon Sep 17 00:00:00 2001
From: Wolfgang Schweer 
Date: Mon, 10 Dec 2018 11:02:26 +0100
Subject: [PATCH] Debian: ltsp-build-client: Trust file:/ mirrors that point to
 /media/cdrom to be able to use a CDROM now that file:/// mirrors are no
 longer trusted by default. https://bugs.debian.org/911380

---
 .../Debian/share/ltsp/ltsp-build-client-functions   | 13 ++---
 .../ltsp-build-client/Debian/010-debootstrap|  6 +-
 2 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/server/Debian/share/ltsp/ltsp-build-client-functions b/server/Debian/share/ltsp/ltsp-build-client-functions
index ae019cc9..8fce9652 100644
--- a/server/Debian/share/ltsp/ltsp-build-client-functions
+++ b/server/Debian/share/ltsp/ltsp-build-client-functions
@@ -16,10 +16,17 @@ add_mirrors() {
 components="$COMPONENTS"
 fi
 
-echo "deb $mirror $dist $components" >> $ROOT/etc/apt/sources.list
 case $mirror in
-*file:///*)
-dir=$(echo "$mirror" | sed -e 's,^file://,,g')
+file:///media/cdrom)
+echo "deb [trusted=yes] $mirror $dist $components" >> $ROOT/etc/apt/sources.list
+;;
+*)
+echo "deb $mirror $dist $components" >> $ROOT/etc/apt/sources.list
+;;
+esac
+
+case $mirror in
+file:///*) dir=$(echo "$mirror" | sed -e 's,^file://,,g')
 mkdir -p $ROOT/$dir
 chroot_mount $dir $dir --bind
 ;;
diff --git a/server/share/ltsp/plugins/ltsp-build-client/Debian/010-debootstrap b/server/share/ltsp/plugins/ltsp-build-client/Debian/010-debootstrap
index f7543cb9..64374c73 100644
--- a/server/share/ltsp/plugins/ltsp-build-client/Debian/010-debootstrap
+++ b/server/share/ltsp/plugins/ltsp-build-client/Debian/010-debootstrap
@@ -30,12 +30,8 @@ case "$MODE" in
 echo "  $0 --dist etch"
 exit 1
 fi
-# Strip out apt options, such as [trusted=yes] file:/// ...
-# https://bugs.debian.org/911380
-DEBOOTSTRAP_MIRROR="$(echo $MIRROR | sed -e 's,\[.*\],,g')"
-
 # Install base packages
-LC_ALL=C ${DEBOOTSTRAP:-"debootstrap"} $DEBOOTSTRAPOPTS --arch $ARCH $DIST $ROOT $DEBOOTSTRAP_MIRROR
+LC_ALL=C ${DEBOOTSTRAP:-"debootstrap"} $DEBOOTSTRAPOPTS --arch $ARCH $DIST $ROOT $MIRROR
 if [ -x "$CROSS_ARCH_EMULATOR" ]; then
 # configured for cross architecture install.  this requires a
 # statically built emulator such as qemu, and binfmt_misc
-- 
2.20.0.rc2



signature.asc
Description: PGP signature

Bug#911380: Adjust server/Debian/share/ltsp/ltsp-build-client-functions

2018-12-08 Thread Vagrant Cascadian 
On 2018-12-07, Vagrant Cascadian wrote:
> On 2018-10-20, Wolfgang Schweer wrote:
>> Due to security concerns, file:/// repositories are no longer considered to 
>> be
>> trusted by default. If a complete ISO image is used to install LTSP in 
>> offline
>> mode, such a repository is actually present. Adding [trusted=yes] enables it.
...
> Rather than hard-coding that file mirrors are always trusted, can't you
> instead use:
>
>   mirror='deb [trusted=yes] file:///some/file/path DIST COMPONENTS'
>
> There may be cases where file mirrors still may require verification.

Looks like this isn't going to work... $MIRROR is also passed directly
to debootstrap, and so it would require the debootstrap plugin to
process out the [trusted=yes] (and while at it, possibly other settings
passed in this manner).

I still think it's possible to have a file:/// url that is signed
properly, so I'm hesitant to hard-code this...

But it's also possible to have some mirrors include a file:/// url
that's trusted and some that are not... so it needs to be specified on a
per-mirror basis.

I guess the thing to do would be for debootstrap to exclude [.*] from
the mirror, and then pass "[trusted=yes other_arbitrary_options=X]
file:/// ..." and then it would work quite generically.


live well,
  vagrant


signature.asc
Description: PGP signature

Bug#911380: Adjust server/Debian/share/ltsp/ltsp-build-client-functions

2018-12-07 Thread Vagrant Cascadian 
On 2018-10-20, Wolfgang Schweer wrote:
> From e643e524802668c43ced718df07c43e80d1978dd Mon Sep 17 00:00:00 2001
> From: Wolfgang Schweer 
> Date: Sat, 20 Oct 2018 10:36:57 +0200
> Subject: [PATCH] Adjust server/Debian/share/ltsp/ltsp-build-client-functions
>  to be apt compliant.
>
> Due to security concerns, file:/// repositories are no longer considered to be
> trusted by default. If a complete ISO image is used to install LTSP in offline
> mode, such a repository is actually present. Adding [trusted=yes] enables it.
> ---
>  server/Debian/share/ltsp/ltsp-build-client-functions | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/server/Debian/share/ltsp/ltsp-build-client-functions 
> b/server/Debian/share/ltsp/ltsp-build-client-functions
> index b338847c..28421b82 100644
> --- a/server/Debian/share/ltsp/ltsp-build-client-functions
> +++ b/server/Debian/share/ltsp/ltsp-build-client-functions
> @@ -18,7 +18,9 @@ add_mirrors() {
>  
>  echo "deb $mirror $dist $components" >> $ROOT/etc/apt/sources.list
>  case $mirror in
> -file:///*) dir=$(echo "$mirror" | sed -e 's,^file://,,g')
> +file:///*)
> + sed -i 's/deb/deb [trusted=yes]/' $ROOT/etc/apt/sources.list
> +dir=$(echo "$mirror" | sed -e 's,^file://,,g')
>  mkdir -p $ROOT/$dir
>  chroot_mount $dir $dir --bind
>  ;;
> -- 
> 2.19.1

Rather than hard-coding that file mirrors are always trusted, can't you
instead use:

  mirror='deb [trusted=yes] file:///some/file/path DIST COMPONENTS'

There may be cases where file mirrors still may require verification.

Thanks to Alkis for the reminder about this option...

live well,
  vagrant


signature.asc
Description: PGP signature