Re: [Rpm-maint] [rpm-software-management/rpm] Invalid free / double free in readFile() / rpmkeys pre signature check (#147)

2017-02-02 Thread Panu Matilainen
Right, thanks for reporting. Fixed in commit 
db1a33c8d36868478f1e2d32261ab99c9b55756f. 

When you say "attached files", did you intend to attach something here or did 
you mean the packages in your other recent reports? At least I was not able to 
reproduce this with those, but found a reproducer from my own collection of 
fuzzed rpms.

If you have separate reproducer package(s) for this one, please attach although 
already fixed, bad packages are always useful.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/issues/147#issuecomment-277182065___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Does not the rpmbuild command execute (de)compression of files in parallel? (#113)

2017-02-02 Thread ニール・ゴンパ
I would hazard to say I would _almost never_ recommend switching to 
multithreaded stuff by default, after accounting for how it would potentially 
break DeltaRPM. Unless there's a way to make _that_ deterministic, I see no 
pathway that would allow for widespread usage of multithreaded compression.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/issues/113#issuecomment-277152331___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Allow SOURCE_DATE_EPOCH to override file timestamps (#144)

2017-02-02 Thread ニール・ゴンパ
@toabctl I'm not particularly a fan of the late byte-compilation technique 
Debian uses, and I'd rather not propagate that down to everyone.

However, @bmwiedemann's idea of getting the .py files set to 
`$SOURCE_DATE_EPOCH` to embed that in .pyc files is interesting.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/144#issuecomment-277151205___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


[Rpm-maint] [rpm-software-management/rpm] Invalid free / double free in readFile() / rpmkeys pre signature check (#147)

2017-02-02 Thread Hanno Böck
The attached files will cause an invalid free or double free. As they're both 
in the same code line I assume it's the same bug in different variations.

This only affects the git code, not the latest release (otherwise I wouldn't 
have reported it to a public bug tracker). This is obviously a very serious 
security issue.

```
==27173==ERROR: AddressSanitizer: attempting double-free on 0x61a12080 in 
thread T0:
#0 0x4cc500 in __interceptor_cfree.localalias.1 (/r/rpm/rpmkeys+0x4cc500)
#1 0x52db63 in readFile /f/rpm/rpm/lib/rpmchecksig.c:157:5
#2 0x52db63 in rpmpkgVerifySigs /f/rpm/rpm/lib/rpmchecksig.c:277
#3 0x52f31a in rpmcliVerifySignatures /f/rpm/rpm/lib/rpmchecksig.c:380:13
#4 0x50420d in main /f/rpm/rpm/rpmkeys.c:74:7
#5 0x7fca86edb78f in __libc_start_main (/lib64/libc.so.6+0x2078f)
#6 0x41c558 in _start (/r/rpm/rpmkeys+0x41c558)

0x61a12080 is located 0 bytes inside of 1153-byte region 
[0x61a12080,0x61a12501)
freed by thread T0 here:
#0 0x4cc500 in __interceptor_cfree.localalias.1 (/r/rpm/rpmkeys+0x4cc500)
#1 0x5c8bac in hdrblobRead /f/rpm/rpm/lib/header.c:1897:2
#2 0x52dab4 in readFile /f/rpm/rpm/lib/rpmchecksig.c:135:9
#3 0x52dab4 in rpmpkgVerifySigs /f/rpm/rpm/lib/rpmchecksig.c:277
#4 0x52f31a in rpmcliVerifySignatures /f/rpm/rpm/lib/rpmchecksig.c:380:13

previously allocated by thread T0 here:
#0 0x4cc6b8 in malloc (/r/rpm/rpmkeys+0x4cc6b8)
#1 0x664504 in rmalloc /f/rpm/rpm/rpmio/rpmmalloc.c:44:13
#2 0x52dab4 in readFile /f/rpm/rpm/lib/rpmchecksig.c:135:9
#3 0x52dab4 in rpmpkgVerifySigs /f/rpm/rpm/lib/rpmchecksig.c:277
#4 0x52f31a in rpmcliVerifySignatures /f/rpm/rpm/lib/rpmchecksig.c:380:13

SUMMARY: AddressSanitizer: double-free (/r/rpm/rpmkeys+0x4cc500) in 
__interceptor_cfree.localalias.1
```

```
==28859==ERROR: AddressSanitizer: attempting free on address which was not 
malloc()-ed: 0x7ffde9ad6100 in thread T0
#0 0x4cc500 in __interceptor_cfree.localalias.1 (/r/rpm/rpmkeys+0x4cc500)
#1 0x52db63 in readFile /f/rpm/rpm/lib/rpmchecksig.c:157:5
#2 0x52db63 in rpmpkgVerifySigs /f/rpm/rpm/lib/rpmchecksig.c:277
#3 0x52f31a in rpmcliVerifySignatures /f/rpm/rpm/lib/rpmchecksig.c:380:13
#4 0x50420d in main /f/rpm/rpm/rpmkeys.c:74:7
#5 0x7fee8e92378f in __libc_start_main (/lib64/libc.so.6+0x2078f)
#6 0x41c558 in _start (/r/rpm/rpmkeys+0x41c558)

AddressSanitizer can not describe address in more detail (wild memory access 
suspected).
SUMMARY: AddressSanitizer: bad-free (/r/rpm/rpmkeys+0x4cc500) in 
__interceptor_cfree.localalias.1
==28859==ABORTING
```

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/issues/147___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


[Rpm-maint] [rpm-software-management/rpm] selinux: use string_to_security_class() instead of class ID (#146)

2017-02-02 Thread Davide Cavalca
SELinux recommends to use string_to_security_class() instead of referencing
class IDs directly. This also fixes a build issue for systems that don't
include flask.h by default.

References:
https://selinuxproject.org/page/NB_Imp_SELinux-aware_Apps#Implementing_SELinux-aware_Applications_2
https://github.com/SELinuxProject/selinux/commit/76913d8adb61b5afe28fd3b4ce91feab29e284dd
You can view, comment on, or merge this pull request online at:

  https://github.com/rpm-software-management/rpm/pull/146

-- Commit Summary --

  * selinux: use string_to_security_class() instead of class ID

-- File Changes --

M plugins/selinux.c (2)

-- Patch Links --

https://github.com/rpm-software-management/rpm/pull/146.patch
https://github.com/rpm-software-management/rpm/pull/146.diff

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/146
___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] set SOURCE_DATE_EPOCH from changelog (#143)

2017-02-02 Thread Bernhard M. Wiedemann
I put it there because the spec says:
> Build systems MUST NOT overwrite this variable for child processes to consume 
> if it is already present.

But then, it probably does not apply if we had set it ourselves (and not the 
user) for the previous rpm to build.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/143#issuecomment-276958227___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] heap out of bounds read in copyTdEntry() (#133)

2017-02-02 Thread Panu Matilainen
-K aka --checksig has been just a popt alias to `rpmkeys` for quite some time 
now, and popt aliases don't work so great from a git checkout. Try using 
./rpmkeys -K instead.

Note that what that codepath does is vastly different from what happens during 
eg rpm -U or rpm -q which have their own, differently flawed signature checking.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/issues/133#issuecomment-276952744___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Allow SOURCE_DATE_EPOCH to override file timestamps (#144)

2017-02-02 Thread Bernhard M. Wiedemann
On 2017-02-02 13:46, Florian Festi wrote:
> I am not too keen on the use of global variable

Do you refer to "oneshot" or "SOURCE_DATE_EPOCH"?

> I wonder how we want to address the Python .pyc file issue

I was thinking that it would be better to do it like Debian and generate
those in %post
which also reduces the size of rpms and allows it to be noarch


-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/144#issuecomment-276951194___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Allow SOURCE_DATE_EPOCH to override file timestamps (#144)

2017-02-02 Thread Florian Festi
I am not too keen on the use of global variable(s) here. May be we can renew 
loading the environ variable for each build and store it in the spec or package 
variable instead.
While it may not matter much in practise I'd rather not like to add more 
technical debt there.

Other than that the patch looks good from a code POV.

I wonder how we want to address the Python .pyc file issue (and probably 
others). Not that this is a prerequisite for getting this patch in. We could 
try to leverage the file attribute mechanism or have a list of REs matching the 
FILECLASS tag. Resulting files could be given an mtime of SOURCE_DATE_EPOCH+1

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/144#issuecomment-276948134___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] set SOURCE_DATE_EPOCH from changelog (#143)

2017-02-02 Thread Florian Festi
Hmm, I am wondering if the 
getenv("SOURCE_DATE_EPOCH") == NULL
condition is really correct here. The problem is that rpmbuild might be used to 
build multiple packages in one go. But even if that was not possible someone 
using the API could.
Do we really need to give precedence of an already set  SOURCE_DATE_EPOCH 
environment variable over %source_date_epoch_from_changelog ?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/143#issuecomment-276941975___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] heap out of bounds read in copyTdEntry() (#133)

2017-02-02 Thread Hanno Böck
> Also it's perhaps worth pointing out that none of the packages in the series 
> crash nor pass through 'rpm -K' verification.

Maybe a bit offtopic here, but I noted that the "-K" parameter no longer works 
in the current git code. Is this intentional? (and if yes: why?) Because I 
specifically wanted to test this and look for pre-signature-verification-bugs, 
but I couldn't.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/issues/133#issuecomment-276938778___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] heap out of bounds read in copyTdEntry() (#133)

2017-02-02 Thread Panu Matilainen
Just FWIW, this is enough to catch all of #133, #135, #136, #138 and #139:

```
--- a/lib/header.c
+++ b/lib/header.c
@@ -255,6 +255,8 @@ static rpmRC hdrblobVerifyInfo(hdrblob blob, char **emsg)
if (end > info.offset)
goto err;
 
+   if (info.tag < HEADER_I18NTABLE)
+   goto err;
if (hdrchkType(info.type))
goto err;
if (hdrchkAlign(info.type, info.offset))
```

Hysterically there are no checks whatsoever on the tag values in rpm. In this 
particular case it's an out-of-place immutable region tag which causes 
assumptions in the code fail, in #135, #136, #138 and #139 it's a required tag 
replaced with a negative value. So catching stuff below the normal tag range 
will minimally cover all these. There are other layers present too, like 
missing sanity checks on tag types all over the place.

Also it's perhaps worth pointing out that none of the packages in the series 
crash nor pass through 'rpm -K' verification.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/issues/133#issuecomment-276936724___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Does not the rpmbuild command execute (de)compression of files in parallel? (#113)

2017-02-02 Thread Panu Matilainen
Multithreaded compression is not some magic holy grail. Sure it can make the 
compression phase faster but it requires considerably more memory and more 
importantly the compression ratio degrades so for distros it might be a bad 
choice. Also AFAICT deltarpm requires bit-by-bit equivalent compression 
everywhere to operate correctly and that's not the case if compression depends 
on thinks like number of processors on the system.

Point is, anybody considering enabling it needs to consider whether it's an 
actual win for a particular use-case - there *are* cases like CI builds where 
speed can be far more important than compression ratio.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/issues/113#issuecomment-276908337___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Does not the rpmbuild command execute (de)compression of files in parallel? (#113)

2017-02-02 Thread Geunsik Lim
> It is the decision of the distributions to support them and change the macros 
> accordingly. 
> Upstream may follow the consensus of the distributions later.

Right. However, I hope that rpm upstream will enable multi threaded xz 
compression by default for them. :) For example, 

```
# https://fedoraproject.org/wiki/Features/XZRpmPayloads
%define _compression_level7
%define _smp_mflags -j%(echo "`/usr/bin/getconf _NPROCESSORS_ONLN`")
%_source_payload   w%{_compression_level}T%{_smp_mflags}.xzdio
%_binary_payload   w%{_compression_level}T%{_smp_mflags}.xzdio
```

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/issues/113#issuecomment-276896275___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint