[PATCH] builddeb: fix file permissions before packaging

[PATCH] builddeb: fix file permissions before packaging

[Henning Schild]

Builddep is not very explicit about file permissions. Actually the file
permissions in the package are largely influenced by the umask of the
user cloning the git and building the package. If that umask does not
set go+r the resulting linux-headers package will prevent non-root users
from building out-of-tree modules. And that is probably just one
unexpected effect.
Being a packaging/install tool builddep should make sure the file
permissions are set correctly and not just derived from a value that is
never checked.

This patch derives the "r" and "x" bits for all files from the file
owner, all directories in the package will be 755.

Signed-off-by: Henning Schild <henning.schild@siemens.com>
---
 scripts/package/builddeb | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/scripts/package/builddeb b/scripts/package/builddeb
index 86e56fe..615e0ed 100755
--- a/scripts/package/builddeb
+++ b/scripts/package/builddeb
@@ -26,6 +26,11 @@ create_package() {
 	# Fix ownership and permissions
 	chown -R root:root "$pdir"
 	chmod -R go-w "$pdir"
+	# in case we are in a restrictive umask environment like 0077
+	# set permissions for dirs and derive "go"-"rx" permissions from "u"
+	find $pdir -type d -exec chmod 0755 {} +
+	find $pdir -type f -perm -u+r -exec chmod go+r {} +
+	find $pdir -type f -perm -u+x -exec chmod go+x {} +
 
 	# Create the package
 	dpkg-gencontrol $forcearch -Vkernel:debarch="${debarch}" -p$pname -P"$pdir"
-- 
2.7.3

Re: [PATCH] builddeb: fix file permissions before packaging

[Henning Schild]

On Thu, 7 Jul 2016 16:50:37 +0200
Henning Schild <henning.schild@siemens.com> wrote:

> Builddep is not very explicit about file permissions. Actually the
> file permissions in the package are largely influenced by the umask
> of the user cloning the git and building the package. If that umask
> does not set go+r the resulting linux-headers package will prevent
> non-root users from building out-of-tree modules. And that is
> probably just one unexpected effect.

This problem was found in a CI-system (buildbot) running with a 0077
umask and building packages. These packages contained headers with 600
instead of 644 and users could not build there modules anymore.

Reproduce with "umask 0077", "git clone ..", ... "make deb-pkg"

> Being a packaging/install tool builddep should make sure the file
> permissions are set correctly and not just derived from a value that
> is never checked.

If that patch is not accepted i strongly suggest at least asserting
that the headers in linux-headers.deb have 644. I am not sure which is
the better solution, but builddeb should not produce such broken
packages.

> This patch derives the "r" and "x" bits for all files from the file
> owner, all directories in the package will be 755.
> 
> Signed-off-by: Henning Schild <henning.schild@siemens.com>
> ---
>  scripts/package/builddeb | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/scripts/package/builddeb b/scripts/package/builddeb
> index 86e56fe..615e0ed 100755
> --- a/scripts/package/builddeb
> +++ b/scripts/package/builddeb
> @@ -26,6 +26,11 @@ create_package() {
>  	# Fix ownership and permissions
>  	chown -R root:root "$pdir"
>  	chmod -R go-w "$pdir"
> +	# in case we are in a restrictive umask environment like 0077
> +	# set permissions for dirs and derive "go"-"rx" permissions
> from "u"
> +	find $pdir -type d -exec chmod 0755 {} +
> +	find $pdir -type f -perm -u+r -exec chmod go+r {} +
> +	find $pdir -type f -perm -u+x -exec chmod go+x {} +
>  
>  	# Create the package
>  	dpkg-gencontrol $forcearch -Vkernel:debarch="${debarch}"
> -p$pname -P"$pdir"

Re: [PATCH] builddeb: fix file permissions before packaging

[Michal Marek]

On 2016-07-07 16:50, Henning Schild wrote:
> Builddep is not very explicit about file permissions. Actually the file
> permissions in the package are largely influenced by the umask of the
> user cloning the git and building the package. If that umask does not
> set go+r the resulting linux-headers package will prevent non-root users
> from building out-of-tree modules. And that is probably just one
> unexpected effect.
> Being a packaging/install tool builddep should make sure the file
> permissions are set correctly and not just derived from a value that is
> never checked.
> 
> This patch derives the "r" and "x" bits for all files from the file
> owner, all directories in the package will be 755.
> 
> Signed-off-by: Henning Schild <henning.schild@siemens.com>
> ---
>  scripts/package/builddeb | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/scripts/package/builddeb b/scripts/package/builddeb
> index 86e56fe..615e0ed 100755
> --- a/scripts/package/builddeb
> +++ b/scripts/package/builddeb
> @@ -26,6 +26,11 @@ create_package() {
>  	# Fix ownership and permissions
>  	chown -R root:root "$pdir"
>  	chmod -R go-w "$pdir"
> +	# in case we are in a restrictive umask environment like 0077
> +	# set permissions for dirs and derive "go"-"rx" permissions from "u"
> +	find $pdir -type d -exec chmod 0755 {} +
> +	find $pdir -type f -perm -u+r -exec chmod go+r {} +
> +	find $pdir -type f -perm -u+x -exec chmod go+x {} +

This can be simplified as

chmod -R a+rX "$pdir"

Michal

[PATCHv2] builddeb: fix file permissions before packaging

[Henning Schild]

Builddep is not very explicit about file permissions. Actually the file
permissions in the package are largely influenced by the umask of the
user cloning the git and building the package. If that umask does not
set go+r the resulting linux-headers package will prevent non-root users
from building out-of-tree modules. And that is probably just one
unexpected effect.
Being a packaging/install tool builddep should make sure the file
permissions are set correctly and not just derived from a value that is
never checked.

This patch sets ugo read permissions for all packaged files and derives
the executable bit for directories and executables from the file-owner.

Signed-off-by: Henning Schild <henning.schild@siemens.com>
---
 scripts/package/builddeb | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/scripts/package/builddeb b/scripts/package/builddeb
index 86e56fe..f72ba49 100755
--- a/scripts/package/builddeb
+++ b/scripts/package/builddeb
@@ -26,6 +26,8 @@ create_package() {
 	# Fix ownership and permissions
 	chown -R root:root "$pdir"
 	chmod -R go-w "$pdir"
+	# in case we are in a restrictive umask environment like 0077
+	chmod -R a+rX "$pdir"
 
 	# Create the package
 	dpkg-gencontrol $forcearch -Vkernel:debarch="${debarch}" -p$pname -P"$pdir"
-- 
2.7.3

Re: [PATCHv2] builddeb: fix file permissions before packaging

[Michal Marek]

Dne 22.7.2016 v 14:46 Henning Schild napsal(a):
> Builddep is not very explicit about file permissions. Actually the file
> permissions in the package are largely influenced by the umask of the
> user cloning the git and building the package. If that umask does not
> set go+r the resulting linux-headers package will prevent non-root users
> from building out-of-tree modules. And that is probably just one
> unexpected effect.
> Being a packaging/install tool builddep should make sure the file
> permissions are set correctly and not just derived from a value that is
> never checked.
> 
> This patch sets ugo read permissions for all packaged files and derives
> the executable bit for directories and executables from the file-owner.
> 
> Signed-off-by: Henning Schild <henning.schild@siemens.com>
> ---
>  scripts/package/builddeb | 2 ++
>  1 file changed, 2 insertions(+)

Applied to kbuild.git#misc, thanks.

Michal

Re: [PATCH] builddeb: fix file permissions before packaging

[Henning Schild]

Am Fri, 22 Jul 2016 14:15:32 +0200
schrieb Michal Marek <mmarek@suse.com>:

> On 2016-07-07 16:50, Henning Schild wrote:
> > Builddep is not very explicit about file permissions. Actually the
> > file permissions in the package are largely influenced by the umask
> > of the user cloning the git and building the package. If that umask
> > does not set go+r the resulting linux-headers package will prevent
> > non-root users from building out-of-tree modules. And that is
> > probably just one unexpected effect.
> > Being a packaging/install tool builddep should make sure the file
> > permissions are set correctly and not just derived from a value
> > that is never checked.
> > 
> > This patch derives the "r" and "x" bits for all files from the file
> > owner, all directories in the package will be 755.
> > 
> > Signed-off-by: Henning Schild <henning.schild@siemens.com>
> > ---
> >  scripts/package/builddeb | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/scripts/package/builddeb b/scripts/package/builddeb
> > index 86e56fe..615e0ed 100755
> > --- a/scripts/package/builddeb
> > +++ b/scripts/package/builddeb
> > @@ -26,6 +26,11 @@ create_package() {
> >  	# Fix ownership and permissions
> >  	chown -R root:root "$pdir"
> >  	chmod -R go-w "$pdir"
> > +	# in case we are in a restrictive umask environment like
> > 0077
> > +	# set permissions for dirs and derive "go"-"rx"
> > permissions from "u"
> > +	find $pdir -type d -exec chmod 0755 {} +
> > +	find $pdir -type f -perm -u+r -exec chmod go+r {} +
> > +	find $pdir -type f -perm -u+x -exec chmod go+x {} +  
> 
> This can be simplified as
> 
> chmod -R a+rX "$pdir"

That 'X' is much nicer than the finds, learned something new today,
Thanks!

Henning

> Michal