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

Delete this message

Reply to this message
Autore: anonym
Data:  
To: The Tails public development discussion list
Oggetto: Re: [Tails-dev] Feature #5301 - Clone or Backup Persistent Volume
Andrew Gallagher:
> 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.


Ok. I'm not entirely sure why you'd need setuid.

> 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. ;-)


Ack!

> Rest of your comments fully understood,


:)

> hands in the air bang to rights guv.


What does this mean? I'm not a native English speaker, which possibly
explains it.

>> 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.


Ok. Still, I'm confused by the need for setuid and going C(++). For the
record, Python's subprocess module would do the same job:

    https://docs.python.org/3/library/subprocess.html


> C has no simple equivalent that I
> am aware of,


I guess that'd be a combo of fork() + exec() + IPC to get the results
(return code, stdout/err) back to the parent. :) Or, preferably, using
some existing library which makes this easier.

> 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]").


Understood.

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


It's not worth the effort. Sorry, but let's drop the C/C++.

> 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? ;-)


I mainly threw myself in the discussion to avert us from adding any
C/C++ code. I leave it to sajolida and u to decide on how the backup
tool should integrate into the installer (or if it should be separate),
and for u and intrigeri to clarify the privileges separation situation
in the installer (I think it's run as a normal users, and udisks is used
to partition, format, luksOpen etc without any risky setuid business).
All this will influence the answers to these questions.

Cheers!