Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Implement cryptographic access to files using GPGME #1949

Merged
merged 2 commits into from
Oct 30, 2020
Merged

Implement cryptographic access to files using GPGME #1949

merged 2 commits into from
Oct 30, 2020

Conversation

spacefrogg
Copy link
Contributor

@spacefrogg spacefrogg commented Jul 28, 2020 •
edited
Loading

Goal

Access encrypted files (given to ledger as filenames) directly from ledger via GPGME++ wrapper for GPGME.

Current State

  • Implemented file access as a std::istream class (same as files are currently used) named decrypted_stream_t
    • Maintain the same interface to files as before
    • Return a std::ifstream if the input is unencrypted
  • Encapsulate communication with GPG in data_streambuffer_t a std::streambuf class:
    • Initialize GPGME library (i.e., works with zero knowledge from the view of decrypted_stream_t users)
    • Abstract std::FILE into a seekable streambuffer
    • Determine encryption method and establish encryption context (works transparently with unencrypted files)
    • Decrypt en bloc at creation time (GPG does not employ stream ciphers, so the whole content has to be decrypted upfront)
  • Implement readout of decrypted content as std::streambuf backing store data_streambuffer_t
    • Read out data from GPGME while the stream is processed
  • Complete decrypted content is held in memory (by GPGME)
  • Support for PGP (symmetric, asymmetric) and CMS (S/MIME) encrypted files
  • No signature verification
  • Rough performance evaluation:
    • On an Intel i5 7xxx, a 46 MB file takes about 3 seconds to decrypt (without disk I/O)

Current possible deficiencies

  • Even when a file is assumed to be unencrypted, it is still handled by GPGME. This might incur a performance penalty.

Missing

  • Tests (not working on NixOS due to linking errors in MathTests and UtilTests)
  • Proper performance evaluation

@spacefrogg spacefrogg marked this pull request as draft July 28, 2020 21:11
@spacefrogg spacefrogg force-pushed the libgpg branch 10 times, most recently from c5dde61 to a1ec1bf Compare July 29, 2020 08:37
@spacefrogg spacefrogg changed the title WIP: Implement cryptographic access to files using GPGME Implement cryptographic access to files using GPGME Sep 17, 2020
@spacefrogg spacefrogg marked this pull request as ready for review September 17, 2020 21:06
@spacefrogg
Copy link
Contributor Author

This one is now good to go from my side.

@tbm
Copy link
Contributor

tbm commented Oct 30, 2020

@spacefrogg thanks for submitting this. It needs to be reviewed by @jwiegley

@tbm
Copy link
Contributor

tbm commented Oct 30, 2020

I think the "default.nix: Fix testing" commit should be removed since it is unrelated.

If default.nix is broken, can you open a separate PR (the commit should describe what you're fixing).

@tbm
Copy link
Contributor

tbm commented Oct 30, 2020

is the export LD_LIBRARY_PATH=$PWD still needed? I recall a fix for that (or maybe it was only a conversation without a fix)

@tbm
Copy link
Contributor

tbm commented Oct 30, 2020 •
edited
Loading

is the export LD_LIBRARY_PATH=$PWD still needed? I recall a fix for that (or maybe it was only a conversation without a fix)

I think it might be related to issue #1721

@tbm tbm requested a review from jwiegley October 30, 2020 09:32
@spacefrogg
Copy link
Contributor Author

is the export LD_LIBRARY_PATH=$PWD still needed? I recall a fix for that (or maybe it was only a conversation without a fix)

Yes, it is needed in this circumstance and is only partially related #1721. The RPATH is set during build time to point to the final location of the library. During testing, though, the library is not there, yet. So, ledger won't find its own libledger.so during testing.

As some modifications to default.nix are related to this PR (adding dependencies and build flags). Should I keep the fixes in this PR, as well, or move them to a separate PR?

@spacefrogg
Copy link
Contributor Author

I see, you've already answered my question. I will make a separate PR.

@tbm
Copy link
Contributor

tbm commented Oct 30, 2020

As some modifications to default.nix are related to this PR (adding dependencies and build flags). Should I keep the fixes in this PR, as well, or move them to a separate PR?

Well, I was talking about the second commit, which looks unrelated. But if it's related, leave it here.

@tbm
Copy link
Contributor

tbm commented Oct 30, 2020

(I know you also modified default.nix as part of the GPGME change, but that was obviously related and should be part of the first commit)

@jwiegley jwiegley merged commit f4b16ef into ledger:master Oct 30, 2020
@jwiegley
Copy link
Member

Looks like great work to me, thank you!

@Rahix
Copy link
Contributor

Rahix commented Aug 4, 2021

I cannot find any documentation or examples on how this feature is meant to be used. Am I correct to assume that ledger will now just automatically call into GPGME when it detects an encrypted file and decrypt it transparently? I.e. there is no additional configuration necessary, it just works (TM)?

@spacefrogg
Copy link
Contributor Author

Yes, it will just work. You could name your ledger ledger.gpg or so, for your own reference, but it won't matter for en-/decryption.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jwiegley jwiegley Awaiting requested review from jwiegley

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@spacefrogg @tbm @jwiegley @Rahix