[prev in list] [next in list] [prev in thread] [next in thread] 

List:       opensuse-buildservice
Subject:    Re: BEWARE: OBS may allow source change after review
From:       Marcus =?utf-8?B?SMO8d2U=?= <suse-tux () gmx ! de>
Date:       2021-06-02 21:50:57
Message-ID: 20210602215057.eadqknihedjj6mlp () linux
[Download RAW message or body]

Hi,

On 2021-05-28 16:10:25 +0200, Ludwig Nussel wrote:
> tl;dr if you rely on package reviews for your development process in OBS
> make sure requests have a revision. Only osc shows the information you
> are looking for.
>
Note that just relying on the presence of a revision is not "sufficient"
because it can point to a link (see below).

<SNIP>

> AFAIK this feature has been in OBS since the very beginning, it's just
> well hidden as the official interfaces osc and the webui do add a
> revision when creating requests. There is no way to turn that off
> either. Also tools such as bots that use osc as python module
> automatically generate submit request with revision. This is not because
> the server enforces it but because the client code does it.

In case of the osc lib, it depends on how you use it. For instance, if
you create a request via

r = osc.core.Request()
r.add_action('submit', src_project='openSUSE:Tools', src_package='osc',
             tgt_project='home:Marcus_H', tgt_package='abc')
r.create(conf.config['apiurl'])

no revision is added. However, if you use

r.create(conf.config['apiurl'], addrevision=True)

the API takes care of adding a revision.

<SNIP>

> Meanwhile the factory-auto bot was enhanced to decline unversioned
> requests to Factory (thanks Fabian).
>
Does it also check if the specified revision points to an expanded
file set? For instance, let's assume that

- prj/tgt is a plain package (no _link file)
- prj/lnk is a link to prj/tgt

Now, create a request via

r = osc.core.Request()
rev = '40e1a6ff74681c68a001adc3ca0c6474'
r.add_action('submit', src_project='prj', src_package='lnk', src_rev=rev,
             tgt_project='prj', tgt_package='bar')
r.create(conf.config['apiurl'])

where 40e1a6ff74681c68a001adc3ca0c6474 points to an unexpanded file set
(that is, it has a _link file).

Such a request is displayed like this

$> osc rq show 1234
Request: #1234

  submit:       prj/lnk@40e1a6ff74681c68a001adc3ca0c6474 -> prj/bar

...
$>

Even if you now run "osc rq show -d 1234", the "expanded" diff is
displayed. That is, it is probably not apparent from a reviewer's
POV that rev 40e1a6ff74681c68a001adc3ca0c6474 is in fact a link.
Now, if a review is accepted, the "attacker" can modify prj/lnk's link
target. If the request is eventually accepted, the modified files
end up prj/bar.


Marcus
[prev in list] [next in list] [prev in thread] [next in thread]