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.

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

Add experimental crypto backend #645

Merged
merged 1 commit into from
Feb 20, 2018
Merged

Conversation

dominikschulz
Copy link
Member

@dominikschulz dominikschulz commented Jan 29, 2018

Experimental Crypto Backend for gopass

This PR contains an experimental crypto backend for gopass.
The goal is to provide an implementation that is feature complete
compared to the GPG backend but doesn't require any external binaries,
especially no GPG. Of course this would break compatilibity to existing
GPG deployments and users of different pass implementations, but
especially for closed teams with no existing GPG deployment this should
make little different.

Motivation

While GPG is believed to be very secure and it supports a wide range of
applications and devices, it's not really user friendly. Even passioned
crypto experts don't enjoy working with GPG and for
newcomers it's a major hurdle. For the gopass developers it's about the
most time consuming task to provide support and implement workaround for
GPG issues. This doesn't mean that GPG is bad, but security is hard and
complex and GPG adds a lot of flexiblity on top of that so the result
is complex and complicated.

Status

Working, needs more testing.

Design

@dominikschulz dominikschulz added this to the 2.0.0 - Break free milestone Jan 29, 2018
@metalmatze
Copy link
Contributor

metalmatze commented Jan 29, 2018

Interesting.
For everyone else looking at this PR, I found this document in the PR describing a bit more what's going on:
https://github.com/justwatchcom/gopass/pull/645/files#diff-4dfeb3555efa9f95ff8378cac2ce7ddd

@codecov
Copy link

codecov bot commented Jan 31, 2018

Codecov Report

Merging #645 into master will increase coverage by 5.67%.
The diff coverage is 60.81%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #645      +/-   ##
=========================================
+ Coverage   58.83%   64.5%   +5.67%     
=========================================
  Files         126     142      +16     
  Lines        7231    7950     +719     
=========================================
+ Hits         4254    5128     +874     
+ Misses       2384    2202     -182     
- Partials      593     620      +27
Impacted Files Coverage Δ
action/templates.go 36.36% <ø> (+1.58%) ⬆️
action/otp.go 69.35% <ø> (ø) ⬆️
backend/crypto/gpg/cli/version.go 70.83% <ø> (-3.59%) ⬇️
store/root/git.go 0% <0%> (-14.58%) ⬇️
utils/pinentry/pinentry_others.go 0% <0%> (ø)
utils/pinentry/pinentry.go 0% <0%> (ø)
backend/crypto/gpg/cli/generate.go 0% <0%> (ø)
store/root/gpg.go 0% <0%> (-100%) ⬇️
backend/crypto/gpg/cli/import.go 0% <0%> (ø)
backend/sync/git/mock/git.go 100% <100%> (+20%) ⬆️
... and 96 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0cb08ac...c95d9dd. Read the comment docs.

@dominikschulz dominikschulz changed the title WIP: Add experimental crypto backend Add experimental crypto backend Jan 31, 2018
@falschparker82
Copy link
Contributor

Wow, that's a large one, don't know if I've got time to review it in full. Crypto primitives choice and design looks solid to me at first sight. Use of gob is a sane choice, but slightly disadvantegeous IMHO when interacting with other languages. Also I understand the secret format is now binary? That might make it a little harder interacting with a tool made for text files like git.

@dominikschulz
Copy link
Member Author

Thank you for the input so far. I'll definitely have another look at the on disk format. The current approach is neither very interoperable nor does it properly solve the backward/forward compatibility.

@dominikschulz
Copy link
Member Author

I've changed to on-disk format to use protobuf (version 3). This makes the seralization a lot cleaner, but still the whole branch needs a lot of testing.

@dominikschulz
Copy link
Member Author

The code should be mostly fine now, but I still need to do some testing wrt. the new backend as well as the refactored old backends.

@dominikschulz
Copy link
Member Author

This PR should be OK to merge. There may be some issues due to the huge refactorings, but we'll fix them when we encounter them. At least the stuff that's unit and integration tested (and that's quite a lot by now) seems to work well.

@dominikschulz dominikschulz modified the milestones: 2.0.0 - Break free, 1.7.0 - Experimental Crypto Feb 20, 2018
This commit adds an experimental crypto backend. It comes with it's own keyring
as well as an agent. The crypto is based on NaCl and Argon2.

The on-disk format uses protobuf version 3.

Fixes gopasspw#154
@dominikschulz dominikschulz merged commit 5437118 into gopasspw:master Feb 20, 2018
@dominikschulz dominikschulz deleted the exp/xc branch February 20, 2018 11:11
kpitt pushed a commit to kpitt/gopass that referenced this pull request Jul 21, 2022
This commit adds an experimental crypto backend. It comes with it's own keyring
as well as an agent. The crypto is based on NaCl and Argon2.

The on-disk format uses protobuf version 3.

Fixes gopasspw#154
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants