Re: [Tails-dev] Feature #5301 - Clone or Backup Persistent V…

Delete this message

Reply to this message
Autor: Andrew Gallagher
Data:  
A: tails-dev
Assumpte: Re: [Tails-dev] Feature #5301 - Clone or Backup Persistent Volume
On 07/01/16 19:41, anonym wrote:
>
> I haven't tested it, but I did have a look at src/tcp-helper.c since the
> necessity of memory unsafe code surprised me:


For the record, I never intended to use C. It was originally written in
perl (like the GUI wrapper), but setuidperl is no longer supported and I
needed to drop any requirement for sudo. The perl documentation suggests
writing a setuid C wrapper that does (more or less)
    "setreuid(0,0); system(MY_PROGRAM);"
and then lists all the reasons why you shouldn't.


So I reluctantly rewrote the helper in C, and after getting angry with C
I gave in and #included <string> (forgetting to rename the source
accordingly). Much of my time since has been spent trying to extricate
myself from the inevitable foot-shooting. ;-)

Rest of your comments fully understood, hands in the air bang to rights guv.

> Best practice is to drop the input sanitation and pick alternatives to
> `system()`/`popen()` that avoids invoking the shell and calls `exec()`
> or similar. I'm too ignorant of C/C++ to recommend you anything.


Yes, these were originally perl system(,,,) and open(,,,) calls, which
are safe as they don't invoke a shell. C has no simple equivalent that I
am aware of, so I cheated by adding the overkill input sanitation
(bearing in mind that in normal use the input parameters will in 99.9%
of cases be "/live/persistence/TailsData_unlocked" and "/dev/sd[a-z]").

> So, I personally would not include this program. Hence I recommend you
> to rewriting it in the same scripting language you end up using for the
> GUI, so Python 3, it seems.


I'd pay good money to have setuidperl back, but upstream had their reasons.

Thanks for the thorough criticism, sincerely. So I have a few questions
for the list:

1. is it worth spending any more time fixing the dodgy C++, or is it
just throwing good effort after bad?

2. is the general structure of userspace GUI + setuid helper still the
way to go?

3. Since tails-installer-invoker and tails-installer remain distinct
programs (and tails-installer closely tracks liveusb-creator), is it
necessary/desirable to merge persistence cloning into either of them, or
should/could it remain a separate (linked) executable? In other words,
do I really need to learn Python? ;-)

A