Bug 218556

Summary: Review Request: ecryptfs-utils - Linux eCryptfs utilities
Product: [Fedora] Fedora Reporter: Michael Halcrow <mhalcrow>
Component: Package ReviewAssignee: Bill Nottingham <notting>
Status: CLOSED NEXTRELEASE QA Contact: Fedora Package Reviews List <fedora-package-review>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: bjohnson, buytenh, dhowells, esandeen, kevin, konradr, mgarski, mhlavink, mnowak, notting, pvrabec, roland.wolters, rvokal
Target Milestone: ---Flags: notting: fedora-review+
gwync: fedora-cvs+
Target Release: ---   
Hardware: All   
OS: Linux   
URL: http://ecryptfs.sourceforge.net/
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2007-06-29 14:56:40 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On:    
Bug Blocks: 221596    
Attachments:
Description Flags
dmesg
none
Some printk() and a BUG_ON() to help track down unmount oops
none
backtrace from dmesg on x86_64 machine none

Description Michael Halcrow 2006-12-05 23:28:57 UTC
Spec URL: http://downloads.sourceforge.net/ecryptfs/ecryptfs-utils.spec?use_mirror=osdn
SRPM URL: http://downloads.sourceforge.net/ecryptfs/ecryptfs-utils-5-0.src.rpm?use_mirror=osdn
Description: eCryptfs is a stacked cryptographic filesystem that ships in Linux kernel versions 2.6.19 and later. This package provides userspace utilities necessary to manage eCryptfs mounts.

Comment 1 Kevin Fenzi 2006-12-06 03:58:16 UTC
Hey Michael. 

From a quick look, it looks like this is your first package, so you will need a
sponsor. I am going to add the FE-NEEDSPONSOR blocker here so sponsors can see
your package. 

You may want to take a look at: 
http://fedoraproject.org/wiki/Extras/HowToGetSponsored

Comment 2 Bernard Johnson 2006-12-08 10:19:02 UTC
I will provide you a review.  It is not an official review as you need a sponsor.

rpmlint -i ecryptfs-utils-5-0.src.rpm 
W: ecryptfs-utils summary-not-capitalized eCryptfs mount helper and support
libraries
Summary doesn't begin with a capital letter.
- This can be ignored.

W: ecryptfs-utils no-url-tag
The URL tag is missing.
-add:
URL: http://ecryptfs.sourceforge.net

W: ecryptfs-utils setup-not-quiet
You should use -q to have a quiet extraction of the source tarball, as this
generate useless lines of log ( for buildbot, for example )
- add "-q" flag to %setup

W: ecryptfs-utils rpm-buildroot-usage %build ./configure
--prefix=$RPM_BUILD_ROOT/usr
$RPM_BUILD_ROOT should not be touched during %build or %prep stage, as it
will break short circuiting.
- change to "%{_configure}

E: ecryptfs-utils configure-without-libdir-spec
A configure script is run without specifying the libdir. configure
options must be augmented with something like --libdir=%{_libdir}.

E: ecryptfs-utils hardcoded-library-path in $RPM_BUILD_ROOT/usr/lib/ecryptfs
A library path is hardcoded to one of the following paths: /lib,
/usr/lib. It should be replaced by something like /%{_lib} or %{_libdir}.
- change:
  /usr/bin to %{_bindir}
  /usr/lib to %{_libdir}

E: ecryptfs-utils hardcoded-library-path in /usr/lib/libecryptfs.so.0.0.0
A library path is hardcoded to one of the following paths: /lib,
/usr/lib. It should be replaced by something like /%{_lib} or %{_libdir}.
- ditto

E: ecryptfs-utils hardcoded-library-path in /usr/lib/libecryptfs.so.0
A library path is hardcoded to one of the following paths: /lib,
/usr/lib. It should be replaced by something like /%{_lib} or %{_libdir}.
- ditto

E: ecryptfs-utils hardcoded-library-path in /usr/lib/libecryptfs.so
A library path is hardcoded to one of the following paths: /lib,
/usr/lib. It should be replaced by something like /%{_lib} or %{_libdir}.
- ditto

E: ecryptfs-utils hardcoded-library-path in
/usr/lib/ecryptfs/libecryptfs_pki_passphrase.so
A library path is hardcoded to one of the following paths: /lib,
/usr/lib. It should be replaced by something like /%{_lib} or %{_libdir}.
- ditto

E: ecryptfs-utils hardcoded-library-path in
/usr/lib/ecryptfs/libecryptfs_pki_openssl.so
A library path is hardcoded to one of the following paths: /lib,
/usr/lib. It should be replaced by something like /%{_lib} or %{_libdir}.
- ditto

E: ecryptfs-utils no-buildroot-tag
The BuildRoot tag isn't used in your spec. It must be used in order to
allow building the package as non root on some systems.
- add:
BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)


You might consider installing rpmdevtools and running fedora-newrpmspec to get a
nice template spec file to work from.  Templated spec files make it faster for
reviewers to review.

Additionally:
make does not use smp flags (see template spec file)
make install does not use DESTDIR (see template spec file)
%defattr has missing param (see template spec file)


Question for submitter: Is ecryptfs already in the kernel?  If it's not, this
would be a blocker until it is.

Once these changes are made, here is the probably output from rpmlint on the
binary rpms:
rpmlint -i mock-results/ecryptfs-utils-5-0.i386.rpm 
E: ecryptfs-utils explicit-lib-dependency libgcrypt
You must let rpm find the library dependencies by itself. Do not put unneeded
explicit Requires: tags.
- remove dependency on libgcrypt

W: ecryptfs-utils summary-not-capitalized eCryptfs mount helper and support
libraries
Summary doesn't begin with a capital letter.
- ignore

W: ecryptfs-utils no-version-in-last-changelog
The last changelog entry doesn't contain a version. Please insert the
version that is coherent with the version of the package and rebuild it.
- please add a version to the changelog entry:
* Mon Dec 04 2006 Mike Halcrow <mhalcrow.com> - 5-0

W: ecryptfs-utils unstripped-binary-or-object
/usr/lib/ecryptfs/libecryptfs_pki_passphrase.so
W: ecryptfs-utils unstripped-binary-or-object
/usr/lib/ecryptfs/libecryptfs_pki_openssl.so
- I believe the problem here is that they were not chmod a+x so they are not
stripped... however, see below first.

E: ecryptfs-utils library-without-ldconfig-postin /usr/lib/libecryptfs.so.0.0.0
This package contains a library and provides no %post scriptlet containing
a call to ldconfig.
- add
  %post -p /sbin/ldconfig

E: ecryptfs-utils library-without-ldconfig-postun /usr/lib/libecryptfs.so.0.0.0
This package contains a library and provides no %postun scriptlet containing
a call to ldconfig.
- add
  %postun -p /sbin/ldconfig

W: ecryptfs-utils devel-file-in-non-devel-package /usr/lib/libecryptfs.so
A development file (usually source code) is located in a non-devel
package. If you want to include source code in your package, be sure to
create a development package.
- If these are development libraries, they should be removed entirely, or put in
a -devel package.  If they are loadable modules, I believe this can be ignored,
but I'll have to look it up.

E: ecryptfs-utils zero-length /usr/share/doc/ecryptfs-utils-5/ChangeLog
- if changelog is 0 length, remove it from the installation



Please fix these issues and repost a new srpm and spec file and I'll continue
your review.

Comment 3 Michael Halcrow 2006-12-09 00:01:06 UTC
Bernard -

Thanks for taking the time to look over the package. I have made updates in
response to your comments. The new SPEC file is at
<http://downloads.sourceforge.net/ecryptfs/ecryptfs-utils.spec?use_mirror=osdn>.
The new source RPM is at
<http://downloads.sourceforge.net/ecryptfs/ecryptfs-utils-6-0.src.rpm?use_mirror=osdn>.

In response to your question, eCryptfs is in the 2.6.19 mainline kernel.

Comment 4 Bernard Johnson 2006-12-09 04:06:39 UTC
rpmlint is much happier now.  Here is the output on the srpm and rpm:

W: ecryptfs-utils summary-not-capitalized eCryptfs mount helper and support
libraries
W: ecryptfs-utils devel-file-in-non-devel-package /usr/lib/libecryptfs.so

Package mock-builds for FC6.

As far as the first one, just to keep lint quit, maybe you'd change the summary
from "eCryptfs mount helper ..." to "The eCryptfs mount helper...".

As far as the second regarding libcryptfs.so, it should be removed from the
package if it is a development file and you don't intend to provide a -devel
package.  I see there are still .so files in /usr/lib/ecryptfs although you
never stated their purpose.  Are they dl'opened modules or develop libraries?

It's recommended that the Source directives are direct download links.  It's
also recommended that your first source sould be Source0:
Source0: http://downloads.sourceforge.net/ecryptfs/ecryptfs-utils-6.tar.bz2

Since its support in the kernel starts at a specific version number, the package
should require this number or greater.  This is a little more complicated that
it sounds, because although a kernel may have a number like 2.6.18, the kernel
developers might have (probably did) pulled in additional snapshot patches.  You
should check with the upstream RPM developers of the kernel or verify for
yourself what kernel number (exactly which RPM release) ecryptfs support starts in.

MUST Items:
- MUST: A package must own all directories that it creates. If it does not
create a directory that it uses, then it should require a package which does
create that directory. The exception to this are directories listed explicitly
in the Filesystem Hierarchy Standard ([WWW]
http://www.pathname.com/fhs/pub/fhs-2.3.html), as it is safe to assume that
those directories exist.

Take ownership of %{_libdir}/ecryptfs via a %dir statement or by recommendation
below.

- MUST: If a package contains library files with a suffix (e.g. libfoo.so.1.1),
then library files that end in .so (without suffix) must go in a -devel package.

See previous comments regarding .so files.



A couple of recommendations:

This is redundant, as make install will do it for you:
mkdir -p $RPM_BUILD_ROOT/sbin
mkdir -p $RPM_BUILD_ROOT/%{_libdir}/ecryptfs
mkdir -p $RPM_BUILD_ROOT/%{_bindir}
mkdir -p $RPM_BUILD_ROOT/%{_mandir}/man7

These lines:
%{_libdir}/ecryptfs/libecryptfs_pki_passphrase.so
%{_libdir}/ecryptfs/libecryptfs_pki_openssl.so
can be replaced with:
%{_libdir}/ecryptfs
which will include the files and directory and solve the directory ownership
problem above.


Comment 5 Bernard Johnson 2006-12-10 01:11:30 UTC
Sorry, I should have left the FE-NEW blocker since I wasn't sponsored.  Fixing that.

Comment 6 Michael Halcrow 2006-12-11 22:28:16 UTC
Updated SPEC is here:
http://downloads.sourceforge.net/ecryptfs/ecryptfs-utils.spec?use_mirror=osdn

Updated SRPM is here:
http://downloads.sourceforge.net/ecryptfs/ecryptfs-utils-7-0.src.rpm?use_mirror=osdn

I will have to get in touch with the kernel package maintainers about eCryptfs
in older kernel versions, but I am almost positive that eCryptfs is not in Red
Hat kernels prior to 2.6.19, since that would involve some backport work. For
now, I have set the kernel version requirement to 2.6.19 or greater. I have also
added a -debug package for the header file and the libecryptfs.so file. The
libraries in /usr/lib/ecryptfs are dlopen'd.

Comment 7 Bernard Johnson 2006-12-12 01:08:21 UTC
I think you missed what I meant regarding kernel versions.  Here's an example of
a kernel spec file for a version of kernel-2.6.18:

http://cvs.fedora.redhat.com/viewcvs/rpms/kernel/devel/kernel-2.6.spec?rev=1.2834&only_with_tag=kernel-2_6_18-1_2834_fc7&view=markup

Notice these patches:
Patch1: patch-2.6.19-rc6.bz2
Patch2: patch-2.6.19-rc6-git10.bz2

Does this mean that a 2.6.18 kernel supports ecryptfs?  Probably, I dunno.  But
that's what I'm looking for in a Require:  Remember you're requires is against
the functionality of the RPM, not what was released upstream.

Comment 8 Michael Halcrow 2006-12-12 01:39:55 UTC
If ``2.6.18'' is really ``2.6.19-rc6,'' then yes, it does support eCryptfs. I'll
ask around about this.

Comment 9 Bernard Johnson 2006-12-12 06:00:30 UTC
FC6 2.6.18-1.2849: no 2.6.19 git patches
FC6 test 2.6.18-1.2860: no 2.6.19 git patches

So FC6 will probably never support it.

rawhide 2.6.18-1.2849.fc6 (not yet respun)

CVS head holds 2.6.19, so the next rawhide respin will be a 2.6.19 kernel.

Therefore, I think requires >= 2.6.19 is ok for this case.


I re-reviewed your spec and the rpmlint output and I don't see any further problems.

One thing to note... In the future, please prepend to your %changelog as you
make any changes.

This ends my review.

Comment 10 Need Real Name 2006-12-20 12:37:07 UTC
The -devel package has Requires: keyutils-lib-dev openssl-dev, both of those
need s/dev/devel/ for the -devel package to be installable.

Comment 11 Kevin Fenzi 2006-12-22 04:11:07 UTC
Hey Michael. 

I'm interested in this package, any chance of an updated version with
corrections from comment #10? 

Note that if the xen mess ever gets figured out in fc6, there will probibly be a
2.6.19 for it, so this may well work in fc6 someday. (At least that is my
understanding)

Comment 13 Kevin Fenzi 2007-01-04 03:58:54 UTC
Hey Michael.

First one general query: You are upstream for this package, do you intend to
only maintain this one package in fedora, or are you planning on submitting
more. If you are just seeking to make sure this is in fedora, perhaps someone
who is already a maintainer might step up to do so. This would save you having
to go through the sponsorship process and also take a task off your hands if you
don't want to do it. ;) 

If you do have additional packages you want to maintain, you might submit 
them as well. Multiple submissions and/or adding review comments to other 
packages helps sponsors see that you know the guidelines and are ready to 
be sponsored. 

That said, here are some general comments/suggestions on this package: 

- You might use the %{?dist} tag. This is very handy when you push the same
version of a package to multiple releases, so upgrades are still smooth. 
http://fedoraproject.org/wiki/Packaging/DistTag

- Build on x86_64 doesn't work. The build process ends with: 
RPM build errors:
    File not found:
/var/tmp/ecryptfs-utils-8-1.fc7-root-mockbuild/usr/lib64/ecryptfs

It looks like the package is installing /usr/lib/ecryptfs on x86_64, when
it should be installing in /usr/lib64/ecryptfs.

If you need a x86_64 test machine, I have one handy and would be happy 
to provide you an account on it. 

- When going through a review, do add a changelog entry to your spec file and 
increment the version for each change. That helps reviewers see that items 
were addressed and when. There shouldn't be a need to increase the version 
upstream unless there were changes to the upstream package. 


Comment 15 Michael Halcrow 2007-01-05 20:55:35 UTC
I don't have access to bug #221596. Is anyone at liberty to disclose what it is?

Comment 16 Michael Halcrow 2007-01-16 22:58:00 UTC
Updated SPEC is here:
http://downloads.sourceforge.net/ecryptfs/ecryptfs-utils.spec

Updated SRPM is here:
http://downloads.sourceforge.net/ecryptfs/ecryptfs-utils-9-0.src.rpm

Kevin Fenzi (kevin) wrote:
> First one general query: You are upstream for this package, do you
> intend to only maintain this one package in fedora, or are you
> planning on submitting more.

This is the only package I intend on maintaining in Fedora at the
moment.

> If you are just seeking to make sure this is in fedora, perhaps
> someone who is already a maintainer might step up to do so.

Perhaps.

> This would save you having to go through the sponsorship process and
> also take a task off your hands if you don't want to do it. ;)

I would like to take the path of least resistance to get eCryptfs
support into Fedora, but I would also like the ability to push changes
into the package without having to bother a middleman. The userspace
utilities change about as rapidly as the kernel module, which is
probably going to be changing on every 2.6.x kernel release for at
least the next year.

> - You might use the %{?dist} tag. This is very handy when you push
> the same version of a package to multiple releases, so upgrades are
> still smooth.  http://fedoraproject.org/wiki/Packaging/DistTag

From what I understand, I can just do this?

---
diff --git a/rpm/ecryptfs-utils.spec b/rpm/ecryptfs-utils.spec
index f6e42ea..6d717f7 100644
--- a/rpm/ecryptfs-utils.spec
+++ b/rpm/ecryptfs-utils.spec
@@ -1,6 +1,6 @@
 Name: ecryptfs-utils
 Version: 9
-Release: 0
+Release: 0%{?dist}
 Summary: The eCryptfs mount helper and support libraries
 Group: System Environment/Base
 License: GPL
---

> - Build on x86_64 doesn't work. The build process ends with: 
> RPM build errors:
>     File not found:
> /var/tmp/ecryptfs-utils-8-1.fc7-root-mockbuild/usr/lib64/ecryptfs
>
> It looks like the package is installing /usr/lib/ecryptfs on x86_64,
> when it should be installing in /usr/lib64/ecryptfs.

This required some changes to the build, but I believe I have this
remedied in the most recent version of the package.

Mike

Comment 17 Kevin Fenzi 2007-01-24 04:24:08 UTC
In reply to comment #16:

>This is the only package I intend on maintaining in Fedora at the
>moment.

ok.

>> This would save you having to go through the sponsorship process and
>> also take a task off your hands if you don't want to do it. ;)

> I would like to take the path of least resistance to get eCryptfs
> support into Fedora, but I would also like the ability to push changes
> into the package without having to bother a middleman. The userspace
> utilities change about as rapidly as the kernel module, which is
> probably going to be changing on every 2.6.x kernel release for at
> least the next year.

Indeed. Understandable.

I would really like to see this package in as well.
So, I can review this package and sponsor you once it's
in an acceptable state.

> From what I understand, I can just do this?
...snipp dist tag patch...

Yes, that should do it.

Some other notes/observations before a formal review:

1. Your version scheme seems a bit odd. Is there any reason why
you do major integer releases every time? Now that 9 is out
it's hard to go to a more traditional 'work toward a stable 1.0',
but it might be worth considering just minor bumps for minor
changes?

2. x86_64 now builds here. :) As does ppc32. 

3. The 2.6.19 kernel in fc6 updates has the ecryptfs module,
so there should be support for this in fc6 with the updated
kernel at least. Not sure how we are going to require that
however. Wonder if we can do a 'Requires: ecryptfs.ko'.

Look for a formal review here soon... 

Comment 18 Kevin Fenzi 2007-01-24 05:24:52 UTC
OK - Package meets naming and packaging guidelines
OK - Spec file matches base package name.
OK - Spec has consistant macro usage.
OK - Meets Packaging Guidelines.
OK - License (GPL)
OK - License field in spec matches
OK - License file included in package
OK - Spec in American English
OK - Spec is legible.
OK - Sources match upstream md5sum:
e6afeeb371c5d7d8c8d3c6dc379dd11a  ecryptfs-utils-9.tar.bz2
e6afeeb371c5d7d8c8d3c6dc379dd11a  ecryptfs-utils-9.tar.bz2.1
bbd72d4036e2b0faf7a8a4ca204fed274c6849cd  ecryptfs-utils-9.tar.bz2
bbd72d4036e2b0faf7a8a4ca204fed274c6849cd  ecryptfs-utils-9.tar.bz2.1
OK - BuildRequires correct
OK - Package has %defattr and permissions on files is good.
OK - Package has a correct %clean section.
OK - Package has correct buildroot
OK - Packages %doc files don't affect runtime.

OK - Headers/static libs in -devel subpackage.
OK - Spec has needed ldconfig in post and postun
OK - .so files in -devel subpackage.
See below - -devel package Requires: %{name} = %{version}-%{release}

OK - Package compiles and builds on at least one arch.
OK - Package has no duplicate files in %files.
OK - Package doesn't own any directories other packages own.
OK - Package owns all the directories it creates.
See below - No rpmlint output.
See below - final provides and requires are sane:

SHOULD Items:

OK - Should build in mock.
OK - Should build on all supported archs
See below - Should have subpackages require base package with fully versioned 
depend.
See below - Should have dist tag
OK - Should package latest version

Issues:

1. Might include the following as %doc files:
AUTHORS NEWS THANKS

2. The devel subpackage should have:
Requires: %{name} = %{version}-%{release}
You have currently => and just version.

3. rpmlint says:

E: ecryptfs-utils binary-or-shlib-defines-rpath /usr/bin/ecryptfsd ['/usr/
lib64']
E: ecryptfs-utils binary-or-shlib-defines-rpath /usr/bin/ecryptfs-manager ['/
usr/lib64']
E: ecryptfs-utils binary-or-shlib-defines-rpath /sbin/mount.ecryptfs ['/usr/
lib64']

See:
http://fedoraproject.org/wiki/Packaging/Guidelines#head-
a1dfb5f46bf4098841e31a75d833e6e1b3e72544

4. The keyutils-libs Requires doesn't seem needed.
keyutils pulls it in, and thats already required.

5. Should add the dist tag, as discussed in previous comments.

A few other notes, not related to packaging:

- http://ecryptfs.sourceforge.net/README seems to be out of date?

- I tried the example in the README, doing:
mkdir /root/crypt /mnt/crypt; mount -t ecryptfs /root/crypt /mnt/crypt
It prompts me for a passphrase, etc, but then I don't see the dir mounted
and writing files just appears to write to the dirs, am I missing a step?
I see reports of someone perhaps having the same issue on a x86_64 box,
which I am also using... Installing the i386 version gets me:
Error mounting ecryptfs

- I'm not sure what the README means by a 'layover mount', can you expand?

- If it would help you are welcome to use my x86_64 test box
to track down x86_64 issues.


Comment 19 Kevin Fenzi 2007-01-24 05:41:52 UTC
FYI, on my x86_64 box with either the x86_64 or i386 package installed, and my 
ppc box, I get the "Error mounting ecryptfs" and in dmesg I see: 

ecryptfs_parse_options: Could not find key with description: [15194abfa1fef279]
process_request_key_err: Unknown error code: [0x00000000ffffff82]
Error parsing options; rc = [-22]


Comment 20 Michael Halcrow 2007-01-24 19:31:33 UTC
Kevin Fenzi wrote:
> ecryptfs_parse_options: Could not find key with description: [15194abfa1fef279]

It sounds like there is a problem getting the key into your user session
keyring. This seems to be becoming a more common problem recently. We are in the
process of updating the userspace tools to do some preliminary sanity checks of
the keyring before attempting to move forward with the mount. The failure
conditions could be more gracefully handled in the userspace mount helper too.

After you attempt a mount, what is the output of ``keyctl show''?

Comment 21 Peter Vrabec 2007-01-25 14:59:11 UTC
Created attachment 146549 [details]
dmesg

I got this error during unmount. Tested on 
Linux dhcp-lab-160.brq.redhat.com 2.6.19-1.2912.fc7 #1 SMP Sun Jan 14 20:08:35
EST 2007 x86_64 x86_64 x86_64 GNU/Linux

Comment 22 Michael Halcrow 2007-01-25 19:52:24 UTC
Created attachment 146611 [details]
Some printk() and a BUG_ON() to help track down unmount oops

Add some printk() and a BUG_ON() statements to help track down unmount oops.

Comment 23 Michael Halcrow 2007-01-25 19:55:13 UTC
Peter Vrabec wrote:
> Created an attachment (id=146549) [edit]
> dmesg
>
> I got this error during unmount.

I think this is something I've seen before, but I don't know how to reproduce
it. Could you provide a sequence of operations that will invoke this stack trace?

In the meantime, I added a patch that might help debug this issue. The stack
trace suggests to me that an inode mutex in the lower filesystem is being locked
and is not being properly unlocked somewhere along the way. Right now I suspect
a missing unlock in ecryptfs_d_release(), but until I can find a way to
reproduce this on my end, I'm sort of in the dark.

Comment 24 Peter Vrabec 2007-01-26 10:09:46 UTC
#mkdir crypt
#mkdir foo
#mount -t ecryptfs crypt foo
Passphrase:
Verify Passphrase:
Cipher
1) AES-128
2) AES-192
3) AES-256
4) CAST5
5) Twofish
6) Triple-DES
7) Blowfish
8) CAST6
Selection [AES-128]:
Attempting to mount with the following options:[enter]
  ecryptfs_cipher=aes
  ecryptfs_key_bytes=16
  ecryptfs_sig=945f0e28ae2dfba0
Mounted ecryptfs
#cd foo
#vim xxx
#cd ..
#umount foo
Neoprávněný přístup do paměti (SIGSEGV)

Message from syslogd@dhcp-lab-160 at Fri Jan 26 09:56:52 2007 ...
dhcp-lab-160 kernel: general protection fault: 0000 [1] SMP



Comment 25 Peter Vrabec 2007-01-26 10:43:55 UTC
this is suspicious, isn't it:

in /etc/mtab is
/rootcrypt /rootfoo ecryptfs 
rw,ecryptfs_sig=945f0e28ae2dfba0,ecryptfs_key_bytes=16,ecryptfs_cipher=aes, 0 
0

in /proc/mounts is
crypt/ /root/foo ecryptfs rw,dir=/root/crypt 0 0


Comment 26 Michael Halcrow 2007-01-26 23:09:34 UTC
Peter Vrabec wrote:
> #umount foo
> Neoprávněný přístup do paměti (SIGSEGV)

With this sequence of operations, everything on my end is sane. It looks like I
am going to have to set up a test machine with your exact kernel to reproduce this.

If you have the time, you might try this with the most recent -mm kernel to see
if the issue is already resolved there.

> this is suspicious, isn't it:
> in /etc/mtab is
> /rootcrypt /rootfoo ecryptfs 
> rw,ecryptfs_sig=945f0e28ae2dfba0,ecryptfs_key_bytes=16,ecryptfs_cipher=aes, 0 
> 0
>
> in /proc/mounts is
> crypt/ /root/foo ecryptfs rw,dir=/root/crypt 0 0

I'm not sure what the consequences are of these not being in sync, but I suppose
it would be a good thing to put on the work queue.

Comment 27 Kevin Fenzi 2007-01-28 04:15:05 UTC
In reply to comment #24:

Are you using a x86_64 machine? I see the same thing on my x86_64 machine here. 
I'll attach the dmesg from the machine. It does sound like a locking issue. 

Michael: Could you post an updated package addressing the issues in comments #17
and #18?





Comment 28 Kevin Fenzi 2007-01-28 04:17:22 UTC
Created attachment 146760 [details]
backtrace from dmesg on x86_64 machine

Comment 29 Michael Halcrow 2007-02-07 00:02:24 UTC
Kevin Fenzi wrote:
> 1. Your version scheme seems a bit odd. Is there any reason why you
> do major integer releases every time? Now that 9 is out it's hard to
> go to a more traditional 'work toward a stable 1.0', but it might be
> worth considering just minor bumps for minor changes?

I have considered using the dot notation in the version, but there is
no notion of ``major'' and ``minor'' releases in our development and
release cycle for the mount helper code. There will never be an
``experimental'' or ``beta'' branch of the code. The feature set is
complete as-is for what is available in the kernel, and the current
version is suitable for general release and use in production
environments; adding extra characters into the version string would
really provide no useful information. The mount helper is small and
simple enough that I just can't justify maintaining multiple
branches. The only other versioning scheme I might consider would be
YYYYMMDD (which we did use for a while in the very early snapshot
releases), but I prefer to minimize the number of characters used to
express the version. In all cases, I recommend using the most recent
release in any deployment of the mount helper code, regardless of its
version number.

> 3. The 2.6.19 kernel in fc6 updates has the ecryptfs module, so here
> should be support for this in fc6 with the updated kernel at
> least. Not sure how we are going to require that however. Wonder if
> we can do a 'Requires: ecryptfs.ko'.

Then there is the case where the user builds his kernel without module
support. I think the approach should be the same as with packages that
only support specific kernel features, such as CIFS. I looked over the
SAMBA spec file, and nothing jumped out at me as a kernel module build
dependency.

I ran several tests eCryptfs on x86_64 with kernel-2.6.19-1.2895.fc6
and ext3 as the lower filesystem, and I did not get any errors on
unmount.

In response to comments #17 and #18, I have updated the SPEC file and
have generated an updated source RPM. Except for an extra comment in
the README, the source tarball remains unchanged.

Source RPM:

http://downloads.sourceforge.net/ecryptfs/ecryptfs-utils-9-1.src.rpm

SPEC file:

http://downloads.sourceforge.net/ecryptfs/ecryptfs-utils.spec

Thanks,
Mike

Comment 30 Kevin Fenzi 2007-02-08 02:47:51 UTC
ok. Understood on the versioning... there are other projects that also
do just iteger releases, for example: xterm (version 223 now).

>Then there is the case where the user builds his kernel without module
>support. I think the approach should be the same as with packages that
>only support specific kernel features, such as CIFS. I looked over the
>SAMBA spec file, and nothing jumped out at me as a kernel module build
>dependency.

Yeah, I don't see anything there either.
I guess just having the 2.6.19 dependency...

>I ran several tests eCryptfs on x86_64 with kernel-2.6.19-1.2895.fc6
>and ext3 as the lower filesystem, and I did not get any errors on
>unmount.

ok. I will retest here...

>In response to comments #17 and #18, I have updated the SPEC file and
>have generated an updated source RPM. Except for an extra comment in
>the README, the source tarball remains unchanged.

>Source RPM:
>
>http://downloads.sourceforge.net/ecryptfs/ecryptfs-utils-9-1.src.rpm
>
>SPEC file:
>
>http://downloads.sourceforge.net/ecryptfs/ecryptfs-utils.spec

Humm. I don't see this addressed:

> 1. Might include the following as %doc files:
> AUTHORS NEWS THANKS

On point 2, you might consider litterally having:
Requires: %{name} = %{version}-%{release}
in the spec. Then you wouldn't need to remember to change the
Requires: ecryptfs = 9-1
on every upgrade.

On point3, I see you have --disable-rpath, so hopefully that fixes the
rpath issues. I will do a build to confirm.

On point4, dist tag looks good.

I am having some mirror issues here, but as soon as thats solved,
I will do another build and do some more testing.
The package is looking pretty good from what I can see...
If you could spin another release addressing the items
above, that would be great.


Comment 31 Kevin Fenzi 2007-02-28 03:26:05 UTC
Hey Michael. Sorry for the delay here. ;( 

I did some testing here on my x86_64 box, running the latest rawhide. 

ecryptfs-utils-9-1
kernel-2.6.20-1.2949.fc7.x86_64

Trying: 
cd /tmp
mkdir secret
mount -t ecryptfs /secret /secret

Unable to get the version number of the kernel
module. Please make sure that you have the eCryptfs
kernel module loaded, you have sysfs mounted, and
the sysfs mount point is in /etc/mtab. This is
necessary so that the mount helper knows which 
kernel options are supported.

Enabling passphrase-mode only for now.

Passphrase: 
Verify Passphrase: 
Cipher
1) Twofish
2) CAST5
3) CAST6
4) Blowfish
5) AES-128
6) AES-192
7) AES-256
8) Triple-DES
Selection [AES-128]: 7
Attempting to mount with the following options:
  ecryptfs_cipher=aes
  ecryptfs_key_bytes=32
  ecryptfs_sig=dba5ed7952a1184d

So, the module wasn't loaded and it didn't autoload it. 
I did a manual 'modprobe ecryptfs' and then tried again. 
It seems to work, but I don't see anything with df, and looking in 
dmesg I get: 

mount.ecryptfs[4322] general protection rip:3226c5e4b9 rsp:7fffa5c67a80 error:0
mount.ecryptfs[4568] general protection rip:3226c5e4b9 rsp:7fff15962c10 error:0

I'll try some more testing with ppc and i386 soon. 
If you could spin a new -2 release addressing the issues in the last comment, we
can try and get this moved forward. 

Thanks. 


Comment 32 Peter Vrabec 2007-03-01 11:50:37 UTC
There are 2 lines before kernel error in messages:
Mar  1 11:33:47 rawhide64 mount.ecryptfs: Your kernel does not support key 
module [openssl]
Mar  1 11:33:59 rawhide64 mount.ecryptfs: Mount opts: 
[ecryptfs_sig=945f0e28ae2dfba0,ecryptfs_key_bytes=16,ecryptfs_cipher=aes,]

# uname -a
Linux rawhide64 2.6.20-1.2949.fc7 #1 SMP Mon Feb 26 18:33:03 EST 2007 x86_64 
x86_64 x86_64 GNU/Linux


Comment 33 Michael Halcrow 2007-03-01 23:14:49 UTC
Kevin Fenzi (kevin) wrote:
> 1. Might include the following as %doc files:
> AUTHORS NEWS THANKS

Done.

> On point 2, you might consider litterally having:
> Requires: %{name} = %{version}-%{release}
> in the spec.

Done.

> So, the module wasn't loaded and it didn't autoload it. 
> I did a manual 'modprobe ecryptfs' and then tried again. 
> It seems to work, but I don't see anything with df, and looking in 
> dmesg I get: 
>
> mount.ecryptfs[4322] general protection rip:3226c5e4b9
> rsp:7fffa5c67a80 error:0
> mount.ecryptfs[4568] general protection rip:3226c5e4b9
> rsp:7fff15962c10 error:0

There was a bug in the error path (free bogus pointer). Fixed in the
ecryptfs-utils-10 release.

Peter Vrabec (pvrabec) wrote:
> There are 2 lines before kernel error in messages:
> Mar 1 11:33:47 rawhide64 mount.ecryptfs: Your kernel does not
> support key module [openssl]
> Mar  1 11:33:59 rawhide64 mount.ecryptfs: Mount opts: 
> [ecryptfs_sig=945f0e28ae2dfba0,ecryptfs_key_bytes=16,ecryptfs_cipher=aes,]

The mount helper is overly verbose; fixed in the ecryptfs-utils-10
release.

I also changed the key module build in ecryptfs-utils-10 so that it is
now possible to disable building the OpenSSL key module; however, that
should not affect the RPM. Future releases will likely include many
more key modules, and if they are all included in the same RPM, then
there could be an extra dependency for each key module (openCryptoki,
TrouSerS, etc.), whether the user wants the key module or not. If it
would be acceptable to break the key modules into individual RPM's
(ecryptfs-keymodule-openssl, ecryptfs-keymodule-tss,
ecryptfs-keymodule-ock, etc.), then that would reduce the dependencies
for ecryptfs-utils.

Updated SPEC is here:
http://downloads.sourceforge.net/ecryptfs/ecryptfs-utils.spec?use_mirror=osdn

Updated SRPM is here:
http://downloads.sourceforge.net/ecryptfs/ecryptfs-utils-10-0.src.rpm

There are also a number of minor kernel bugfixes that Red Hat may want to pick
up as separate patches in its 2.6.20 kernel.

Mike

Comment 34 Peter Vrabec 2007-03-02 14:38:51 UTC
Mar  2 14:25:26 rawhide64 kernel: 
=============================================
[ INFO: possible recursive locking detected ]
2.6.20-1.2949.fc7 #1
---------------------------------------------
bash/3868 is trying to acquire lock:
(&inode->i_mutex){--..}, at: [<ffffffff802624d6>] mutex_lock+0x2a/0x2e

but task is already holding lock:
(&inode->i_mutex){--..}, at: [<ffffffff802624d6>] mutex_lock+0x2a/0x2e

other info that might help us debug this:
1 lock held by bash/3868:
 #0:  (&inode->i_mutex){--..}, at: [<ffffffff802624d6>] mutex_lock+0x2a/0x2e

stack backtrace:

Call Trace:
[<ffffffff802a30ad>] __lock_acquire+0x151/0xbc4
[<ffffffff8843e114>] :ecryptfs:ecryptfs_filldir+0x0/0x7d
[<ffffffff802a3f16>] lock_acquire+0x4c/0x65
[<ffffffff802624d6>] mutex_lock+0x2a/0x2e
[<ffffffff80262312>] __mutex_lock_slowpath+0xff/0x299
[<ffffffff802624d6>] mutex_lock+0x2a/0x2e
[<ffffffff80235ab6>] vfs_readdir+0x61/0xb1
[<ffffffff80226604>] filldir+0x0/0xc5
[<ffffffff8843e374>] :ecryptfs:ecryptfs_readdir+0x6c/0xb2
[<ffffffff80226604>] filldir+0x0/0xc5
[<ffffffff80235ad1>] vfs_readdir+0x7c/0xb1
[<ffffffff8023927f>] sys_getdents+0x7a/0xc4
[<ffffffff8025c24a>] tracesys+0x71/0xe1


Comment 35 Michael Halcrow 2007-03-03 00:58:36 UTC
Peter Vrabec wrote:
> INFO: possible recursive locking detected

I'm not sure that there's really a problem here. This warning goes away when I
insert a few printk() statements, which indicates that the mutex is properly
released when some time is given to the code that does this release.

Comment 36 Kevin Fenzi 2007-03-04 21:23:07 UTC
ok, all the package issues I saw seem to have been solved... 
I would still like to get it working on x86_64 before we import the package
however. 

With the -10 release here I get: 

Attempting to mount with the following options:
  ecryptfs_cipher=aes
  ecryptfs_key_bytes=16
  ecryptfs_sig=dba5ed7952a1184d
Error mounting eCryptfs; rc = [-2]; strerr = [No such file or directory]

Any idea on that one?

Nothing at all in dmesg, so those things seem solved to me, but it doesn't
appear to work still. ;( 

Could someone try a rawhide i386 machine? I don't have one handy right at the
moment... 

Comment 37 Michael Halcrow 2007-03-06 22:59:24 UTC
Out of curiosity, is the eCryptfs utilities package on-track for inclusion in FC7?

Comment 38 Kevin Fenzi 2007-03-06 23:15:14 UTC
Well, currently I see no issues with how it's packaged, so normally I could
approve it now and get you to move forward in the procedure, but I was hoping we
could make sure the package works before importing and building it for users. ;) 

Any idea what the error in comment #36 means? It doesn't work for me here on
x86_64. Just spits out that error. ;( 
Also, can someone confirm it works on i386? I can try and test that later
tonight... 

Once we can confirm it's working, it should be ready to import and build for fc7
(and fc6 if you like). 

Thanks for all your patience on this, and hopefully we can get it all set soon. 

Comment 39 Michael Halcrow 2007-03-06 23:29:25 UTC
Kevin Fenzi (kevin) wrote:
> Any idea what the error in comment #36 means? It doesn't work for me here on
> x86_64. Just spits out that error. ;( 
> Also, can someone confirm it works on i386? I can try and test that later
> tonight...

What is the actual mount command you are giving? Does this work when run as root?

modprobe ecryptfs; modprobe aes; modprobe md5; mkdir /secret; mount -t ecryptfs
-o key=passphrase:passwd=test,passthrough=n,cipher=aes /secret /secret


Comment 40 Kevin Fenzi 2007-03-06 23:34:45 UTC
# modprobe ecryptfs; modprobe aes; modprobe md5; mkdir /secret; mount -t
ecryptfs -o key=passphrase:passwd=test,passthrough=n,cipher=aes /secret /secret
FATAL: Module md5 not found.
Attempting to mount with the following options:
  ecryptfs_cipher=aes
  ecryptfs_sig=d395309aaad4de06
Error mounting eCryptfs; rc = [-22]; strerr = [Invalid argument]

# dmesg | tail

ecryptfs_parse_options: Could not find key with description: [d395309aaad4de06]
process_request_key_err: Unknown error code: [0x00000000ffffff82]
Error parsing options; rc = [-22]

I suppose we could take this to email? Or if you are on irc at all, we could try
and debug it more interactively via irc later?
Hopefully the above gives some clues... 

Comment 41 Eric Sandeen 2007-03-29 17:43:32 UTC
re: the lockdep warnings, I think ecryptfs is just confusing lockdep.

ecryptfs hits vfs_filldir 2x for example, for both upper & lower filesystems,
and takes i_mutex both times.  Granted, these are actually different inodes,
once for upper & once for lower, but lockdep doesn't know that.  And I'm not
sure how to teach it .  :)

Comment 42 Karsten Hopp 2007-04-18 10:30:35 UTC
What's the status of this package ? I'd really like to see it in Fedora as soon
as possible. It seems to be quite stable on 32bit but needs some work on 64bit.
More public exposure/testing would probably help a lot.

Comment 43 Kevin Fenzi 2007-04-18 16:16:03 UTC
I can do another round of testing tonight with ppc32 and x86_64 rawhide. 
I don't have a handy i386/rawhide machine around right now. 
If someone out there could do some testing with i386/rawhide that would be great. 

The packaging itself is good to approve now, but I hate to have something in 
that doesn't work. ;(


Comment 44 Kevin Fenzi 2007-04-19 03:32:19 UTC
ok. I have done some testing here with the latest rawhide kernel and ecrypts-utils. 

- ppc32 works just fine. No oops. No issues. I can mount, read/write and unmount
ok. 

- x86_64 works ok. There is a locking message when you first try and use the
mount after mounting, but there is no ill effect. mount/read/write/umount all
work ok. 

Here's the locking messages I see in dmesg: 

=============================================
[ INFO: possible recursive locking detected ]
2.6.20-1.3079.fc7 #1
---------------------------------------------
ls/23073 is trying to acquire lock:
 (&inode->i_mutex){--..}, at: [<ffffffff802622fe>] mutex_lock+0x2a/0x2e

but task is already holding lock:
 (&inode->i_mutex){--..}, at: [<ffffffff802622fe>] mutex_lock+0x2a/0x2e

other info that might help us debug this:
1 lock held by ls/23073:
 #0:  (&inode->i_mutex){--..}, at: [<ffffffff802622fe>] mutex_lock+0x2a/0x2e

stack backtrace:

Call Trace:
 [<ffffffff802a2e62>] __lock_acquire+0x151/0xbd1
 [<ffffffff883f2120>] :ecryptfs:ecryptfs_filldir+0x0/0x7d
 [<ffffffff802a3cd8>] lock_acquire+0x4c/0x65
 [<ffffffff802622fe>] mutex_lock+0x2a/0x2e
 [<ffffffff8026213a>] __mutex_lock_slowpath+0xff/0x299
 [<ffffffff802622bb>] __mutex_lock_slowpath+0x280/0x299
 [<ffffffff802622fe>] mutex_lock+0x2a/0x2e
 [<ffffffff80235b75>] vfs_readdir+0x61/0xb1
 [<ffffffff8022668a>] filldir+0x0/0xc5
 [<ffffffff883f2384>] :ecryptfs:ecryptfs_readdir+0x6c/0xb2
 [<ffffffff8022668a>] filldir+0x0/0xc5
 [<ffffffff80235b90>] vfs_readdir+0x7c/0xb1
 [<ffffffff80239340>] sys_getdents+0x7a/0xc4
 [<ffffffff8025c11e>] system_call+0x7e/0x83

That looks like it's a cosmetic bug more than one that causes any real problems. 

So, since there are no more packaging issues, and things seem to work now, I
will go ahead and APPROVE this package. 

Michael: You can continue the process at: 
http://fedoraproject.org/wiki/PackageMaintainers/Join#head-a601c13b0950a89568deafa65f505b4b58ee869b

If you have any questions at all about the process, feel free to email me, or
catch me on irc.freenode.net in #fedora-devel (nick: nirik). 

Oh, I see that -14 is out now. If you would like me to look over the updated
package before you import it, I would be happy to do so. 


Comment 45 Michael Halcrow 2007-04-19 19:16:39 UTC
Kevin Fenzi (kevin) wrote:
> Oh, I see that -14 is out now. If you would like me to look over the
> updated package before you import it, I would be happy to do so.

I added a PAM module that can be used to automatically insert the
user's key (based on his login passphrase) into his keyring at
login. This can be used to perform automatic eCryptfs mounts.

Updated spec:

http://downloads.sourceforge.net/ecryptfs/ecryptfs-utils.spec

New source RPM package (version 15):

http://downloads.sourceforge.net/ecryptfs/ecryptfs-utils-15-0.src.rpm

Thanks,
Mike

Comment 46 Kevin Fenzi 2007-04-20 04:48:59 UTC
I see in the build log: 
...
PAM directory: [/lib/security]
./configure: line 20473: gtk-config: command not found
./configure: line 20474: gtk-config: command not found
./configure: line 20475: gtk-config: command not found
GTK not found

Is there a missing BuildRequires: on gtk?
(Not sure what it would be used for though). 

Also, the permissions don't look right on the pam module. 
It's mode 644, but should be 755? This results in it not appearing in
the debuginfo since it's not executable. All the other pam modules appear to be
755... 

Other than that it looks good... ;) 





Comment 47 Michael Halcrow 2007-04-20 15:32:37 UTC
Kevin Fenzi (kevin) wrote:
> I see in the build log: 
> ...
> PAM directory: [/lib/security]
> ./configure: line 20473: gtk-config: command not found
> ./configure: line 20474: gtk-config: command not found
> ./configure: line 20475: gtk-config: command not found
> GTK not found
>
> Is there a missing BuildRequires: on gtk?
> (Not sure what it would be used for though). 

Not for this release. Someone put some work into a graphical eCryptfs
mount management tool last summer, but it never got into a state that
is worthy of inclusion into the utils package. So the check for gtk
will be needed once the code is available, but for now, the
./configure warning can be ignored.

> Also, the permissions don't look right on the pam module.  It's mode
> 644, but should be 755? This results in it not appearing in the
> debuginfo since it's not executable. All the other pam modules
> appear to be 755...

It turns out that some distro's set the PAM permissions to 644 and
others to 755. It looks like the only effect in Fedora is the
inclusion in debuginfo.

Version 15-1 changes the PAM module permission from 644 to 755.

Spec file:

http://downloads.sourceforge.net/ecryptfs/ecryptfs-utils.spec

Source RPM:

http://downloads.sourceforge.net/ecryptfs/ecryptfs-utils-15-1.src.rpm


Comment 48 Kevin Fenzi 2007-04-20 19:06:08 UTC
>Not for this release. Someone put some work into a graphical eCryptfs
>mount management tool last summer, but it never got into a state that
>is worthy of inclusion into the utils package. So the check for gtk
>will be needed once the code is available, but for now, the
>./configure warning can be ignored.

ok. Makes sense. 

>It turns out that some distro's set the PAM permissions to 644 and
>others to 755. It looks like the only effect in Fedora is the
>inclusion in debuginfo.
>
>Version 15-1 changes the PAM module permission from 644 to 755.

Humm. I don't see that change in the -1 version. Only the changelog and 
the version are changed. Can you doublecheck the version you updated?
In any case you can fix that before you check it in... 


Comment 49 Michael Halcrow 2007-04-20 19:25:54 UTC
Kevin Fenzi (kevin) wrote:
> I don't see that change in the -1 version.

The permission change was actually done in src/pam_ecryptfs/Makefile.am in the
source tarball.

I'm working on getting through some red tape to create my Fedora account.
Hopefully I'll be able to get the package in by next week.


Comment 50 Kevin Fenzi 2007-04-20 22:25:23 UTC
In reply to comment #49: 

Ah, I looked and didn't see an increment in the release or change in the md5sum,
but perhaps it was cached. It's best to increase release for any changes (no
matter how minor) so people know it was changed. 

Please let me know if I can assist any with the fedora account/red tape...

Comment 51 Kevin Fenzi 2007-06-06 19:01:37 UTC
Hey Michael. While the legal gears are grinding along, would you mind if I went
ahead and imported this package and maintained it for now? 

Then, once the red tape is beaten back I can add you as a co-maintainer? 

Let me know if you find that acceptable for now... 

Comment 52 Michael Halcrow 2007-06-06 21:10:36 UTC
Kevin -

Yes, if you have the cycles to take care of that, it would be fine by me.

Mike

Comment 53 Kevin Fenzi 2007-06-06 22:36:13 UTC
Excellent. Please do let me know as soon as you are ready to be a co-maintainer. 
I will set you for inital CC's on bugs... 

I am only requesting F-7 and devel for now. I think we could support FC-6, but
we will need to be careful to pull in the right update kernel that has ecryptfs
support. 

New Package CVS Request
=======================
Package Name: ecryptfs-utils
Short Description: The eCryptfs mount helper and support libraries
Owners: kevin
Branches: F-7
InitialCC: mhalcrow.com

Comment 54 Kevin Fenzi 2007-06-07 02:08:59 UTC
Sorry about that, just realized that I shouldn't both approve and maintain... 

Can someone else do a review for me? It should be pretty easy hopefully. 
All the issues I saw are solved. 

Comment 55 Bill Nottingham 2007-06-07 06:49:22 UTC
MUST items:

 - Package meets naming and packaging guidelines - OK
 - Spec file matches base package name. - OK
 - Spec has consistant macro usage. - OK
 - Meets Packaging Guidelines. - OK
 - License  - OK
 - License field in spec matches - OK
 - License file included in package - OK
 - Spec in American English - OK
 - Spec is legible. - OK
 - Sources match upstream md5sum: - OK

 - Package needs ExcludeArch - OK
 - BuildRequires correct - OK
 - Spec handles locales/find_lang - N/A
 - Package is relocatable and has a reason to be. - N/A
 - Package has %defattr and permissions on files is good. - OK
 - Package has a correct %clean section. - OK
 - Package has correct buildroot - OK
      %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
 - Package is code or permissible content. - OK
 - Doc subpackage needed/used. - N/A
 - Packages %doc files don't affect runtime. - OK

 - Headers/static libs in -devel subpackage. - OK
 - Spec has needed ldconfig in post and postun - OK
 - .pc files in -devel subpackage/requires pkgconfig - N/A
 - .so files in -devel subpackage. - OK
 - -devel package Requires: %{name} = %{version}-%{release} - OK
 - .la files are removed.  - OK

 - Package is a GUI app and has a .desktop file - N/A

 - Package compiles and builds on at least one arch. - OK
 - Package has no duplicate files in %files. - OK
 - Package doesn't own any directories other packages own. - OK
 - Package owns all the directories it creates. - OK
 - No rpmlint output. ***

E: ecryptfs-utils binary-or-shlib-defines-rpath /sbin/mount.ecryptfs ['/usr/lib64']
E: ecryptfs-utils binary-or-shlib-defines-rpath /lib64/security/pam_ecryptfs.so
['/usr/lib64']
E: ecryptfs-utils binary-or-shlib-defines-rpath /usr/bin/ecryptfsd ['/usr/lib64']
E: ecryptfs-utils binary-or-shlib-defines-rpath /usr/bin/ecryptfs-manager
['/usr/lib64']

 - final provides and requires are sane: ***

Main package has:

Requires: keyutils openssl pam kernel >= 2.6.19

1) keyutils, openssl, pam requirements should be superfluous - library
dependencies take care of this
2) kernel requires are tricky. Generally, we do

Conflicts: kernel < 2.6.19

as there's no reason, for example, to pull a kernel into a buildroot.

SHOULD Items:

 - Should build in mock. - OK (tried x86_64)
 - Should build on all supported archs - build i386 non-mock
 - Should function as described. - mounted FS successfully
 - Should have sane scriptlets. - OK
 - Should have subpackages require base package with fully versioned depend. - OK
 - Should have dist tag - OK
 - Should package latest version - build checks were done with 16, even though
15 was here :)

MISC item:

So, general sanity checking:

/lib/security/pam_ecryptfs.so:
        linux-gate.so.1 =>  (0x00e9e000)
...        libdl.so.2 => /lib/libdl.so.2 (0x006a6000)
        libecryptfs.so.0 => /usr/lib/libecryptfs.so.0 (0x00b11000)
        libgcrypt.so.11 => /usr/lib/libgcrypt.so.11 (0x007c4000)
...
        libgpg-error.so.0 => /usr/lib/libgpg-error.so.0 (0x0056f000)


/sbin/mount.ecryptfs:
        linux-gate.so.1 =>  (0x00397000)
        libgpg-error.so.0 => /usr/lib/libgpg-error.so.0 (0x038a8000)
        libecryptfs.so.0 => /usr/lib/libecryptfs.so.0 (0x008eb000)
...
        libgcrypt.so.11 => /usr/lib/libgcrypt.so.11 (0x03e3a000)

That's bad; these shouldn't be linked against things in /usr/lib. (Yes, some
people still run /usr separate.) Moreover, I suspect that both of these will
also dlopen the plugins in $(libdir)/ecryptfs?

-devel: what, if anything, will ever build against this? If there's nothing, it
may not be worth shipping. (Also,does this package maintain a stable ABI?)



Comment 56 Kevin Fenzi 2007-06-07 23:24:36 UTC
Thanks for looking at this!

We had gotten rid of the rpath in the past... looks like it's crept back. 
Will get that fixed. 

>1) keyutils, openssl, pam requirements should be superfluous - library
>dependencies take care of this

ok. Removed. 

>2) kernel requires are tricky. Generally, we do
>
>Conflicts: kernel < 2.6.19
>
>as there's no reason, for example, to pull a kernel into a buildroot.

Well, the tricky part here is we need to require a kernel with ecryptfs.ko in
it. For F-7 and devel no problems, as all of them have it. For FC-6 however, the
early kernels didn't, and the updated ones do. I thought that the error end
users get from yum on Requires is more usefull than Conflicts?

>That's bad; these shouldn't be linked against things in /usr/lib. (Yes, some
>people still run /usr separate.) Moreover, I suspect that both of these will
>also dlopen the plugins in $(libdir)/ecryptfs?

Thats a good Question. Michael? Any thoughts?
Yes, those .so's under libdir/ecryptfs/ do dlopen the so's. 
This sounds like something for upstream to change?

> -devel: what, if anything, will ever build against this? If there's nothing,
>it may not be worth shipping. (Also,does this package maintain a stable ABI?)

I don't know of anything... Michael?

Comment 57 Michael Halcrow 2007-06-08 16:50:50 UTC
Kevin wrote:
> Thanks for looking at this!
> 
> We had gotten rid of the rpath in the past... looks like it's crept back. 
> Will get that fixed. 

I thought I had addressed this issue with the --disable-rpath
configure flag. I don't think there's anything funny in my makefiles
that should re-introduce it.

> >1) keyutils, openssl, pam requirements should be superfluous - library
> >dependencies take care of this
> 
> ok. Removed. 
> 
> >2) kernel requires are tricky. Generally, we do
> >
> >Conflicts: kernel < 2.6.19
> >
> >as there's no reason, for example, to pull a kernel into a buildroot.
> 
> Well, the tricky part here is we need to require a kernel with ecryptfs.ko in
> it. For F-7 and devel no problems, as all of them have it. For FC-6 however, the
> early kernels didn't, and the updated ones do. I thought that the error end
> users get from yum on Requires is more usefull than Conflicts?
> 
> >That's bad; these shouldn't be linked against things in /usr/lib. (Yes, some
> >people still run /usr separate.) Moreover, I suspect that both of these will
> >also dlopen the plugins in $(libdir)/ecryptfs?
> 
> Thats a good Question. Michael? Any thoughts?

It makes sense to link these statically. I will make it so in the next
ecryptfs-utils release.

> Yes, those .so's under libdir/ecryptfs/ do dlopen the so's. 
> This sounds like something for upstream to change?

The files in libdir/ecryptfs/ are pluggable key modules, sort of like
OpenSSL engines in /usr/lib/engines/.

However, they are necessary in order to insert the key for the given
key type into the user session keyring, and hence are necessary in
order to mount.

It sounds like the key modules shipping with the ecryptfs-utils
package would best be statically linked too.

> > -devel: what, if anything, will ever build against this? If there's nothing,
> >it may not be worth shipping. (Also,does this package maintain a stable ABI?)
> 
> I don't know of anything... Michael?

libecryptfs exists specifically to allow others to write their own
utilities (a GUI, for instance) to work with eCryptfs key management
functions. For instance, anyone who wants to write their own pluggable
key modules to interface with their own key management system will
need the -devel package.

The exported symbols in libecryptfs can be considered stable.

Mike

Comment 58 Bill Nottingham 2007-06-08 17:02:41 UTC
(In reply to comment #57)
> I thought I had addressed this issue with the --disable-rpath
> configure flag. I don't think there's anything funny in my makefiles
> that should re-introduce it.

It's probably libtool. See the 'Removing RPATH' section of 
http://fedoraproject.org/wiki/Packaging/Guidelines - you may need
some of the sed goo there.

> It makes sense to link these statically. I will make it so in the next
> ecryptfs-utils release.
> 
> > Yes, those .so's under libdir/ecryptfs/ do dlopen the so's. 
> > This sounds like something for upstream to change?
> 
> The files in libdir/ecryptfs/ are pluggable key modules, sort of like
> OpenSSL engines in /usr/lib/engines/.
> 
> However, they are necessary in order to insert the key for the given
> key type into the user session keyring, and hence are necessary in
> order to mount.

Into the *user session* keyring? If it's not being done until the user logs
in,you can assume that /usr is available, so it's not an issue.

> libecryptfs exists specifically to allow others to write their own
> utilities (a GUI, for instance) to work with eCryptfs key management
> functions. For instance, anyone who wants to write their own pluggable
> key modules to interface with their own key management system will
> need the -devel package.


OK, thanks.

Re: kernel requires vs. conflicts; conflicts is what we've historically done in
packages such as initscripts, hal, and so on that don't work with older kernels.

Comment 59 Kevin Fenzi 2007-06-08 17:07:58 UTC
>I thought I had addressed this issue with the --disable-rpath
>configure flag. I don't think there's anything funny in my makefiles
>that should re-introduce it.

Yeah, me too. Will investigate more... 

>It makes sense to link these statically. I will make it so in the next
>ecryptfs-utils release.

Well, not sure they need to be static... I think the idea was to move the
libraries from /usr/ to /lib|/lib64. That would allow them to work fine for
people who have /usr on a remote fs or the like. 

>It sounds like the key modules shipping with the ecryptfs-utils
>package would best be statically linked too.

Again, not sure static is needed, simply move them to a /lib/ecryptfs/ instead
of /usr/lib/

Notting: Any input on static vs simply moving to /lib ?

Thanks for looking at this Mike!

>Re: kernel requires vs. conflicts; conflicts is what we've historically done in
>packages such as initscripts, hal, and so on that don't work with older >kernels.

ok, can leave the Conflicts for now. 

Comment 60 Bill Nottingham 2007-06-08 17:46:46 UTC
Moving to /lib is fine, but there's still things like libgcrypt, openssl, etc.
that those plugins may link to.

Comment 61 Kevin Fenzi 2007-06-15 05:51:14 UTC
Hey Michael. Any word on a new upstream release moving to /lib or making things
static? 

Comment 62 Michael Halcrow 2007-06-15 12:41:58 UTC
eCryptfs is broken in 2.6.22-rc4 (kernel hang), so I've been busy working on
some patches to fix it. I think I have the issues resolved, so I'll try to find
some time this weekend to resume work on the userspace package.

Comment 63 Michael Halcrow 2007-06-19 23:30:58 UTC
Given the fact that it links to libgcrypt, it's probably best to link
mount.ecrypt statically. But then there is the same problem with
pam_ecryptfs.so, which also needs libgcrypt and libgpg-error. My naive attempts
to just link them statically into pam_ecryptfs.so have been met with protests by
the tools about portability (and, in any case, I haven't yet found a way to
coerce libtool to do that via a "proper" Makefile.am entry).

Note that plenty of other /lib/security files dynamically link stuff under
/usr/lib, including libcrack, libkrb4, libsmime3, etc. Since so many other
/lib/security/*so files link against libraries in /usr/lib, is it okay for
pam_ecryptfs.so to do the same?

Comment 64 Bill Nottingham 2007-06-20 03:40:22 UTC
pam_* doesn't need the static linking.

Comment 65 Michael Halcrow 2007-06-20 18:50:27 UTC
Statically linking mount.ecryptfs is still problematic.

In order to perform its key management functions (i.e., parsing the user's rc
file and pulling in dynamic key modules), libecryptfs needs to call getpwuid and
dlopen. Any attempts I have made thus far to statically link mount.ecryptfs have
resulted in warnings from the tools about specific versions of glibc runtime and
shared libraries needing to be available, and I am getting nothing but
segfaults, with valgrind complaining about conditional jumps to libc functions
depending on uninitialized values.

However, the eCryptfs kernel module depends on keys being available in the user
session keyring on mount. Bill mentioned this in a previous comment:

> Into the *user session* keyring? If it's not being done until the user logs
> in,you can assume that /usr is available, so it's not an issue.

So if dynamically linking against libraries in /usr/lib is a non-issue in the
case of eCryptfs, then can we consider any remaining linking issues brought up
in comment #55 to be non-applicable?

Thanks,
Mike

Comment 66 Bill Nottingham 2007-06-20 19:37:29 UTC
Possibly not. Are there no provisions for system-level encryption?

Comment 67 Michael Halcrow 2007-06-20 22:39:19 UTC
eCryptfs is pretty well hooked in with the kernel keyring. There could be some
modifications done in the kernel module to allow key registration via a mount
option or a sysfs handle, if that is something that users will really want, but
to date, nobody has requested such a thing. eCryptfs links against libgcrypt
mainly because it is less complicated in terms of licensing (it's all GPL). The
next-best option I would choose would be to just copy in what libecryptfs needs
from libtomcrypt.

Release 17 keeps all the linking as-is, but it also has a built-in fallback for
the passphrase key module in the event that the usr/lib/ecryptfs/ directory is
not set up as expected.

I added several new userspace utilities for managing wrapped passphrases. This
involves keeping a permanent mount passphrase wrapped by your login. This is
helpful, for instance, if you are using pam_ecryptfs.so and want to be able to
change your login passphrase without having to re-encrypt all of your data.

I added the commands to run check-rpath in my ~/.rpmmacros file, as suggested in
the packaging guidelines, and I did not see any errors generated by check-rpath.

Release 17 is here:

http://downloads.sourceforge.net/ecryptfs/ecryptfs-utils-17.tar.bz2

Along with an updated SPEC file (although I'm not sure what Kevin has done with
the SPEC file since I last changed it):

http://downloads.sourceforge.net/ecryptfs/ecryptfs-utils.spec

Comment 68 Kevin Fenzi 2007-06-21 00:10:26 UTC
I had a local updated spec, but hadn't done anything much with it pending the
static discussion. ;) 

I just used your updated spec here and it looks to me like it solves all the
issues notting saw, with the possible exception of the static linking idea. 

notting: can you look and see if there are any remaining blockers here? 



Comment 69 Bill Nottingham 2007-06-21 15:56:06 UTC
1. Still requires kernel > 2.6.19; this should be a conflicts
2. -devel requires openssl-devel and pam-devel even though it's not actually
linked against either of those (and I don't see definitions that would need them
in the header, but I might be reading wrong.)


Comment 70 Kevin Fenzi 2007-06-21 16:40:08 UTC
Ah, good catch. 

1. I must have been looking at my local spec. Fixed now. 
2. fixed by removing those requires. I don't think it currently needs them either.

spec: http://www.scrye.com/~kevin/fedora/ecryptfs-utils.spec
srpm: http://www.scrye.com/~kevin/fedora/ecryptfs-utils-17-1.fc8.src.rpm



Comment 71 Bill Nottingham 2007-06-21 16:45:37 UTC
Looks OK to me. Approved.

Comment 72 Kevin Fenzi 2007-06-21 18:40:48 UTC
Thanks for the review and spotting those issues. 

(Reassigning to you as reviewer)

Michael: Do let me know when you are through the red tape and ready to
co-maintain. In the mean time please let me know if there is anything else I
should update, etc. I am adding you in the initialcc for bugs. 

Package Name: ecryptfs-utils
Short Description: The eCryptfs mount helper and support libraries
Owners: kevin
Branches: F-7 FC-6
InitialCC: mhalcrow.com


Comment 73 Michael Halcrow 2007-06-21 18:59:32 UTC
Through heroic efforts on the part of several brave souls, in the face of
seemingly insurmountable bureaucratic opposition, I am happy to report that I
have finally been added to the cla_done group. I am ready to co-maintain.

Comment 74 Kevin Fenzi 2007-06-21 19:26:12 UTC
Hurray. Excellent news. 

Can you go to this step: 
http://fedoraproject.org/wiki/PackageMaintainers/Join#head-0dbf12f9c493a3f20fae545bb9c1396cb0a88053

and request sponsorship?

(updated cvs request, once Michael is sponsored). 

Package Name: ecryptfs-utils
Short Description: The eCryptfs mount helper and support libraries
Owners: mhalcrow.com,kevin
Branches: F-7 FC-6
InitialCC: 


Comment 75 Kevin Fenzi 2007-06-26 02:37:07 UTC
cvs done. 

Comment 76 Michael Halcrow 2007-06-29 14:56:40 UTC
make build with release 18-0 completed successfully. Closing bug as NEXTRELEASE
(per the PackageMaintainers/Join instructions).

Comment 77 Michael Halcrow 2007-10-03 22:20:58 UTC
Package Change Request
======================
Package Name: ecryptfs-utils
New Branches: F-8

I request that ecryptfs-utils be included in Fedora 8. This supports the
eCryptfs filesystem in kernel versions 2.6.19 and higher.

Comment 78 Eric Sandeen 2007-10-03 23:52:07 UTC
Mike, as I understand it, ecryptfs-utils is already in F8; the request above
will get you a branch specifically for F8 rather than leaving it in devel (which
eventually will become F9 after everything gets cloned off to F8)

[root@newbox v4l-dvb]# cat /etc/redhat-release 
Fedora release 7.91 (Rawhide)
[root@newbox v4l-dvb]# rpm -q ecryptfs-utils
ecryptfs-utils-18-1.fc8

From jkeating's fedora-devel note today, "If a package isn't
branched yet, builds from devel/ continue to go to dist-f8."

-Eric

Comment 79 Kevin Fenzi 2007-10-04 02:51:16 UTC
Yeah, unless you want a F-8 branch now so you can keep a stable version for F-8
and move forward with a test version in devel/rawhide, there is no need to
request an F-8 branch now. 

All current devel packages will be branched for F-8 before release. 

Please reset the fedora-cvs flag if you want such a branch now. 

Comment 80 Michal Hlavinka 2015-01-05 15:59:52 UTC
Package Change Request
======================
Package Name: ecryptfs-utils
New Branches: epel7
Owners: mhlavink

Comment 81 Gwyn Ciesla 2015-01-05 18:08:26 UTC
Git done (by process-git-requests).