Bug 915726 - rpm silently removes %config(noreplace) files when they become symlinks
rpm silently removes %config(noreplace) files when they become symlinks
Status: CLOSED WONTFIX
Product: Red Hat Enterprise Linux 6
Classification: Red Hat
Component: rpm (Show other bugs)
6.5
All Linux
medium Severity medium
: rc
: ---
Assigned To: packaging-team-maint
BaseOS QE Security Team
: Reopened, Triaged
Depends On: 445202
Blocks:
  Show dependency treegraph
 
Reported: 2013-02-26 06:35 EST by Miloslav Trmač
Modified: 2015-04-07 06:30 EDT (History)
12 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: 445202
Environment:
Last Closed: 2015-04-07 06:30:44 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)
Fix regression which causes data loss of config files when type changed (1.47 KB, patch)
2013-04-30 11:14 EDT, Stef Walter
stefw: review? (packaging-team-maint)
tmraz: review+
Details | Diff
Test case rpm: original config files (2.54 KB, application/x-rpm)
2013-04-30 11:46 EDT, Stef Walter
no flags Details
Test case rpm: replaced config files (2.77 KB, application/x-rpm)
2013-04-30 11:47 EDT, Stef Walter
no flags Details
config-1.spec (836 bytes, text/plain)
2013-04-30 11:47 EDT, Stef Walter
no flags Details
config-2.spec (1009 bytes, text/plain)
2013-04-30 11:47 EDT, Stef Walter
no flags Details
Patch for rpm-4.10.x (1.47 KB, patch)
2013-04-30 13:15 EDT, Stef Walter
no flags Details | Diff
Patch for rpm-4.10.x (1.41 KB, patch)
2013-05-01 03:30 EDT, Stef Walter
no flags Details | Diff

  None (edit)
Description Miloslav Trmač 2013-02-26 06:35:52 EST
This functionality is needed for the planned transition/compatibility path for migrating ca-certificates to the p11-kit-based implementation.

+++ This bug was initially created as a clone of Bug #445202 +++

Description of problem:
When a package contains files marked as %config(noreplace) and in a later
release of this package these files become symlinks, then these files are
silently removed and no .rpmsave file is kept.

Version-Release number of selected component (if applicable):
4.4.2.2-7.fc8

How reproducible:
always

Steps to Reproduce:
1. yum install mock-0.7.6-1.fc8
2. edit /etc/mock/fedora-5-i386-epel.cfg
3. yum update mock (to version 0.9.7-2.fc8)
  
Actual results:
after update /etc/mock/fedora-5-i386-epel.cfg is a symlink to epel-5-i386.cfg.
The original file fedora-5-i386-epel.cfg is lost.

Expected results:
1a) There should be a new symlink created: /etc/mock/fedora-5-i386-epel.cfg.rpmnew
1b) Or: fedora-5-i386-epel.cfg should be saved as epel-5-i386.cfg and a
epel-5-i386.cfg.rpmnew should be created
In conclusion: There should be copy of the fedora-5-i386-epel.cfg kept somewhere
and rpm should indicate, that manual intervention is needed.

--- Additional comment from Panu Matilainen on 2009-01-08 07:36:39 EST ---

Still happens in F10

--- Additional comment from Till Maas on 2010-01-03 17:04:10 EST ---

reproducible with rpm 4.7.2-1.fc12

--- Additional comment from Miloslav Trmač on 2013-02-26 06:28:57 EST ---

Created attachment 702800 [details]
old.spec

Still seeing this in rpm-4.10.3.1-1.fc18.x86_64.

To reproduce
> rpmbuild -ba old.spec 
> rpmbuild -ba new.spec 
> rpm -Uvh .../config-1-1.x86_64.rpm 
modify /etc/cfgxx/filename 
> rpm -Uvh .../config-2-1.x86_64.rpm 

Actual results: /etc/cfgxx/filename has been replaced by a symlink, throwing away the old contents.

Expected results: /etc/cfgxx/filename.rpmnew symlink created, /etc/cfgxx/filename unmodified.

--- Additional comment from Miloslav Trmač on 2013-02-26 06:29:23 EST ---

Created attachment 702801 [details]
new.spec
Comment 1 Miloslav Trmač 2013-02-26 06:38:02 EST
Tested on rpm-4.8.0-27.el6.i686.
Comment 3 Panu Matilainen 2013-02-26 06:58:33 EST
This should be fixed in rpm >= 4.11, along with myriad of other similar corner-cases in %config file handling. Backporting those fixes is an entirely different matter though, it'd be painful and risky to backport them even to 4.10, never mind 4.8.x of RHEL-6.

I dont remember the details off-hand and its *possible* this particular case might be borderline doable, but determining the feasibility will require much closer inspection.
Comment 4 RHEL Product and Program Management 2013-03-03 01:47:14 EST
This request was not resolved in time for the current release.
Red Hat invites you to ask your support representative to
propose this request, if still desired, for consideration in
the next release of Red Hat Enterprise Linux.
Comment 5 Panu Matilainen 2013-03-27 03:36:17 EDT
From https://bugzilla.redhat.com/show_bug.cgi?id=445202#c10:
> This is no longer a dependency of the System Certificates work. The
> processing of the symlinks required custom processing, which Kai has written
> a script for.

I take this is the same as "migrating ca-certificates to the p11-kit-based implementation". Closing NEXTRELEASE as per comment #3: backporting the related and required fixes to the much older codebase would be not only painful but very risky as well.
Comment 6 Stef Walter 2013-03-27 03:47:40 EDT
The custom solution that Kai did is fine for Fedora. It bends the rules of %config(noreplace), and does not produce .rpmnew files. The custom solution produces .rpmsave files. 

The above solution is not fine for RHEL 6.x, when we include the Shared System Certificates feature in RHEL 6.x. Administrators will expect the right .rpmnew behavior for their modified ca-bundle.crt and ca-bundle.trust.crt files.

Reopening on RHEL 6.5.
Comment 7 Panu Matilainen 2013-03-27 04:07:02 EDT
I'd suggest you investigate the script possibility for rhel-6 as well. We can certainly investigate whether this particular corner case is backportable within reason but I seriously doubt it.
Comment 8 Stef Walter 2013-03-27 04:57:29 EDT
I'm scratching my head on how to do such a script, but we can try. This is significantly different from the Fedora script, which is a hack that writes out an .rpmsave.

But I'll put down here what I understand so far. 

Goal: The new rpm is replacing certain %config(noreplace) files with symlinks. If those files have been modified then the symlinks should go to .rpmnew.

 * We would run 'rpm' from within the %pre to detect if the files in question 
   have been changed. Is running rpm recursively from within an rpm install's
   %pre section a tested and supported thing to do?

 * Once we know whether the files have changed, we would change the path to 
   which the symlinks coming from the new rpm are installed. How do we do this?
   Any ideas here?

 * Perhaps we could make a backup in %pre, let the symlink overwrite the file
   and then move the symlink in %post, and write the original file back.
   This is really racy and fragile.

But yes, the open question is whether this big nasty hack around the brokenness of rpm in 6.5 is worse than backporting an rpm fix for this case.
Comment 9 Panu Matilainen 2013-03-27 05:45:17 EDT
Calling rpm from within rpm is not an option at all. Even if it might seem to work on casual testing, it will go crazy sooner or later when you least suspect.
Something like the third option of always backing up (and perhaps shuffling it around if the backup happens to be identical to the created file) is on the lines I was thinking.

I wouldn't be suggesting such dirty kludges around what's obviously a bug in rpm if the area wasn't such a minefield. I know it sounds like a case of "how hard can it possibly be?" but I spent a good part of a month going through it last year and the answer is: very tricky, time-consuming and treacherous. The risk of introducing new regressions is high as the rpm 4.8.x test-suite barely touches this ground and upstream test-cases cannot be used without backporting a huge amount of test-suite related other work first. The related upstream work is way too invasive to backport as such, so I'll have to see whether this hole can be plugged with a bit of duct-tape separately.
Comment 10 Stef Walter 2013-03-27 06:07:27 EDT
(In reply to comment #9)
> Calling rpm from within rpm is not an option at all. Even if it might seem
> to work on casual testing, it will go crazy sooner or later when you least
> suspect.

Okay, so that prevents the whole concept of using a script to work around the rpm brokenness. Unless you know of another way to detect whether %config written by a previous rpm have changed or not?

> Something like the third option of always backing up (and perhaps shuffling
> it around if the backup happens to be identical to the created file) is on
> the lines I was thinking.

It wasn't listed as an option. But rather something we'd need to do in addition to detecting whether the %config(noreplace) file has been changed. So now the question is: How can we detect whether a %config file has changed without running 'rpm'?

But I agree that if we can detect the change of the file, then we can probably hack around this with the ugly script.

> I'll have to see whether this hole can be plugged with a bit of duct-tape
> separately.

Yes, I imagine that's what we'd have to do, if we can't find a way to make the script work.
Comment 11 Miloslav Trmač 2013-03-27 10:47:39 EDT
(In reply to comment #10)
> It wasn't listed as an option. But rather something we'd need to do in
> addition to detecting whether the %config(noreplace) file has been changed.
> So now the question is: How can we detect whether a %config file has changed
> without running 'rpm'?

An extremely ugly way might be to ship a %config(noreplace) file with known contents in the "new" RPM, and in %post check whether the file actually has the known contents (== has replaced an earlier unmodified file).

But when the "new" RPM is updated to an "even newer" RPM, the "even newer" RPM will see something else - the %post scrip thas replaced the file with known contents by a symlink.

So something like
> if ($path is not a symlink to $new_destination) && ($path is a regular file of known contents) {
>     rm -f $path; ln -s $new_destination $path; }


Can something like the above work?
Comment 12 Stef Walter 2013-03-27 11:59:44 EDT
That is really nasty, but it might work.
Comment 13 Panu Matilainen 2013-03-28 03:54:29 EDT
Yeah, detecting whether a config file has changed without calling rpm requires either shipping the known content separately, or shipping a digest (sha256 or whatever) of it so the script can compare it without having the actual content.

Ugly it is either way, sure.
Comment 14 Stef Walter 2013-03-28 04:16:26 EDT
(In reply to comment #13)
> Yeah, detecting whether a config file has changed without calling rpm
> requires either shipping the known content separately, or shipping a digest
> (sha256 or whatever) of it so the script can compare it without having the
> actual content.

Not *a* digest, but every possible digest for every one of these files ever shipped by RHEL 6.5. Otherwise boom, data loss for the customer. 

> Ugly it is either way, sure.

Not just ugly. Completely broken.
Comment 15 Panu Matilainen 2013-04-08 04:14:01 EDT
Oh well... devel ack for applying some minimal band-aid to rpm to get a backup created in this case.
Comment 17 Stef Walter 2013-04-30 10:27:54 EDT
After digging into it a bit, the bug was introduced by this patch:

commit e64bf5b93ab689e6031fce4489e4ae38ebaebef1
Author: Panu Matilainen <pmatilai@redhat.com>
Date:   Tue Aug 28 09:04:09 2007 +0300

    Avoid .rpmnew when the file hasn't changed in package (rhbz#194246)
    
    The current behavior of %config(noreplace) creates a .rpmnewfile iff the typ
    of the current file has been changed wrto what was originally installed.
    
    The patch changes this behavior so when old and new (in db and in package) i
    identical -> not changed, the function returns FA_SKIP -> it won't clobber
    anything, it simply skips installation of the file from the package.
    This patch handles also the opposite case when old and new packages contain
    %config symlink and we have regular file on disk.
    
    Patch from Tomas Mraz.

In particular this line change:

--- a/lib/rpmfi.c
+++ b/lib/rpmfi.c
@@ -579,7 +579,7 @@ fileAction rpmfiDecideFate(const rpmfi ofi, rpmfi nfi, int s
     if (newWhat == XDIR)
        return FA_CREATE;
 
-    if (diskWhat != newWhat)
+    if (diskWhat != newWhat && dbWhat != REG && dbWhat != LINK)
        return save;
     else if (newWhat != dbWhat && diskWhat != dbWhat)
        return save;
Comment 18 Stef Walter 2013-04-30 11:08:37 EDT
(In reply to comment #17)
> -    if (diskWhat != newWhat)
> +    if (diskWhat != newWhat && dbWhat != REG && dbWhat != LINK)
>         return save;
>      else if (newWhat != dbWhat && diskWhat != dbWhat)
>         return save;

So the problem here is that the additional conditions to the 'if' statement are intended to cause the execution to fall through to the MD5 code below. But what it does instead is cause execution to fall through to a later 'else if (dbWhat != newWhat)' which returns FA_CREATE.
Comment 19 Stef Walter 2013-04-30 11:14:58 EDT
Created attachment 741903 [details]
Fix regression which causes data loss of config files when type changed

This fixes a regression in e64bf5b93ab689e6031fce4489e4ae38ebaebef1
which caused modified %config(noreplace) files to be overwritten
when the file was replaced by a symlink in the rpm.

This patch adjusts the conditions to match the behavior of the original
patch while accounting for these additional corner cases.

e64bf5b adjusts the conditions in which changing of file types cause
file comparisons to occur. However in doing this it caused certain
cases to fall through directly to FA_CREATE without comparison.
Comment 20 Stef Walter 2013-04-30 11:43:58 EDT
Will attach test case files...

Behavior before the patch:

# rpm -i builds/config-1-1.noarch.rpm 
# rpm -V config
# echo "modified" > /etc/cfgxx/config-file; echo "modified" > /etc/cfgxx/becomes-link 
# rpm -V config
S.5....T.  c /etc/cfgxx/becomes-link
S.5....T.  c /etc/cfgxx/config-file
# ls -l /etc/cfgxx
total 8
-rw-r--r--. 1 root root 9 30. Apr 17:40 becomes-link
-rw-r--r--. 1 root root 9 30. Apr 17:40 config-file
lrwxrwxrwx. 1 root root 9 30. Apr 17:39 config-link -> /dev/null
# rpm -U builds/config-2-1.noarch.rpm 
warning: /etc/cfgxx/config-file created as /etc/cfgxx/config-file.rpmnew
# ls -l /etc/cfgxx
total 12
lrwxrwxrwx. 1 root root 11 30. Apr 17:40 becomes-link -> normal-file
-rw-r--r--. 1 root root  9 30. Apr 17:40 config-file
-rw-r--r--. 1 root root 48 30. Apr 17:37 config-file.rpmnew
lrwxrwxrwx. 1 root root  9 30. Apr 17:40 config-link -> /dev/null
-rw-r--r--. 1 root root 26 30. Apr 17:37 normal-file
# rpm -V config
S.5....T.  c /etc/cfgxx/config-file

Note the data loss of modifications to 'becomes-link' after the upgrade.
Comment 21 Stef Walter 2013-04-30 11:45:11 EDT
Expected behavior:

# /opt/build/bin/rpm -i builds/config-1-1.noarch.rpm 
# /opt/build/bin/rpm -V config
# echo "modified" > /etc/cfgxx/config-file; echo "modified" > /etc/cfgxx/becomes-link 
# /opt/build/bin/rpm -V config
S.5....T.  c /etc/cfgxx/becomes-link
S.5....T.  c /etc/cfgxx/config-file
# ls -l /etc/cfgxx
total 8
-rw-r--r--. 1 root root 9 30. Apr 17:38 becomes-link
-rw-r--r--. 1 root root 9 30. Apr 17:38 config-file
lrwxrwxrwx. 1 root root 9 30. Apr 17:38 config-link -> /dev/null
# /opt/build/bin/rpm -U builds/config-2-1.noarch.rpm 
warning: /etc/cfgxx/becomes-link created as /etc/cfgxx/becomes-link.rpmnew
warning: /etc/cfgxx/config-file created as /etc/cfgxx/config-file.rpmnew
# ls -l /etc/cfgxx
total 16
-rw-r--r--. 1 root root  9 30. Apr 17:38 becomes-link
lrwxrwxrwx. 1 root root 11 30. Apr 17:39 becomes-link.rpmnew -> normal-file
-rw-r--r--. 1 root root  9 30. Apr 17:38 config-file
-rw-r--r--. 1 root root 48 30. Apr 17:37 config-file.rpmnew
lrwxrwxrwx. 1 root root  9 30. Apr 17:39 config-link -> /dev/null
-rw-r--r--. 1 root root 26 30. Apr 17:37 normal-file
# /opt/build/bin/rpm -V config
SM5....T.  c /etc/cfgxx/becomes-link
S.5....T.  c /etc/cfgxx/config-file

Note that the new link 'becomes-link' has been redirected to 'becomes-link.rpmnew' as expected, and the original customer data remains in place.
Comment 22 Stef Walter 2013-04-30 11:46:21 EDT
Created attachment 741907 [details]
Test case rpm: original config files
Comment 23 Stef Walter 2013-04-30 11:47:17 EDT
Created attachment 741908 [details]
Test case rpm: replaced config files
Comment 24 Stef Walter 2013-04-30 11:47:38 EDT
Created attachment 741909 [details]
config-1.spec
Comment 25 Stef Walter 2013-04-30 11:47:56 EDT
Created attachment 741910 [details]
config-2.spec
Comment 26 Stef Walter 2013-04-30 13:15:17 EDT
Created attachment 741931 [details]
Patch for rpm-4.10.x
Comment 27 Stef Walter 2013-04-30 13:16:52 EDT
The relevant patches have been tested on:

tag: rpm-4.8.0-release (ie: RHEL 6.x)
branch: rpm 4.8.x (ie: 4.8.1)
branch: rpm 4.10.x (ie: 4.10.3.1, Fedora)

As Panu noted above, the patch is not necessary on rpm's git master (ie: 4.11.x)
Comment 28 Stef Walter 2013-05-01 03:30:43 EDT
Created attachment 742104 [details]
Patch for rpm-4.10.x
Comment 29 Tomas Mraz 2013-05-02 02:31:29 EDT
Comment on attachment 741903 [details]
Fix regression which causes data loss of config files when type changed

The patch does not regress the original bugfix that caused this regression and it should work for this bug report. I do not know though whether it does not regress some additional corner case. It's apparent that the rpmfiDecideFate() code is extremely sensitive to that.
Comment 30 Stef Walter 2013-05-03 15:16:51 EDT
It turns out we are unable to depend on this for delivery of the Shared System Certificates feature in RHEL 6.5. This is because rpm would need to be updated in a separate transaction than the rpms that rely on this fix.

We will either need to find a solution to work around this, or due to this bug, wait until 6.6 to deliver the Shared System Certificates feature, having first delivered this fix as part of 6.5.

In both cases, I still think this is a good thing to fix as a regression in RHEL 6.x. Others have reported this as a bug from time to time.
Comment 31 Panu Matilainen 2013-05-06 03:13:20 EDT
Stef, thanks for digging into this, your patch looks just the kind of minimal bandaid I had in mind but hadn't had a chance to look into it yet. 

I also hadn't realized that this was actually a regression introduced by Tomas' .rpmnew avoidance patch, I assumed it had always been broken... because it actually has always been broken on general level: deciding whether a backup is needed on an existing file cannot be correctly done before comparing the on-disk content to what's in db, so the whole pile of fooWhat vs barWhat comparisons was always incorrect to begin with:
http://rpm.org/gitweb?p=rpm.git;a=commitdiff;h=0936fbc7bb51f4643e05dfe9a98ac30c82d215e2
Comment 33 Stef Walter 2013-05-27 11:00:24 EDT
Panu thanks for looking at the patch.

(In reply to Panu Matilainen from comment #13)
> Yeah, detecting whether a config file has changed without calling rpm
> requires either shipping the known content separately, or shipping a digest
> (sha256 or whatever) of it so the script can compare it without having the
> actual content.

For the record, I tried this. And it doesn't work properly because rpm will always think the file has changed, after the new rpm has been installed. Since what's in the rpm does not match what the %post script later replaced it with. This then confuses future upgrades/downgrades.
Comment 34 Stef Walter 2013-05-28 09:45:22 EDT
(In reply to Stef Walter from comment #33)
> For the record, I tried this. And it doesn't work properly because rpm will
> always think the file has changed, after the new rpm has been installed.
> Since what's in the rpm does not match what the %post script later replaced
> it with. This then confuses future upgrades/downgrades.

By the way, this comment was about Mirek's and Panu's work around suggestions. 

Kai found a different work around which we have implemented in Fedora 19, which is stable and works, but obviously isn't beautiful.

See:

http://pkgs.fedoraproject.org/cgit/ca-certificates.git/tree/ca-certificates.spec

and this:

https://fedoraproject.org/wiki/QA:Testcase_Certificate_Trust:Upgrade_Bundle

Further explanation in bug 544376 comment 45
Comment 35 RHEL Product and Program Management 2013-07-25 17:10:25 EDT
This request was evaluated by Red Hat Product Management for
inclusion in a Red Hat Enterprise Linux release.  Product
Management has requested further review of this request by
Red Hat Engineering, for potential inclusion in a Red Hat
Enterprise Linux release for currently deployed products.
This request is not yet committed for inclusion in a release.
Comment 37 Ľuboš Kardoš 2015-04-07 06:30:44 EDT
Because the patch is very risky and this functionality is not needed any more, closing this bug as wontfix.

Note You need to log in before you can comment on or make changes to this bug.