-
Notifications
You must be signed in to change notification settings - Fork 314
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
[bldr] encryption/decryption #171
Conversation
8d814af
to
aa159a1
Compare
728ba9b
to
24c5d60
Compare
I have a test and a fix that I'll commit tomorrow AM. |
if url.is_empty() { | ||
try!(gpg::import(&config.key())); | ||
} | ||
pub fn import(config: &Config) -> BldrResult<()> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If somebody specifies both an --infile
and -u
flag this function will silently prefer the --infile
option. I'd recommend that we make the flags exclusive and notify the user or condense them into a single flag representing "location" where a value starting with http://
or https://
is a remote location and anything else is considered a filesystem location
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nevermind! It looks like that you've covered this in docopt by specifying these two flags with an |
operator :)
@metadave overall things are looking great! I left some comments about the heavy usage of |
4a58a74
to
dd10510
Compare
// prepend BLDR_KEY_PREFIX to the key name, these are the bldr keys | ||
let full_userkey = gen_user_key_name(&userkey); | ||
let full_servicekey = gen_service_key_name(&servicekey, config.group()); | ||
let password = match config.password().clone() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@reset This is the only clone I'm stuck on
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is because when you're peforming your match the binding p
is attempting to take ownership of the value wrapped in the option returned by password()
. You need to use the ref
keyword in the match to step around this:
match config.password() {
Some(ref p) => p,
None => { ... },
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried that, but now p
is a &str
and since I want to return a string from this block, don't I need a String
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think you want a String
back from that block. It looks like you're just passing password
to gpg::encrypt_and_sign()
which takes an &str
value for password
dd10510
to
6d32f35
Compare
This PR has passed 'Verify' and is ready for review and approval! |
@delivery approve |
Merged change 35832acd-e8fe-4656-ba15-7b8398183ae4 From review branch dp_tales_from_the_encrypt into master Signed-off-by: reset <reset@chef.io>
Change: 35832acd-e8fe-4656-ba15-7b8398183ae4 approved by: @reset |
This PR implements or enhances all of these commands*:
Notes:
ENCRYPT_ALWAYS_TRUST
flag.--password
isn't specified toencrypt
, then an echoless password prompt appears. We can talk about other ways to obtain a password in another PR (~/.bldr/config
?)import-key
andexport-key
only do public key IO, we can't export private keys.--infile
and--outfile
.download-repo-key
/upload-repo-key
need to be readdressed in a separate PR.bldr_
prefix to distinguish them from any other key that is imported/exported into the GPG cache. Additionally, GPG has a 4 character minimum on key names, so having a common bldr prefix allows keys with >= 1 character. (Really, I just wanted to have a key namedfoo
).TODO: The only wrinkle here is that specifying a name on the CLI for a key should work with both normalized + non-normalized names (unless there are keys with normalized + non-normalized names that are the same).BLDR_GPG_CACHE
environment variable to point at a different GPG cache. This is primarily used for testing encrypt/decrypt/import/export.TODO:
test Decrypt as userNot possible, I need to be able to export private keysbldr_foo
orfoo
?)