Merge lp:~mturquette/duplicity/duplicity into lp:~duplicity-team/duplicity/0.7-series

Proposed by Michael Turquette
Status: Rejected
Rejected by: Kenneth Loafman
Proposed branch: lp:~mturquette/duplicity/duplicity
Merge into: lp:~duplicity-team/duplicity/0.7-series
Diff against target: 38 lines (+14/-12)
1 file modified
bin/duplicity (+14/-12)
To merge this branch: bzr merge lp:~mturquette/duplicity/duplicity
Reviewer Review Type Date Requested Status
Kenneth Loafman Disapprove
edso Needs Fixing
Review via email: mp+232153@code.launchpad.net

Description of the change

I ran into a problem while using duplicity on my laptop, where I store my public gpg key, but not my private key. This is because my laptop may be stolen, lost or otherwise detained.

I still want to encrypt backups, which I can do sufficiently with a public key and Duplicity does this just fine. But things fall apart when Duplicity tries to decrypt the first volume of the backup in validate_encryption_settings() and fails on my laptop due to the lack of private key.

To handle this case I have introduced the --no-secret-key option, which was first proposed on the list in 2010:
http://article.gmane.org/gmane.comp.sysutils.backup.duplicity.general/4299

This option simply skips the encryption settings validation if set. The responsibility is on the user to periodically sanity check backups.

To post a comment you must log in.
Revision history for this message
edso (ed.so) wrote :

On 26.08.2014 03:28, Mike Turquette wrote:
> Mike Turquette has proposed merging lp:~mturquette/duplicity/duplicity into lp:duplicity.
>
> Requested reviews:
> duplicity-team (duplicity-team)
>
> For more details, see:
> https://code.launchpad.net/~mturquette/duplicity/duplicity/+merge/232153
>
> I ran into a problem while using duplicity on my laptop, where I store my public gpg key, but not my private key. This is because my laptop may be stolen, lost or otherwise detained.
>
> I still want to encrypt backups, which I can do sufficiently with a public key and Duplicity does this just fine. But things fall apart when Duplicity tries to decrypt the first volume of the backup in validate_encryption_settings() and fails on my laptop due to the lack of private key.
>
> To handle this case I have introduced the --no-secret-key option, which was first proposed on the list in 2010:
> http://article.gmane.org/gmane.comp.sysutils.backup.duplicity.general/4299

if you read the thread more thorough you'll see that the switch was meant for a totally different point in the code. the sanity decode routine was not introduced at that time.

>
> This option simply skips the encryption settings validation if set. The responsibility is on the user to periodically sanity check backups.
>

it also seems to disable the "was backup (un)encrypted before and is still check" in line 330

so i see these problems:

- collections.py must be patched as well, as drafted in the ml thread above
- the manpage bin/duplicity.1 has to be updated with a proper explanation of the parameter and it's dangers
- only disable the decryption trial in validate_encryption_settings()

make sure that you deliver a patch for lp:duplicity , which is the new current developing branch for an upcoming duplicity 0.7

..ede/duply.net

Revision history for this message
edso (ed.so) :
review: Needs Fixing
Revision history for this message
Michael Turquette (mturquette) wrote :

On Tue, Aug 26, 2014 at 2:33 AM, edso <email address hidden> wrote:
> On 26.08.2014 03:28, Mike Turquette wrote:
>> Mike Turquette has proposed merging lp:~mturquette/duplicity/duplicity into lp:duplicity.
>>
>> Requested reviews:
>> duplicity-team (duplicity-team)
>>
>> For more details, see:
>> https://code.launchpad.net/~mturquette/duplicity/duplicity/+merge/232153
>>
>> I ran into a problem while using duplicity on my laptop, where I store my public gpg key, but not my private key. This is because my laptop may be stolen, lost or otherwise detained.
>>
>> I still want to encrypt backups, which I can do sufficiently with a public key and Duplicity does this just fine. But things fall apart when Duplicity tries to decrypt the first volume of the backup in validate_encryption_settings() and fails on my laptop due to the lack of private key.
>>
>> To handle this case I have introduced the --no-secret-key option, which was first proposed on the list in 2010:
>> http://article.gmane.org/gmane.comp.sysutils.backup.duplicity.general/4299
>
>
> if you read the thread more thorough you'll see that the switch was meant for a totally different point in the code. the sanity decode routine was not introduced at that time.
>
>>
>> This option simply skips the encryption settings validation if set. The responsibility is on the user to periodically sanity check backups.
>>
>
> it also seems to disable the "was backup (un)encrypted before and is still check" in line 330
>
> so i see these problems:
>
> - collections.py must be patched as well, as drafted in the ml thread above
> - the manpage bin/duplicity.1 has to be updated with a proper explanation of the parameter and it's dangers
> - only disable the decryption trial in validate_encryption_settings()

Edgar,

Thanks for reviewing and for providing more context and pointers.

>
> make sure that you deliver a patch for lp:duplicity , which is the new current developing branch for an upcoming duplicity 0.7

My itch is scratched so I likely won't be revisiting this code any
time soon, if ever. However if I do decide to refresh this fix (e.g.
after my distro pulls a new, unpatched version of duplicity and I hit
this bug agin) then I will address your points and resubmit.

Thanks!
Mike

>
> ..ede/duply.net
>
> --
> https://code.launchpad.net/~mturquette/duplicity/duplicity/+merge/232153
> You are the owner of lp:~mturquette/duplicity/duplicity.

Revision history for this message
edso (ed.so) wrote :

On 26.08.2014 22:42, Mike Turquette wrote:
> On Tue, Aug 26, 2014 at 2:33 AM, edso <email address hidden> wrote:
>> On 26.08.2014 03:28, Mike Turquette wrote:
>>> Mike Turquette has proposed merging lp:~mturquette/duplicity/duplicity into lp:duplicity.
>>>
>>> Requested reviews:
>>> duplicity-team (duplicity-team)
>>>
>>> For more details, see:
>>> https://code.launchpad.net/~mturquette/duplicity/duplicity/+merge/232153
>>>
>>> I ran into a problem while using duplicity on my laptop, where I store my public gpg key, but not my private key. This is because my laptop may be stolen, lost or otherwise detained.
>>>
>>> I still want to encrypt backups, which I can do sufficiently with a public key and Duplicity does this just fine. But things fall apart when Duplicity tries to decrypt the first volume of the backup in validate_encryption_settings() and fails on my laptop due to the lack of private key.
>>>
>>> To handle this case I have introduced the --no-secret-key option, which was first proposed on the list in 2010:
>>> http://article.gmane.org/gmane.comp.sysutils.backup.duplicity.general/4299
>>
>>
>> if you read the thread more thorough you'll see that the switch was meant for a totally different point in the code. the sanity decode routine was not introduced at that time.
>>
>>>
>>> This option simply skips the encryption settings validation if set. The responsibility is on the user to periodically sanity check backups.
>>>
>>
>> it also seems to disable the "was backup (un)encrypted before and is still check" in line 330
>>
>> so i see these problems:
>>
>> - collections.py must be patched as well, as drafted in the ml thread above
>> - the manpage bin/duplicity.1 has to be updated with a proper explanation of the parameter and it's dangers
>> - only disable the decryption trial in validate_encryption_settings()
>
> Edgar,
>
> Thanks for reviewing and for providing more context and pointers.
>
>>
>> make sure that you deliver a patch for lp:duplicity , which is the new current developing branch for an upcoming duplicity 0.7
>
> My itch is scratched so I likely won't be revisiting this code any
> time soon, if ever. However if I do decide to refresh this fix (e.g.
> after my distro pulls a new, unpatched version of duplicity and I hit
> this bug agin) then I will address your points and resubmit.
>

up to you. hope you will as dev time on duplicity is rather sparse these days. ..ede/duply.net

Revision history for this message
edso (ed.so) wrote :

On 26.08.2014 22:42, Mike Turquette wrote:
> On Tue, Aug 26, 2014 at 2:33 AM, edso <email address hidden> wrote:
>> On 26.08.2014 03:28, Mike Turquette wrote:
>>> Mike Turquette has proposed merging lp:~mturquette/duplicity/duplicity into lp:duplicity.
>>>
>>> Requested reviews:
>>> duplicity-team (duplicity-team)
>>>
>>> For more details, see:
>>> https://code.launchpad.net/~mturquette/duplicity/duplicity/+merge/232153
>>>
>>> I ran into a problem while using duplicity on my laptop, where I store my public gpg key, but not my private key. This is because my laptop may be stolen, lost or otherwise detained.
>>>
>>> I still want to encrypt backups, which I can do sufficiently with a public key and Duplicity does this just fine. But things fall apart when Duplicity tries to decrypt the first volume of the backup in validate_encryption_settings() and fails on my laptop due to the lack of private key.
>>>
>>> To handle this case I have introduced the --no-secret-key option, which was first proposed on the list in 2010:
>>> http://article.gmane.org/gmane.comp.sysutils.backup.duplicity.general/4299
>>
>>
>> if you read the thread more thorough you'll see that the switch was meant for a totally different point in the code. the sanity decode routine was not introduced at that time.
>>
>>>
>>> This option simply skips the encryption settings validation if set. The responsibility is on the user to periodically sanity check backups.
>>>
>>
>> it also seems to disable the "was backup (un)encrypted before and is still check" in line 330
>>
>> so i see these problems:
>>
>> - collections.py must be patched as well, as drafted in the ml thread above
>> - the manpage bin/duplicity.1 has to be updated with a proper explanation of the parameter and it's dangers
>> - only disable the decryption trial in validate_encryption_settings()
>
> Edgar,
>
> Thanks for reviewing and for providing more context and pointers.
>
>>
>> make sure that you deliver a patch for lp:duplicity , which is the new current developing branch for an upcoming duplicity 0.7
>
> My itch is scratched so I likely won't be revisiting this code any
> time soon, if ever. However if I do decide to refresh this fix (e.g.
> after my distro pulls a new, unpatched version of duplicity and I hit
> this bug agin) then I will address your points and resubmit.
>

btw. current suggested method to protect your private key is to use a machine specific keypair plus your own public key. encrypt against the machine and your public key, sign with the passwordless machine secret key.

..ede/duply.net

Revision history for this message
Michael Turquette (mturquette) wrote :
Download full text (3.4 KiB)

Quoting edso (2014-08-27 01:54:20)
> On 26.08.2014 22:42, Mike Turquette wrote:
> > On Tue, Aug 26, 2014 at 2:33 AM, edso <email address hidden> wrote:
> >> On 26.08.2014 03:28, Mike Turquette wrote:
> >>> Mike Turquette has proposed merging lp:~mturquette/duplicity/duplicity into lp:duplicity.
> >>>
> >>> Requested reviews:
> >>> duplicity-team (duplicity-team)
> >>>
> >>> For more details, see:
> >>> https://code.launchpad.net/~mturquette/duplicity/duplicity/+merge/232153
> >>>
> >>> I ran into a problem while using duplicity on my laptop, where I store my public gpg key, but not my private key. This is because my laptop may be stolen, lost or otherwise detained.
> >>>
> >>> I still want to encrypt backups, which I can do sufficiently with a public key and Duplicity does this just fine. But things fall apart when Duplicity tries to decrypt the first volume of the backup in validate_encryption_settings() and fails on my laptop due to the lack of private key.
> >>>
> >>> To handle this case I have introduced the --no-secret-key option, which was first proposed on the list in 2010:
> >>> http://article.gmane.org/gmane.comp.sysutils.backup.duplicity.general/4299
> >>
> >>
> >> if you read the thread more thorough you'll see that the switch was meant for a totally different point in the code. the sanity decode routine was not introduced at that time.
> >>
> >>>
> >>> This option simply skips the encryption settings validation if set. The responsibility is on the user to periodically sanity check backups.
> >>>
> >>
> >> it also seems to disable the "was backup (un)encrypted before and is still check" in line 330
> >>
> >> so i see these problems:
> >>
> >> - collections.py must be patched as well, as drafted in the ml thread above
> >> - the manpage bin/duplicity.1 has to be updated with a proper explanation of the parameter and it's dangers
> >> - only disable the decryption trial in validate_encryption_settings()
> >
> > Edgar,
> >
> > Thanks for reviewing and for providing more context and pointers.
> >
> >>
> >> make sure that you deliver a patch for lp:duplicity , which is the new current developing branch for an upcoming duplicity 0.7
> >
> > My itch is scratched so I likely won't be revisiting this code any
> > time soon, if ever. However if I do decide to refresh this fix (e.g.
> > after my distro pulls a new, unpatched version of duplicity and I hit
> > this bug agin) then I will address your points and resubmit.
> >
>
> btw. current suggested method to protect your private key is to use a machine specific keypair plus your own public key. encrypt against the machine and your public key, sign with the passwordless machine secret key.

Thanks for the info. In fact I have settled on something similar to
this, which is another reason why reworking the --no-secret-key fix is
low priority for me.

In the course of this problem I became educated on the proper use of
subkeys and so now my laptop has the public/private subkey, but only the
public keys for my primary signing key and my other subkeys which I do
not plan to use while traveling, so I'm not even using this feature
myself right now, though it did work for me at the time that I...

Read more...

Revision history for this message
Kenneth Loafman (kenneth-loafman) wrote :

Going to mark this as rejected due to the discussion above.

review: Disapprove
Revision history for this message
Michael Turquette (mturquette) wrote :

Fine by me.

Unmerged revisions

990. By Michael Turquette

Introduce --no-secret-key option for the purpose of encrypting backups
with only a public key. Without this change duplicity begins a backup,
but then fails subsequent backups while trying to decrypt the first
volume. This scheme is most useful for laptops and other mobile devices
where the user does not wish to keep the secret key on disk.

This concept was first published to the list in 2010:
http://article.gmane.org/gmane.comp.sysutils.backup.duplicity.general/4299

Signed-off-by: Mike Turquette <email address hidden>

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'bin/duplicity'
2--- bin/duplicity 2014-05-11 11:50:12 +0000
3+++ bin/duplicity 2014-08-26 01:27:30 +0000
4@@ -323,20 +323,22 @@
5 that we're using the same encryption settings (i.e. we don't switch
6 from encrypted to non in the middle of a backup chain), so we check
7 that the vol1 filename on the server matches the settings of this run.
8+ Skip this check if the --no-secret-key option is set.
9 """
10- vol1_filename = file_naming.get(backup_type, 1,
11- encrypted=globals.encryption,
12- gzipped=globals.compression)
13- if vol1_filename != backup_set.volume_name_dict[1]:
14- log.FatalError(_("Restarting backup, but current encryption "
15- "settings do not match original settings"),
16- log.ErrorCode.enryption_mismatch)
17+ if not globals.no_secret_key:
18+ vol1_filename = file_naming.get(backup_type, 1,
19+ encrypted=globals.encryption,
20+ gzipped=globals.compression)
21+ if vol1_filename != backup_set.volume_name_dict[1]:
22+ log.FatalError(_("Restarting backup, but current encryption "
23+ "settings do not match original settings"),
24+ log.ErrorCode.enryption_mismatch)
25
26- # Settings are same, let's check passphrase itself if we are encrypted
27- if globals.encryption:
28- fileobj = restore_get_enc_fileobj(globals.backend, vol1_filename,
29- manifest.volume_info_dict[1])
30- fileobj.close()
31+ # Settings are same, let's check passphrase itself if we are encrypted
32+ if globals.encryption:
33+ fileobj = restore_get_enc_fileobj(globals.backend, vol1_filename,
34+ manifest.volume_info_dict[1])
35+ fileobj.close()
36
37 if not globals.restart:
38 # normal backup start

Subscribers

People subscribed via source and target branches