Regression: Duplicity will (wrongly) accept a change of encryption password during an incremental backup

We've gone back and forth on this! Or rather Ubuntu upstream and I have. The only possible "fix" I can think of that will not be removed by upstream is to create a ~/.config/duplicity dir and add a passwd_hash_<backup_name> to it using a strong hash. This would be as safe as the systems password, so should be acceptable to all.

Thoughts?

added bugmedium label

I thought I had fixed this in the past with the validate_encryption_settings method in dup_main.py -- I can investigate this today and maybe propose a change. I know 1.0.0 changed how it interprets gpg errors and that may be a part of why something is different in 0.8 and 1.0.

Thanks! Yes, we changed to use the returncode rather than stderr output. That lets us work when the text of stderr is not English.

Your user may have been bit by this:

        if not (config.s3_use_deep_archive or config.s3_use_glacier or config.s3_use_glacier_ir):
            validate_encryption_settings(config.restart.last_backup, mf)
        else:
            log.Warn(_(u"Skipping encryption validation due to glacier/deep storage"))

Good luck!

Can you check the patches Ubuntu applies? The problem may be there as well.

I think the recent gpg changes are not correct -- or at least, my copy of gpg acts differently than yours. The return code from the process itself is always 2 when an error occurs (for me).

Further, the recent change ignores return-code 2 (which is the only return code I get for errors) because that's the invalid-packet error. So in my testing, we end up ignoring all gpg errors.

Looking at the gnupg source, the always-returning-2-for-error-case seems like intended behavior (look at uses of g10_exit). Looks like we would have to scan the result of the status-fd if we want to machine-parse for gpg errors. And even then, it's not a convenient thing like 'GPG_ERROR: xxx' but rather each error situation has a custom string we'd have to look for (which we could do using the same kind of thing gpg_error_codes.py does).

For example, for a bad password encryption check, the following is given on the status fd, and we'd have to look for the DECRYPTION_FAILED line:

[GNUPG:] NEED_PASSPHRASE_SYM 9 3 2
[GNUPG:] BEGIN_DECRYPTION
[GNUPG:] DECRYPTION_COMPLIANCE_MODE 23
[GNUPG:] DECRYPTION_INFO 2 9 0
[GNUPG:] DECRYPTION_FAILED
[GNUPG:] END_DECRYPTION

I think something else is wrong. We went to returncode because they have an exhaustive list of return codes and their meaning in gpg_error_codes.py and we could use that instead of scanning. If we go to scanning status fd again, we might as well drop any internationalization, which would be a very bad thing for non-English speakers.

So, bottom line, how do we get an accurate return code? I don't want to have to tell non-English speakers that we have to run only on English locales.

I understand your desire for machine-readable error codes. I'm saying that I don't think the return-code-reading bits in 1.0.0 are working like you expect.

At least, not for my copy of the gpg executable. Are you using one that isn't GnuPG? GnuPG looks like it always returns 2 when a gpg error happens and puts the real interesting error code inside the status FD.

(Side note: Deja Dup notices bad password decryption attempts for non-English locales by scanning the duplicity output for the translated version of the expected error message by calling gpg_strerror. I'd love it if I could drop that for a more reliable method, though it has been working alright for years.)

(Second side note: I was hoping to fix the first case mentioned in this ticket -- "Bug: allows change of password mid-incremental with cache in 0.8.23" by making the encryption-failure-when-reading-the-remote-manifest a fatal error. But looks like that is intentionally not a fatal error. Feels like it maybe should be though...?)

The only error code we scan for is the invalid-packet one? Is that the only international concern? Might be easy enough to dig through status-fd for that one code in that case (looks like it might print something like [GNUPG:] STATUS_NODATA 3 based on a quick scan of gnupg code)

We don't scan the Status FD at all anymore. We list it so the user might get something from it, but we use the return code from the process. We must be doing something wrong there, or GPG is broken for any rational form of use. Scanning while translated means that GPG translations have to be the same as ours (Crowdin), or we will miss it. That's not an acceptable solution.

We used to scan for something like ctb=14 and ignore that for error 2, then just

I will need to do some more searching and cursing. Shoulda gone to GPGMe instead.

Scanning while translated means that GPG translations have to be the same as ours (Crowdin), or we will miss it. That's not an acceptable solution.

The way Deja Dup does it is to call gpg_strerror(GPG_ERR_BAD_KEY) to get the official translation for that error code, then scans for that -- so no mismatch.

There is a possibility that the gpg-error library you link against and the running gpg binary could have been from different gpg releases, if the user has a non-traditional setup. Then the string might not match as a result. Not saying it's perfect, but anyway, that's how we've worked around this problem for a while.

But if duplicity did manage to extract real machine-readable codes, then exposed it in the logging system, Deja Dup could parse that and stop doing this string-searching. That would be nice.

I'm not linking against anything, just using the stock gpg and the error strings I found in libgpg, using this tool.

I will revert the changes then try to find a new fix. The simplest would be to use your scanning approach with gpg_error_codes.py and hope the translations agree. Either way, we would still have the stderr_fp log to help the users. (@edeso Good call on that one!)

Correction on above, status_fp ==> stderr_fp.

OK, fixed.

export PASSPHRASE=goodpass
bin/duplicity --name issue147 --no-print -vNOTICE `pwd` file:///tmp/testbackup
export PASSPHRASE=badpass
bin/duplicity --name issue147 --no-print -vNOTICE `pwd` file:///tmp/testbackup

returns

$ testing/manual/issue147.sh
/Users/ken/workspace/duplicity-origin
Local and Remote metadata are synchronized, no sync needed.
Last full backup date: none
No signatures found, switching to full backup.
Local and Remote metadata are synchronized, no sync needed.
Last full backup date: Mon Oct  3 10:47:16 2022
Error processing remote manifest (duplicity-full.20221003T154716Z.manifest.gpg): GPG Failed, see log below:
===== Begin GnuPG log =====
gpg: AES256.CFB encrypted data
gpg: encrypted with 1 passphrase
gpg: decryption failed: Bad session key
===== End GnuPG log =====

I think the most we can do in the future is to take some common problems and provide them with an inline help. Bad session key may be meaningful to us, but not to our users.

OK, it's out except for the snaps. Remote build on LP is having Rust problems. Here's the error:

The package rust-all has unmet dependencies: llvm-14
Build failed
Traceback (most recent call last):
  File "/usr/lib/python3/dist-packages/lpbuildd/target/build_snap.py", line 205, in run
    self.pull()
  File "/usr/lib/python3/dist-packages/lpbuildd/target/build_snap.py", line 167, in pull
    self.run_build_command(
  File "/usr/lib/python3/dist-packages/lpbuildd/target/operation.py", line 46, in run_build_command
    return self.backend.run(args, cwd=cwd, env=full_env, **kwargs)
  File "/usr/lib/python3/dist-packages/lpbuildd/target/lxd.py", line 538, in run
    subprocess.check_call(cmd, **kwargs)
  File "/usr/lib/python3.8/subprocess.py", line 364, in check_call
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['lxc', 'exec', 'lp-focal-amd64', '--env', 'LANG=C.UTF-8', '--env', 'SHELL=/bin/sh', '--env', 'http_proxy=http://10.10.10.1:8222/', '--env', 'https_proxy=http://10.10.10.1:8222/', '--env', 'GIT_PROXY_COMMAND=/usr/local/bin/lpbuildd-git-proxy', '--env', 'SNAPPY_STORE_NO_CDN=1', '--env', 'SNAPCRAFT_LOCAL_SOURCES=1', '--env', 'SNAPCRAFT_SETUP_CORE=1', '--env', 'SNAPCRAFT_BUILD_INFO=1', '--env', 'SNAPCRAFT_IMAGE_INFO={"build-request-id": "lp-74465029", "build-request-timestamp": "2022-10-03T19:53:25Z", "build_url": "https://launchpad.net/~kenneth-loafman/+snap/snapcraft-duplicity-ed26ed8900e683068c17a56c0df05dd7/+build/1900036"}', '--env', 'SNAPCRAFT_BUILD_ENVIRONMENT=host', '--', '/bin/sh', '-c', 'cd /build/snapcraft-duplicity-ed26ed8900e683068c17a56c0df05dd7 && linux64 snapcraft pull']' returned non-zero exit status 2.

want me to have a look at the snaps?

ran a quick trial. fails for all archs. guess they broke the latest rust package currently in repo for Ubuntu20. let's wait a little.

Yep. Tried several times, even tried amd64 on my VM. Same error.

closed with commit b49f1360

https://packages.ubuntu.com/focal/rust-all

says llvm-14 is not avail, which is bad as it is a hard dependency ;(

try commenting rust-all in snapcraft.yaml . it's only needed for the other archs.

Building now. Seems to be working. Thanks!

Spoke too soon. Must be needed during the last part of the build.

amd64 is out, the others are waiting for rust-all to reappear.

I made a few more tries and nothing seems to work, even building llvm-14 from llvm.org directly.

@edeso Since you've had luck building the alt-arch snaps, I'm leaving their builds to you. Don't get too excited! I think snap builds are cursed for non-amd64 arches. I have not had success with them in the last several releases. I may abandon them completely if Ubuntu/LP does not get its act together.

Thank you for the quick fix for all this! I wish gpg was easier to talk to.

Hi all, I'm running duplicity v 1.2.1 (from the release ppa), and I'm still affected by this bug. A wrong password gives a GPG error:

Error processing remote manifest (duplicity-inc.20230106T225005Z.to.20230112T205006Z.manifest.gpg): GPG Failed, see log below:
===== Begin GnuPG log =====
gpg: AES256 encrypted data
gpg: encrypted with 1 passphrase
gpg: decryption failed: Bad session key
===== End GnuPG log =====

And then proceeds with the incremental backup anyway.

reopened

removed bugmedium label

added bughigh label

Thanks for reporting this!

I took a second look and repaired the test case and the duplicity code. Appearances can be deceiving if you expect a test case to pass and it does. You will still get the above error, but duplicity will stop as it should. It'll be out in 1.2.2 soon.

I thank you!

I don't know if it is significant, but I find it strange that duplicity asks twice for the password on an incremental backup (first time, then confirmation). If the encryption password is known to duplicity from an existing target, it should ask once, and if they match, it is fine to proceed. But the fact that it asks for confirmation on an incremental suggests duplicity thinks it could accept an arbitrary password at that point, when it's not really the case.

The code that gets the password is reused all over the place, so it adheres to the standard of ask, repeat, and compare. I don't think changing it for this one case is needed, but I'd welcome a patch if you think it's worth the effort.

The fact itself is fine. I just thought it might be a hint on why duplicity was getting confused in this case. And I'm happy as long as duplicity ultimately does not accept a wrong password on the incremental. :-) Looking forward to 1.2.2!

mentioned in commit 72a373af

closed

mentioned in issue #698 (closed)

mentioned in issue #799 (closed)

Regression: Duplicity will (wrongly) accept a change of encryption password during an incremental backup

I have:

Summary

Environment

Bug: allows change of password mid-incremental with cache in 0.8.23

Correct: prevents change of password mid-incremental without cache in 0.8.23

Correct: tells you it can't restore with wrong password in 0.8.23

Kind-of-correct: prevents change of password mid-incremental with cache in 1.0.0

Bug: allows change of password mid-incremental without cache in 1.0.0

Bug: tells you there are no files in backup when restoring with wrong password in 1.0.0

Designs

Child items ...

Activity