Bug#657076: Updating and maintaining barry in Debian / Ubuntu

[Chris Frey <cdfrey@foursquare.net>]

On Fri, May 11, 2012, intrigeri wrote:
> Next review round results in:

Thanks :-)


> 1. debian/copyright may be syntaxically correct, but it looks weird
>    compared to how people usually put things in there.
>    Attached patch fixes this. Please consider applying it.

I applied the patch, with a slight change.

I didn't realize that the License section could be all by itself,
so thanks for that!


> 2. I don't think "src/vformat.{h,c}" is supported syntax in a Files:
>    field in debian/copyright.

Probably not, according to the spec.  I changed it to list both files
separately.


> 3. The homepage set in debian/control is not the same as the one set
>    in debian/copyright. This looks wrong.

The one in copyright, under the Source: field, is where you get the source
code, while the one in control, under Homepage, is where you find the
welcome and documentation.  I've updated copyright to make this clearer.


> 4. barrydesktop Suggests: gksu. How usable is it without gksu?

I picked Suggests so that it would be easy to install without gksu.
It is for the modem mode, and if the user is a member of the dip group,
then it is not needed.  But if not, the desktop warns the user to add
themselves to the same group used by /usr/sbin/pppd, and then offers a
way to continue anyway, by calling gksu.


> 5. It seems to me BARRY_FIFO_NAME is handled in a way that opens
>    avenues to symlink attacks. Am I wrong? Files in world-writable
>    directories must be treated with care.

I believe it is safe.  Here is the comment I added to the code in git to
explain my reasoning:

                // Security Note:
                // --------------
                // This should be safe from symlink attacks, since
                // mkfifo(), in the constructor, will fail if any other
                // file or symlink already exists, and therefore we will
                // never get to this open() call if that fails.  And if
                // mkfifo() succeeds, then we are guaranteed (assuming /tmp
                // permissions are correct) that only root or our own
                // user can replace the fifo with something else, such
                // as a symlink.
                //
                // The server side is not intended to run as root, yet
                // if it is, then we are still safe, due to the above logic.
                // The client side can run as root (depending on what pppd
                // does with pppob), and has no control over creation of
                // the fifo, but only opens it for reading, never for
                // creation or writing. (See FifoClient() below.)
                //
                // Therefore, we can only be attacked, via symlink,
                // by root or ourselves.
                //
                int fd = open(BARRY_FIFO_NAME, O_WRONLY | O_NONBLOCK);
                if( fd == -1 ) {
			...


> Also, Lintian in "--info --display-info --pedantic" mode still outputs
> a bunch of relevant comments.

In my testing there were only info statements, which for the most part
I thought could be ignored.  I beefed up the descriptions a bit to
avoid the extended-description-is-probably-too-short message.

For desktop-entry-contains-encoding-key, I added overrides with the
following explanation:

   # The encoding key is set to UTF-8, and this is the default, and according
   # to lintian help text, this is harmless.  For the sake of simplicity
   # and backward compatibility, it has been left in.

For no-symbols-control-file, I did some research, and found that it is
pretty tricky business to accomplish this for C++ libraries.  I added
overrides with the following message:

   # Barry is a C++ library, and as such it encounters similar struggles
   # as documented here: http://www.eyrie.org/~eagle/journal/2012-01/008.html
   # and here: http://www.eyrie.org/~eagle/journal/2012-02/001.html
   # As well, Barry already uses the ABI Compliance Checker from
   # linuxtesting.org to discover ABI/API breaks, and intends to bump the
   # major number each time, and has had this policy since 0.17.x.
   # See the following page for details:
   # http://linuxtesting.org/upstream-tracker/versions/barry.html

Those were all the lintian messages that I found on my sid testing.



Since these issues only affected the debian/ directory, I spun a version
0.18.1-2 and have released it here:

   http://sourceforge.net/projects/barry/files/barry/barry-0.18.1/sources/

I assume we can label the changelog version anything we want, until we
actually perform our first upload, and then it is written in stone.
Otherwise I have to bump the upstream version, and I'd prefer to avoid
that if possible, as it wastes space on sourceforge.net.

Let me know what you think.

Thanks,
- Chris