New megatools backend in 0.7.14

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
14 messages Options
Reply | Threaded
Open this post in threaded view
|

New megatools backend in 0.7.14

duplicity-talk mailing list
Hello,

The new megabackend.py calls self.subprocess_popen(cmd) where cmd is a
list of arguments (in upload function and others). But
Backend.subprocess_popen() expects a string.

The file linked in this
post: https://bugs.launchpad.net/duplicity/+bug/1394386/comments/7 calls
subprocess.call(cmd)

 --
 Richard

_______________________________________________
Duplicity-talk mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/duplicity-talk
Reply | Threaded
Open this post in threaded view
|

Re: New megatools backend in 0.7.14

duplicity-talk mailing list
On 06.09.2017 12:23, Richard McGraw via Duplicity-talk wrote:

> Hello,
>
> The new megabackend.py calls self.subprocess_popen(cmd) where cmd is a
> list of arguments (in upload function and others). But
> Backend.subprocess_popen() expects a string.
>
> The file linked in this
> post: https://bugs.launchpad.net/duplicity/+bug/1394386/comments/7 calls
> subprocess.call(cmd)
>

for ease of use, we should probably just extend backend.__subprocess_popen() to detect if commandline/args is a list and use it directly, skipping the arg list generation.

Ken: want me to give it a try?
Richard: do you want to?

..ede/duply.net

_______________________________________________
Duplicity-talk mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/duplicity-talk
Reply | Threaded
Open this post in threaded view
|

Re: New megatools backend in 0.7.14

duplicity-talk mailing list
I'll take care of it.  My bad.

...Ken


On Wed, Sep 6, 2017 at 5:58 AM, <[hidden email]> wrote:
On 06.09.2017 12:23, Richard McGraw via Duplicity-talk wrote:
> Hello,
>
> The new megabackend.py calls self.subprocess_popen(cmd) where cmd is a
> list of arguments (in upload function and others). But
> Backend.subprocess_popen() expects a string.
>
> The file linked in this
> post: https://bugs.launchpad.net/duplicity/+bug/1394386/comments/7 calls
> subprocess.call(cmd)
>

for ease of use, we should probably just extend backend.__subprocess_popen() to detect if commandline/args is a list and use it directly, skipping the arg list generation.

Ken: want me to give it a try?
Richard: do you want to?

..ede/duply.net


_______________________________________________
Duplicity-talk mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/duplicity-talk
Reply | Threaded
Open this post in threaded view
|

Re: New megatools backend in 0.7.14

duplicity-talk mailing list
ok,

the man page needs to be updated accordingly. can you take care of this as well? i just posted a request on the megabackend bug ticket and reopened it.

..ede/duply.net

On 06.09.2017 13:57, Kenneth Loafman wrote:

> I'll take care of it.  My bad.
>
> ...Ken
>
>
> On Wed, Sep 6, 2017 at 5:58 AM, <[hidden email]> wrote:
>
>> On 06.09.2017 12:23, Richard McGraw via Duplicity-talk wrote:
>>> Hello,
>>>
>>> The new megabackend.py calls self.subprocess_popen(cmd) where cmd is a
>>> list of arguments (in upload function and others). But
>>> Backend.subprocess_popen() expects a string.
>>>
>>> The file linked in this
>>> post: https://bugs.launchpad.net/duplicity/+bug/1394386/comments/7 calls
>>> subprocess.call(cmd)
>>>
>>
>> for ease of use, we should probably just extend
>> backend.__subprocess_popen() to detect if commandline/args is a list and
>> use it directly, skipping the arg list generation.
>>
>> Ken: want me to give it a try?
>> Richard: do you want to?
>>
>> ..ede/duply.net
>>
>


_______________________________________________
Duplicity-talk mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/duplicity-talk
Reply | Threaded
Open this post in threaded view
|

Re: New megatools backend in 0.7.14

duplicity-talk mailing list
No problem, will do.

...Ken


On Wed, Sep 6, 2017 at 7:32 AM, <[hidden email]> wrote:
ok,

the man page needs to be updated accordingly. can you take care of this as well? i just posted a request on the megabackend bug ticket and reopened it.

..ede/duply.net

On 06.09.2017 13:57, Kenneth Loafman wrote:
> I'll take care of it.  My bad.
>
> ...Ken
>
>
> On Wed, Sep 6, 2017 at 5:58 AM, <[hidden email]> wrote:
>
>> On 06.09.2017 12:23, Richard McGraw via Duplicity-talk wrote:
>>> Hello,
>>>
>>> The new megabackend.py calls self.subprocess_popen(cmd) where cmd is a
>>> list of arguments (in upload function and others). But
>>> Backend.subprocess_popen() expects a string.
>>>
>>> The file linked in this
>>> post: https://bugs.launchpad.net/duplicity/+bug/1394386/comments/7 calls
>>> subprocess.call(cmd)
>>>
>>
>> for ease of use, we should probably just extend
>> backend.__subprocess_popen() to detect if commandline/args is a list and
>> use it directly, skipping the arg list generation.
>>
>> Ken: want me to give it a try?
>> Richard: do you want to?
>>
>> ..ede/duply.net
>>
>



_______________________________________________
Duplicity-talk mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/duplicity-talk
Reply | Threaded
Open this post in threaded view
|

Re: New megatools backend in 0.7.14

duplicity-talk mailing list
Done.

...Ken


On Wed, Sep 6, 2017 at 8:48 AM, Kenneth Loafman <[hidden email]> wrote:
No problem, will do.

...Ken


On Wed, Sep 6, 2017 at 7:32 AM, <[hidden email]> wrote:
ok,

the man page needs to be updated accordingly. can you take care of this as well? i just posted a request on the megabackend bug ticket and reopened it.

..ede/duply.net

On 06.09.2017 13:57, Kenneth Loafman wrote:
> I'll take care of it.  My bad.
>
> ...Ken
>
>
> On Wed, Sep 6, 2017 at 5:58 AM, <[hidden email]> wrote:
>
>> On 06.09.2017 12:23, Richard McGraw via Duplicity-talk wrote:
>>> Hello,
>>>
>>> The new megabackend.py calls self.subprocess_popen(cmd) where cmd is a
>>> list of arguments (in upload function and others). But
>>> Backend.subprocess_popen() expects a string.
>>>
>>> The file linked in this
>>> post: https://bugs.launchpad.net/duplicity/+bug/1394386/comments/7 calls
>>> subprocess.call(cmd)
>>>
>>
>> for ease of use, we should probably just extend
>> backend.__subprocess_popen() to detect if commandline/args is a list and
>> use it directly, skipping the arg list generation.
>>
>> Ken: want me to give it a try?
>> Richard: do you want to?
>>
>> ..ede/duply.net
>>
>




_______________________________________________
Duplicity-talk mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/duplicity-talk
Reply | Threaded
Open this post in threaded view
|

Re: New megatools backend in 0.7.14

duplicity-talk mailing list
Ken,

sorry to rain on your parade. but joining the list is not proper. the entries may contain spaces. you could start trying to quote wrap but it'd be much easier and cleaner to simply to deliver the list down to the Popen call, which actually uses a list as well. now you join the list just to have it split into a list again later on for subprocess.Popen().

..ede/duply.net

On 9/6/2017 16:40, Kenneth Loafman wrote:

> Done.
>
> ...Ken
>
>
> On Wed, Sep 6, 2017 at 8:48 AM, Kenneth Loafman <[hidden email]> wrote:
>
>> No problem, will do.
>>
>> ...Ken
>>
>>
>> On Wed, Sep 6, 2017 at 7:32 AM, <[hidden email]> wrote:
>>
>>> ok,
>>>
>>> the man page needs to be updated accordingly. can you take care of this
>>> as well? i just posted a request on the megabackend bug ticket and reopened
>>> it.
>>>
>>> ..ede/duply.net
>>>
>>> On 06.09.2017 13:57, Kenneth Loafman wrote:
>>>> I'll take care of it.  My bad.
>>>>
>>>> ...Ken
>>>>
>>>>
>>>> On Wed, Sep 6, 2017 at 5:58 AM, <[hidden email]> wrote:
>>>>
>>>>> On 06.09.2017 12:23, Richard McGraw via Duplicity-talk wrote:
>>>>>> Hello,
>>>>>>
>>>>>> The new megabackend.py calls self.subprocess_popen(cmd) where cmd is a
>>>>>> list of arguments (in upload function and others). But
>>>>>> Backend.subprocess_popen() expects a string.
>>>>>>
>>>>>> The file linked in this
>>>>>> post: https://bugs.launchpad.net/duplicity/+bug/1394386/comments/7
>>> calls
>>>>>> subprocess.call(cmd)
>>>>>>
>>>>>
>>>>> for ease of use, we should probably just extend
>>>>> backend.__subprocess_popen() to detect if commandline/args is a list
>>> and
>>>>> use it directly, skipping the arg list generation.
>>>>>
>>>>> Ken: want me to give it a try?
>>>>> Richard: do you want to?
>>>>>
>>>>> ..ede/duply.net
>>>>>
>>>>
>>>
>>>
>>
>


_______________________________________________
Duplicity-talk mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/duplicity-talk
Reply | Threaded
Open this post in threaded view
|

Re: New megatools backend in 0.7.14

duplicity-talk mailing list
ede,

I need to do some research first.  There was a script injection bug a few years ago and we went to single string across the board, so there's more than just spaces to worry about either way.

...Ken


On Wed, Sep 6, 2017 at 4:16 PM, <[hidden email]> wrote:
Ken,

sorry to rain on your parade. but joining the list is not proper. the entries may contain spaces. you could start trying to quote wrap but it'd be much easier and cleaner to simply to deliver the list down to the Popen call, which actually uses a list as well. now you join the list just to have it split into a list again later on for subprocess.Popen().

..ede/duply.net

On 9/6/2017 16:40, Kenneth Loafman wrote:
> Done.
>
> ...Ken
>
>
> On Wed, Sep 6, 2017 at 8:48 AM, Kenneth Loafman <[hidden email]> wrote:
>
>> No problem, will do.
>>
>> ...Ken
>>
>>
>> On Wed, Sep 6, 2017 at 7:32 AM, <[hidden email]> wrote:
>>
>>> ok,
>>>
>>> the man page needs to be updated accordingly. can you take care of this
>>> as well? i just posted a request on the megabackend bug ticket and reopened
>>> it.
>>>
>>> ..ede/duply.net
>>>
>>> On 06.09.2017 13:57, Kenneth Loafman wrote:
>>>> I'll take care of it.  My bad.
>>>>
>>>> ...Ken
>>>>
>>>>
>>>> On Wed, Sep 6, 2017 at 5:58 AM, <[hidden email]> wrote:
>>>>
>>>>> On 06.09.2017 12:23, Richard McGraw via Duplicity-talk wrote:
>>>>>> Hello,
>>>>>>
>>>>>> The new megabackend.py calls self.subprocess_popen(cmd) where cmd is a
>>>>>> list of arguments (in upload function and others). But
>>>>>> Backend.subprocess_popen() expects a string.
>>>>>>
>>>>>> The file linked in this
>>>>>> post: https://bugs.launchpad.net/duplicity/+bug/1394386/comments/7
>>> calls
>>>>>> subprocess.call(cmd)
>>>>>>
>>>>>
>>>>> for ease of use, we should probably just extend
>>>>> backend.__subprocess_popen() to detect if commandline/args is a list
>>> and
>>>>> use it directly, skipping the arg list generation.
>>>>>
>>>>> Ken: want me to give it a try?
>>>>> Richard: do you want to?
>>>>>
>>>>> ..ede/duply.net
>>>>>
>>>>
>>>
>>>
>>
>



_______________________________________________
Duplicity-talk mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/duplicity-talk
Reply | Threaded
Open this post in threaded view
|

Re: New megatools backend in 0.7.14

duplicity-talk mailing list
i seem to remember the issue. this one?
  https://lists.launchpad.net/duplicity-team/msg03262.html

there is pretty little we can do for programms like eg. rsync that run other programs according to parameter (eg. rsync --rsh "rm -rf /").

but as the bug
  https://bugs.launchpad.net/ubuntu/+source/duplicity/+bug/1519103
states correctly subprocess.Popen(), which we use works correctly as it only runs the binary named in args[0] and wraps the rest of the parameters, having bad parameters in the worst case, but no shell injection.

so again, just route the args list to subprocess.Popen() and everything is safe and sound. i think we switched to subprocess.Popen() in backend exactly for this reason.

..ede/duply.net


On 07.09.2017 12:31, Kenneth Loafman wrote:

> ede,
>
> I need to do some research first.  There was a script injection bug a few
> years ago and we went to single string across the board, so there's more
> than just spaces to worry about either way.
>
> ...Ken
>
>
> On Wed, Sep 6, 2017 at 4:16 PM, <[hidden email]> wrote:
>
>> Ken,
>>
>> sorry to rain on your parade. but joining the list is not proper. the
>> entries may contain spaces. you could start trying to quote wrap but it'd
>> be much easier and cleaner to simply to deliver the list down to the Popen
>> call, which actually uses a list as well. now you join the list just to
>> have it split into a list again later on for subprocess.Popen().
>>
>> ..ede/duply.net
>>
>> On 9/6/2017 16:40, Kenneth Loafman wrote:
>>> Done.
>>>
>>> ...Ken
>>>
>>>
>>> On Wed, Sep 6, 2017 at 8:48 AM, Kenneth Loafman <[hidden email]>
>> wrote:
>>>
>>>> No problem, will do.
>>>>
>>>> ...Ken
>>>>
>>>>
>>>> On Wed, Sep 6, 2017 at 7:32 AM, <[hidden email]> wrote:
>>>>
>>>>> ok,
>>>>>
>>>>> the man page needs to be updated accordingly. can you take care of this
>>>>> as well? i just posted a request on the megabackend bug ticket and
>> reopened
>>>>> it.
>>>>>
>>>>> ..ede/duply.net
>>>>>
>>>>> On 06.09.2017 13:57, Kenneth Loafman wrote:
>>>>>> I'll take care of it.  My bad.
>>>>>>
>>>>>> ...Ken
>>>>>>
>>>>>>
>>>>>> On Wed, Sep 6, 2017 at 5:58 AM, <[hidden email]> wrote:
>>>>>>
>>>>>>> On 06.09.2017 12:23, Richard McGraw via Duplicity-talk wrote:
>>>>>>>> Hello,
>>>>>>>>
>>>>>>>> The new megabackend.py calls self.subprocess_popen(cmd) where cmd
>> is a
>>>>>>>> list of arguments (in upload function and others). But
>>>>>>>> Backend.subprocess_popen() expects a string.
>>>>>>>>
>>>>>>>> The file linked in this
>>>>>>>> post: https://bugs.launchpad.net/duplicity/+bug/1394386/comments/7
>>>>> calls
>>>>>>>> subprocess.call(cmd)
>>>>>>>>
>>>>>>>
>>>>>>> for ease of use, we should probably just extend
>>>>>>> backend.__subprocess_popen() to detect if commandline/args is a list
>>>>> and
>>>>>>> use it directly, skipping the arg list generation.
>>>>>>>
>>>>>>> Ken: want me to give it a try?
>>>>>>> Richard: do you want to?
>>>>>>>
>>>>>>> ..ede/duply.net
>>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>
>>>
>>
>>
>


_______________________________________________
Duplicity-talk mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/duplicity-talk
Reply | Threaded
Open this post in threaded view
|

Re: New megatools backend in 0.7.14

duplicity-talk mailing list
Yes, that one, so no problem now.

It's fixed and committed.

...Ken


On Thu, Sep 7, 2017 at 7:22 AM, <[hidden email]> wrote:
i seem to remember the issue. this one?
  https://lists.launchpad.net/duplicity-team/msg03262.html

there is pretty little we can do for programms like eg. rsync that run other programs according to parameter (eg. rsync --rsh "rm -rf /").

but as the bug
  https://bugs.launchpad.net/ubuntu/+source/duplicity/+bug/1519103
states correctly subprocess.Popen(), which we use works correctly as it only runs the binary named in args[0] and wraps the rest of the parameters, having bad parameters in the worst case, but no shell injection.

so again, just route the args list to subprocess.Popen() and everything is safe and sound. i think we switched to subprocess.Popen() in backend exactly for this reason.

..ede/duply.net


On 07.09.2017 12:31, Kenneth Loafman wrote:
> ede,
>
> I need to do some research first.  There was a script injection bug a few
> years ago and we went to single string across the board, so there's more
> than just spaces to worry about either way.
>
> ...Ken
>
>
> On Wed, Sep 6, 2017 at 4:16 PM, <[hidden email]> wrote:
>
>> Ken,
>>
>> sorry to rain on your parade. but joining the list is not proper. the
>> entries may contain spaces. you could start trying to quote wrap but it'd
>> be much easier and cleaner to simply to deliver the list down to the Popen
>> call, which actually uses a list as well. now you join the list just to
>> have it split into a list again later on for subprocess.Popen().
>>
>> ..ede/duply.net
>>
>> On 9/6/2017 16:40, Kenneth Loafman wrote:
>>> Done.
>>>
>>> ...Ken
>>>
>>>
>>> On Wed, Sep 6, 2017 at 8:48 AM, Kenneth Loafman <[hidden email]>
>> wrote:
>>>
>>>> No problem, will do.
>>>>
>>>> ...Ken
>>>>
>>>>
>>>> On Wed, Sep 6, 2017 at 7:32 AM, <[hidden email]> wrote:
>>>>
>>>>> ok,
>>>>>
>>>>> the man page needs to be updated accordingly. can you take care of this
>>>>> as well? i just posted a request on the megabackend bug ticket and
>> reopened
>>>>> it.
>>>>>
>>>>> ..ede/duply.net
>>>>>
>>>>> On 06.09.2017 13:57, Kenneth Loafman wrote:
>>>>>> I'll take care of it.  My bad.
>>>>>>
>>>>>> ...Ken
>>>>>>
>>>>>>
>>>>>> On Wed, Sep 6, 2017 at 5:58 AM, <[hidden email]> wrote:
>>>>>>
>>>>>>> On 06.09.2017 12:23, Richard McGraw via Duplicity-talk wrote:
>>>>>>>> Hello,
>>>>>>>>
>>>>>>>> The new megabackend.py calls self.subprocess_popen(cmd) where cmd
>> is a
>>>>>>>> list of arguments (in upload function and others). But
>>>>>>>> Backend.subprocess_popen() expects a string.
>>>>>>>>
>>>>>>>> The file linked in this
>>>>>>>> post: https://bugs.launchpad.net/duplicity/+bug/1394386/comments/7
>>>>> calls
>>>>>>>> subprocess.call(cmd)
>>>>>>>>
>>>>>>>
>>>>>>> for ease of use, we should probably just extend
>>>>>>> backend.__subprocess_popen() to detect if commandline/args is a list
>>>>> and
>>>>>>> use it directly, skipping the arg list generation.
>>>>>>>
>>>>>>> Ken: want me to give it a try?
>>>>>>> Richard: do you want to?
>>>>>>>
>>>>>>> ..ede/duply.net
>>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>
>>>
>>
>>
>



_______________________________________________
Duplicity-talk mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/duplicity-talk
Reply | Threaded
Open this post in threaded view
|

Re: New megatools backend in 0.7.14

duplicity-talk mailing list
hmm,

did you possibly forget to modify backend.__subprocess_popen() accordingly? afaics it still tries to split the commandline argument via shlex.
  http://bazaar.launchpad.net/~duplicity-team/duplicity/0.8-series/view/head:/duplicity/backend.py#L453

maybe you could move the logging there as well?

..ede/duply.net

On 07.09.2017 16:48, Kenneth Loafman wrote:

> Yes, that one, so no problem now.
>
> It's fixed and committed.
>
> ...Ken
>
>
> On Thu, Sep 7, 2017 at 7:22 AM, <[hidden email]> wrote:
>
>> i seem to remember the issue. this one?
>>   https://lists.launchpad.net/duplicity-team/msg03262.html
>>
>> there is pretty little we can do for programms like eg. rsync that run
>> other programs according to parameter (eg. rsync --rsh "rm -rf /").
>>
>> but as the bug
>>   https://bugs.launchpad.net/ubuntu/+source/duplicity/+bug/1519103
>> states correctly subprocess.Popen(), which we use works correctly as it
>> only runs the binary named in args[0] and wraps the rest of the parameters,
>> having bad parameters in the worst case, but no shell injection.
>>
>> so again, just route the args list to subprocess.Popen() and everything is
>> safe and sound. i think we switched to subprocess.Popen() in backend
>> exactly for this reason.
>>
>> ..ede/duply.net
>>
>>
>> On 07.09.2017 12:31, Kenneth Loafman wrote:
>>> ede,
>>>
>>> I need to do some research first.  There was a script injection bug a few
>>> years ago and we went to single string across the board, so there's more
>>> than just spaces to worry about either way.
>>>
>>> ...Ken
>>>
>>>
>>> On Wed, Sep 6, 2017 at 4:16 PM, <[hidden email]> wrote:
>>>
>>>> Ken,
>>>>
>>>> sorry to rain on your parade. but joining the list is not proper. the
>>>> entries may contain spaces. you could start trying to quote wrap but
>> it'd
>>>> be much easier and cleaner to simply to deliver the list down to the
>> Popen
>>>> call, which actually uses a list as well. now you join the list just to
>>>> have it split into a list again later on for subprocess.Popen().
>>>>
>>>> ..ede/duply.net
>>>>
>>>> On 9/6/2017 16:40, Kenneth Loafman wrote:
>>>>> Done.
>>>>>
>>>>> ...Ken
>>>>>
>>>>>
>>>>> On Wed, Sep 6, 2017 at 8:48 AM, Kenneth Loafman <[hidden email]>
>>>> wrote:
>>>>>
>>>>>> No problem, will do.
>>>>>>
>>>>>> ...Ken
>>>>>>
>>>>>>
>>>>>> On Wed, Sep 6, 2017 at 7:32 AM, <[hidden email]> wrote:
>>>>>>
>>>>>>> ok,
>>>>>>>
>>>>>>> the man page needs to be updated accordingly. can you take care of
>> this
>>>>>>> as well? i just posted a request on the megabackend bug ticket and
>>>> reopened
>>>>>>> it.
>>>>>>>
>>>>>>> ..ede/duply.net
>>>>>>>
>>>>>>> On 06.09.2017 13:57, Kenneth Loafman wrote:
>>>>>>>> I'll take care of it.  My bad.
>>>>>>>>
>>>>>>>> ...Ken
>>>>>>>>
>>>>>>>>
>>>>>>>> On Wed, Sep 6, 2017 at 5:58 AM, <[hidden email]> wrote:
>>>>>>>>
>>>>>>>>> On 06.09.2017 12:23, Richard McGraw via Duplicity-talk wrote:
>>>>>>>>>> Hello,
>>>>>>>>>>
>>>>>>>>>> The new megabackend.py calls self.subprocess_popen(cmd) where cmd
>>>> is a
>>>>>>>>>> list of arguments (in upload function and others). But
>>>>>>>>>> Backend.subprocess_popen() expects a string.
>>>>>>>>>>
>>>>>>>>>> The file linked in this
>>>>>>>>>> post: https://bugs.launchpad.net/duplicity/+bug/1394386/
>> comments/7
>>>>>>> calls
>>>>>>>>>> subprocess.call(cmd)
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> for ease of use, we should probably just extend
>>>>>>>>> backend.__subprocess_popen() to detect if commandline/args is a
>> list
>>>>>>> and
>>>>>>>>> use it directly, skipping the arg list generation.
>>>>>>>>>
>>>>>>>>> Ken: want me to give it a try?
>>>>>>>>> Richard: do you want to?
>>>>>>>>>
>>>>>>>>> ..ede/duply.net
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>>
>>>
>>
>>
>


_______________________________________________
Duplicity-talk mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/duplicity-talk
Reply | Threaded
Open this post in threaded view
|

Re: New megatools backend in 0.7.14

duplicity-talk mailing list
OK, this last update should do it.  Cleaned up some as well.

...Ken


On Thu, Sep 7, 2017 at 10:17 AM, <[hidden email]> wrote:
hmm,

did you possibly forget to modify backend.__subprocess_popen() accordingly? afaics it still tries to split the commandline argument via shlex.
  http://bazaar.launchpad.net/~duplicity-team/duplicity/0.8-series/view/head:/duplicity/backend.py#L453

maybe you could move the logging there as well?

..ede/duply.net

On 07.09.2017 16:48, Kenneth Loafman wrote:
> Yes, that one, so no problem now.
>
> It's fixed and committed.
>
> ...Ken
>
>
> On Thu, Sep 7, 2017 at 7:22 AM, <[hidden email]> wrote:
>
>> i seem to remember the issue. this one?
>>   https://lists.launchpad.net/duplicity-team/msg03262.html
>>
>> there is pretty little we can do for programms like eg. rsync that run
>> other programs according to parameter (eg. rsync --rsh "rm -rf /").
>>
>> but as the bug
>>   https://bugs.launchpad.net/ubuntu/+source/duplicity/+bug/1519103
>> states correctly subprocess.Popen(), which we use works correctly as it
>> only runs the binary named in args[0] and wraps the rest of the parameters,
>> having bad parameters in the worst case, but no shell injection.
>>
>> so again, just route the args list to subprocess.Popen() and everything is
>> safe and sound. i think we switched to subprocess.Popen() in backend
>> exactly for this reason.
>>
>> ..ede/duply.net
>>
>>
>> On 07.09.2017 12:31, Kenneth Loafman wrote:
>>> ede,
>>>
>>> I need to do some research first.  There was a script injection bug a few
>>> years ago and we went to single string across the board, so there's more
>>> than just spaces to worry about either way.
>>>
>>> ...Ken
>>>
>>>
>>> On Wed, Sep 6, 2017 at 4:16 PM, <[hidden email]> wrote:
>>>
>>>> Ken,
>>>>
>>>> sorry to rain on your parade. but joining the list is not proper. the
>>>> entries may contain spaces. you could start trying to quote wrap but
>> it'd
>>>> be much easier and cleaner to simply to deliver the list down to the
>> Popen
>>>> call, which actually uses a list as well. now you join the list just to
>>>> have it split into a list again later on for subprocess.Popen().
>>>>
>>>> ..ede/duply.net
>>>>
>>>> On 9/6/2017 16:40, Kenneth Loafman wrote:
>>>>> Done.
>>>>>
>>>>> ...Ken
>>>>>
>>>>>
>>>>> On Wed, Sep 6, 2017 at 8:48 AM, Kenneth Loafman <[hidden email]>
>>>> wrote:
>>>>>
>>>>>> No problem, will do.
>>>>>>
>>>>>> ...Ken
>>>>>>
>>>>>>
>>>>>> On Wed, Sep 6, 2017 at 7:32 AM, <[hidden email]> wrote:
>>>>>>
>>>>>>> ok,
>>>>>>>
>>>>>>> the man page needs to be updated accordingly. can you take care of
>> this
>>>>>>> as well? i just posted a request on the megabackend bug ticket and
>>>> reopened
>>>>>>> it.
>>>>>>>
>>>>>>> ..ede/duply.net
>>>>>>>
>>>>>>> On 06.09.2017 13:57, Kenneth Loafman wrote:
>>>>>>>> I'll take care of it.  My bad.
>>>>>>>>
>>>>>>>> ...Ken
>>>>>>>>
>>>>>>>>
>>>>>>>> On Wed, Sep 6, 2017 at 5:58 AM, <[hidden email]> wrote:
>>>>>>>>
>>>>>>>>> On 06.09.2017 12:23, Richard McGraw via Duplicity-talk wrote:
>>>>>>>>>> Hello,
>>>>>>>>>>
>>>>>>>>>> The new megabackend.py calls self.subprocess_popen(cmd) where cmd
>>>> is a
>>>>>>>>>> list of arguments (in upload function and others). But
>>>>>>>>>> Backend.subprocess_popen() expects a string.
>>>>>>>>>>
>>>>>>>>>> The file linked in this
>>>>>>>>>> post: https://bugs.launchpad.net/duplicity/+bug/1394386/
>> comments/7
>>>>>>> calls
>>>>>>>>>> subprocess.call(cmd)
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> for ease of use, we should probably just extend
>>>>>>>>> backend.__subprocess_popen() to detect if commandline/args is a
>> list
>>>>>>> and
>>>>>>>>> use it directly, skipping the arg list generation.
>>>>>>>>>
>>>>>>>>> Ken: want me to give it a try?
>>>>>>>>> Richard: do you want to?
>>>>>>>>>
>>>>>>>>> ..ede/duply.net
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>>
>>>
>>
>>
>



_______________________________________________
Duplicity-talk mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/duplicity-talk
Reply | Threaded
Open this post in threaded view
|

Re: New megatools backend in 0.7.14

duplicity-talk mailing list
that looks as if it should work :), all is well that ends well.. ede

PS: sorry to have been persistent.

On 9/7/2017 19:01, Kenneth Loafman wrote:

> OK, this last update should do it.  Cleaned up some as well.
>
> ...Ken
>
>
> On Thu, Sep 7, 2017 at 10:17 AM, <[hidden email]> wrote:
>
>> hmm,
>>
>> did you possibly forget to modify backend.__subprocess_popen()
>> accordingly? afaics it still tries to split the commandline argument via
>> shlex.
>>   http://bazaar.launchpad.net/~duplicity-team/duplicity/0.8-
>> series/view/head:/duplicity/backend.py#L453
>>
>> maybe you could move the logging there as well?
>>
>> ..ede/duply.net
>>
>> On 07.09.2017 16:48, Kenneth Loafman wrote:
>>> Yes, that one, so no problem now.
>>>
>>> It's fixed and committed.
>>>
>>> ...Ken
>>>
>>>
>>> On Thu, Sep 7, 2017 at 7:22 AM, <[hidden email]> wrote:
>>>
>>>> i seem to remember the issue. this one?
>>>>   https://lists.launchpad.net/duplicity-team/msg03262.html
>>>>
>>>> there is pretty little we can do for programms like eg. rsync that run
>>>> other programs according to parameter (eg. rsync --rsh "rm -rf /").
>>>>
>>>> but as the bug
>>>>   https://bugs.launchpad.net/ubuntu/+source/duplicity/+bug/1519103
>>>> states correctly subprocess.Popen(), which we use works correctly as it
>>>> only runs the binary named in args[0] and wraps the rest of the
>> parameters,
>>>> having bad parameters in the worst case, but no shell injection.
>>>>
>>>> so again, just route the args list to subprocess.Popen() and everything
>> is
>>>> safe and sound. i think we switched to subprocess.Popen() in backend
>>>> exactly for this reason.
>>>>
>>>> ..ede/duply.net
>>>>
>>>>
>>>> On 07.09.2017 12:31, Kenneth Loafman wrote:
>>>>> ede,
>>>>>
>>>>> I need to do some research first.  There was a script injection bug a
>> few
>>>>> years ago and we went to single string across the board, so there's
>> more
>>>>> than just spaces to worry about either way.
>>>>>
>>>>> ...Ken
>>>>>
>>>>>
>>>>> On Wed, Sep 6, 2017 at 4:16 PM, <[hidden email]> wrote:
>>>>>
>>>>>> Ken,
>>>>>>
>>>>>> sorry to rain on your parade. but joining the list is not proper. the
>>>>>> entries may contain spaces. you could start trying to quote wrap but
>>>> it'd
>>>>>> be much easier and cleaner to simply to deliver the list down to the
>>>> Popen
>>>>>> call, which actually uses a list as well. now you join the list just
>> to
>>>>>> have it split into a list again later on for subprocess.Popen().
>>>>>>
>>>>>> ..ede/duply.net
>>>>>>
>>>>>> On 9/6/2017 16:40, Kenneth Loafman wrote:
>>>>>>> Done.
>>>>>>>
>>>>>>> ...Ken
>>>>>>>
>>>>>>>
>>>>>>> On Wed, Sep 6, 2017 at 8:48 AM, Kenneth Loafman <[hidden email]
>>>
>>>>>> wrote:
>>>>>>>
>>>>>>>> No problem, will do.
>>>>>>>>
>>>>>>>> ...Ken
>>>>>>>>
>>>>>>>>
>>>>>>>> On Wed, Sep 6, 2017 at 7:32 AM, <[hidden email]> wrote:
>>>>>>>>
>>>>>>>>> ok,
>>>>>>>>>
>>>>>>>>> the man page needs to be updated accordingly. can you take care of
>>>> this
>>>>>>>>> as well? i just posted a request on the megabackend bug ticket and
>>>>>> reopened
>>>>>>>>> it.
>>>>>>>>>
>>>>>>>>> ..ede/duply.net
>>>>>>>>>
>>>>>>>>> On 06.09.2017 13:57, Kenneth Loafman wrote:
>>>>>>>>>> I'll take care of it.  My bad.
>>>>>>>>>>
>>>>>>>>>> ...Ken
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> On Wed, Sep 6, 2017 at 5:58 AM, <[hidden email]> wrote:
>>>>>>>>>>
>>>>>>>>>>> On 06.09.2017 12:23, Richard McGraw via Duplicity-talk wrote:
>>>>>>>>>>>> Hello,
>>>>>>>>>>>>
>>>>>>>>>>>> The new megabackend.py calls self.subprocess_popen(cmd) where
>> cmd
>>>>>> is a
>>>>>>>>>>>> list of arguments (in upload function and others). But
>>>>>>>>>>>> Backend.subprocess_popen() expects a string.
>>>>>>>>>>>>
>>>>>>>>>>>> The file linked in this
>>>>>>>>>>>> post: https://bugs.launchpad.net/duplicity/+bug/1394386/
>>>> comments/7
>>>>>>>>> calls
>>>>>>>>>>>> subprocess.call(cmd)
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> for ease of use, we should probably just extend
>>>>>>>>>>> backend.__subprocess_popen() to detect if commandline/args is a
>>>> list
>>>>>>>>> and
>>>>>>>>>>> use it directly, skipping the arg list generation.
>>>>>>>>>>>
>>>>>>>>>>> Ken: want me to give it a try?
>>>>>>>>>>> Richard: do you want to?
>>>>>>>>>>>
>>>>>>>>>>> ..ede/duply.net
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>
>>>>
>>>
>>
>>
>


_______________________________________________
Duplicity-talk mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/duplicity-talk
Reply | Threaded
Open this post in threaded view
|

Re: New megatools backend in 0.7.14

duplicity-talk mailing list
If you weren't, our users would be.  Thanks for the catches!

...Ken


On Thu, Sep 7, 2017 at 3:05 PM, <[hidden email]> wrote:
that looks as if it should work :), all is well that ends well.. ede

PS: sorry to have been persistent.

On 9/7/2017 19:01, Kenneth Loafman wrote:
> OK, this last update should do it.  Cleaned up some as well.
>
> ...Ken
>
>
> On Thu, Sep 7, 2017 at 10:17 AM, <[hidden email]> wrote:
>
>> hmm,
>>
>> did you possibly forget to modify backend.__subprocess_popen()
>> accordingly? afaics it still tries to split the commandline argument via
>> shlex.
>>   http://bazaar.launchpad.net/~duplicity-team/duplicity/0.8-
>> series/view/head:/duplicity/backend.py#L453
>>
>> maybe you could move the logging there as well?
>>
>> ..ede/duply.net
>>
>> On 07.09.2017 16:48, Kenneth Loafman wrote:
>>> Yes, that one, so no problem now.
>>>
>>> It's fixed and committed.
>>>
>>> ...Ken
>>>
>>>
>>> On Thu, Sep 7, 2017 at 7:22 AM, <[hidden email]> wrote:
>>>
>>>> i seem to remember the issue. this one?
>>>>   https://lists.launchpad.net/duplicity-team/msg03262.html
>>>>
>>>> there is pretty little we can do for programms like eg. rsync that run
>>>> other programs according to parameter (eg. rsync --rsh "rm -rf /").
>>>>
>>>> but as the bug
>>>>   https://bugs.launchpad.net/ubuntu/+source/duplicity/+bug/1519103
>>>> states correctly subprocess.Popen(), which we use works correctly as it
>>>> only runs the binary named in args[0] and wraps the rest of the
>> parameters,
>>>> having bad parameters in the worst case, but no shell injection.
>>>>
>>>> so again, just route the args list to subprocess.Popen() and everything
>> is
>>>> safe and sound. i think we switched to subprocess.Popen() in backend
>>>> exactly for this reason.
>>>>
>>>> ..ede/duply.net
>>>>
>>>>
>>>> On 07.09.2017 12:31, Kenneth Loafman wrote:
>>>>> ede,
>>>>>
>>>>> I need to do some research first.  There was a script injection bug a
>> few
>>>>> years ago and we went to single string across the board, so there's
>> more
>>>>> than just spaces to worry about either way.
>>>>>
>>>>> ...Ken
>>>>>
>>>>>
>>>>> On Wed, Sep 6, 2017 at 4:16 PM, <[hidden email]> wrote:
>>>>>
>>>>>> Ken,
>>>>>>
>>>>>> sorry to rain on your parade. but joining the list is not proper. the
>>>>>> entries may contain spaces. you could start trying to quote wrap but
>>>> it'd
>>>>>> be much easier and cleaner to simply to deliver the list down to the
>>>> Popen
>>>>>> call, which actually uses a list as well. now you join the list just
>> to
>>>>>> have it split into a list again later on for subprocess.Popen().
>>>>>>
>>>>>> ..ede/duply.net
>>>>>>
>>>>>> On 9/6/2017 16:40, Kenneth Loafman wrote:
>>>>>>> Done.
>>>>>>>
>>>>>>> ...Ken
>>>>>>>
>>>>>>>
>>>>>>> On Wed, Sep 6, 2017 at 8:48 AM, Kenneth Loafman <[hidden email]
>>>
>>>>>> wrote:
>>>>>>>
>>>>>>>> No problem, will do.
>>>>>>>>
>>>>>>>> ...Ken
>>>>>>>>
>>>>>>>>
>>>>>>>> On Wed, Sep 6, 2017 at 7:32 AM, <[hidden email]> wrote:
>>>>>>>>
>>>>>>>>> ok,
>>>>>>>>>
>>>>>>>>> the man page needs to be updated accordingly. can you take care of
>>>> this
>>>>>>>>> as well? i just posted a request on the megabackend bug ticket and
>>>>>> reopened
>>>>>>>>> it.
>>>>>>>>>
>>>>>>>>> ..ede/duply.net
>>>>>>>>>
>>>>>>>>> On 06.09.2017 13:57, Kenneth Loafman wrote:
>>>>>>>>>> I'll take care of it.  My bad.
>>>>>>>>>>
>>>>>>>>>> ...Ken
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> On Wed, Sep 6, 2017 at 5:58 AM, <[hidden email]> wrote:
>>>>>>>>>>
>>>>>>>>>>> On 06.09.2017 12:23, Richard McGraw via Duplicity-talk wrote:
>>>>>>>>>>>> Hello,
>>>>>>>>>>>>
>>>>>>>>>>>> The new megabackend.py calls self.subprocess_popen(cmd) where
>> cmd
>>>>>> is a
>>>>>>>>>>>> list of arguments (in upload function and others). But
>>>>>>>>>>>> Backend.subprocess_popen() expects a string.
>>>>>>>>>>>>
>>>>>>>>>>>> The file linked in this
>>>>>>>>>>>> post: https://bugs.launchpad.net/duplicity/+bug/1394386/
>>>> comments/7
>>>>>>>>> calls
>>>>>>>>>>>> subprocess.call(cmd)
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> for ease of use, we should probably just extend
>>>>>>>>>>> backend.__subprocess_popen() to detect if commandline/args is a
>>>> list
>>>>>>>>> and
>>>>>>>>>>> use it directly, skipping the arg list generation.
>>>>>>>>>>>
>>>>>>>>>>> Ken: want me to give it a try?
>>>>>>>>>>> Richard: do you want to?
>>>>>>>>>>>
>>>>>>>>>>> ..ede/duply.net
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>
>>>>
>>>
>>
>>
>



_______________________________________________
Duplicity-talk mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/duplicity-talk