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 encrypted file upload to gossip ring #452

Merged
merged 1 commit into from
Apr 29, 2016
Merged

Conversation

bookshelfdave
Copy link
Contributor

@bookshelfdave bookshelfdave commented Apr 27, 2016

This PR adds the hab file upload subcommand. In order for this command to work, you need a user key and a service key.

To encrypt, hab upload needs to be able to find the service public key and the user secret key in /hab/cache/keys (or override with HAB_CACHE_KEY_PATH). To decrypt a message, the supervisor needs a service secret key and a user public key.

If the supervisor receives the encrypted file via gossip, but is not the intended recipient, the file remains encrypted in memory. If the supervisor is the recipient and has service secret key + user public key, then the decrypted file will be written out to disk in the running service directory (ex: /hab/svc/redis/files/foo). If a service is the recipient but does not have the appropriate keys to decrypt, the GossipFileList will retry the write with exponential backoff.

Gossip file write failures are available in the sidecar:

root@d0b0b2323cca:/src/components/hab# curl http://localhost:9631/gossip | jq .file_write_retries
{
  "foo": {
    "file_name": "foo",
    "last_failure_reason": "Crypto error: Cannot find user key dparfitt-20160426212017",
    "total_retries": 4
  }
}
  • HAB_ORG is honored if you don't specify --org
  • ServiceGroup now has an organization component, and it's from_str can parse strings in "foo.bar" and "foo.bar@baz" format
  • ConfigFile renamed to GossipFile
  • ConfigFileList renamed to GossipFileList
  • maximum file upload size is 4096 bytes
  • remove dead key tests in sup

To play around with this feature:

hab user key generate someuser
hab service key generate redis.default xyz

Open 3 terminals:
/src/components/sup/target/debug/hab-sup start core/redis --permanent-peer  --topology leader --group default --org xyz
/src/components/sup/target/debug/hab-sup start core/redis --permanent-peer --peer 172.17.0.2:9634 --topology leader --group default --org xyz
/src/components/sup/target/debug/hab-sup start core/redis --permanent-peer --peer 172.17.0.2:9634 --topology leader --group default --org xyz
(note, you may need to change the `--peer` IP address, which you can find by running `ip a`)

date > foo
./target/debug/hab file upload redis.default ./foo 1 someuser  --org xyz

# on a supervisor node, you can cat the file:
cat /hab/svc/redis/files/foo

you can make this fail on the sup side by setting HAB_CACHE_KEY_PATH to a directory without keys:

export HAB_CACHE_KEY_PATH=/some/dir

and then uploading a new file. Check for failures with:

curl http://localhost:9631/gossip | jq .file_write_retries

and resolve the error by resetting:

export HAB_CACHE_KEY_PATH=/hab/cache/keys

TODO:

  • receiving an encrypted payload without having appropriate keys to decrypt results in sup crash/restart. Slack discussion
    • bubble up severe error to sidecar
    • don't crash the supervisor and log an error
  • more testing: GossipFile + GossipFileList definitely need more testing, but that will have to come in a later PR to make it easier to mock fs:svc_path*
  • if we want "destination path" for an uploaded file, I'd like to do that in a separate PR Slack discussion

@thesentinels
Copy link
Contributor

By analyzing the blame information on this pull request, we identified @fnichol, @adamhjk, @reset and @juliandunn to be potential reviewers

@bookshelfdave bookshelfdave changed the title [WIP] Add encrypted file upload to gossip ring Add encrypted file upload to gossip ring Apr 29, 2016
@bookshelfdave
Copy link
Contributor Author

@habitat-sh/habitat-core-maintainers this is ready for review

@adamhjk
Copy link
Contributor

adamhjk commented Apr 29, 2016

@metadave it's worth squashing these three commits down into one, since the feature all goes together.

.iter()
.any(|(&(ref sg, _), ref cf)| sg == &self.my_service_group && cf.written == false)
}
/// Write the files out to disk. We currently are a bit badly factored here - we have both the
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need a little whitespace here.

@adamhjk
Copy link
Contributor

adamhjk commented Apr 29, 2016

Other than squashing the commits and adding one line of whitespace this looks great. The tests in particular are great. Nice work!

gif-keyboard-18349095133980737447

@bookshelfdave
Copy link
Contributor Author

I'll squash and rebase against master

ping @davidwrede

@thesentinels
Copy link
Contributor

☔ The latest upstream changes (presumably c86dfab) made this pull request unmergeable. Please resolve the merge conflicts.

- ServiceGroup now has an organization component, and it's from_str
  can parse strings in "foo.bar" and "foo.bar@baz" format
- ConfigFile renamed to GossipFile
- ConfigFileList renamed to GossipFileList
- remove dead key tests in sup
- add retry + exponential backoff for decrypt file writes
- add FileWriteRetries to /gossip sidecar

Signed-off-by: Dave Parfitt <dparfitt@chef.io>
@bookshelfdave
Copy link
Contributor Author

@thesentinels r+

@thesentinels
Copy link
Contributor

📌 Commit b9676d9 has been approved by metadave

@thesentinels
Copy link
Contributor

⌛ Testing commit b9676d9 with merge 53f1526...

thesentinels pushed a commit that referenced this pull request Apr 29, 2016
- ServiceGroup now has an organization component, and it's from_str
  can parse strings in "foo.bar" and "foo.bar@baz" format
- ConfigFile renamed to GossipFile
- ConfigFileList renamed to GossipFileList
- remove dead key tests in sup
- add retry + exponential backoff for decrypt file writes
- add FileWriteRetries to /gossip sidecar

Signed-off-by: Dave Parfitt <dparfitt@chef.io>

Pull request: #452
Approved by: metadave
@thesentinels
Copy link
Contributor

☀️ Test successful - travis

@thesentinels thesentinels merged commit b9676d9 into master Apr 29, 2016
smith pushed a commit that referenced this pull request May 2, 2016
- ServiceGroup now has an organization component, and it's from_str
  can parse strings in "foo.bar" and "foo.bar@baz" format
- ConfigFile renamed to GossipFile
- ConfigFileList renamed to GossipFileList
- remove dead key tests in sup
- add retry + exponential backoff for decrypt file writes
- add FileWriteRetries to /gossip sidecar

Signed-off-by: Dave Parfitt <dparfitt@chef.io>

Pull request: #452
Approved by: metadave
jtimberman pushed a commit that referenced this pull request Jun 12, 2016
- ServiceGroup now has an organization component, and it's from_str
  can parse strings in "foo.bar" and "foo.bar@baz" format
- ConfigFile renamed to GossipFile
- ConfigFileList renamed to GossipFileList
- remove dead key tests in sup
- add retry + exponential backoff for decrypt file writes
- add FileWriteRetries to /gossip sidecar

Signed-off-by: Dave Parfitt <dparfitt@chef.io>

Pull request: #452
Approved by: metadave
@reset reset deleted the dp_tales_from_decrypt branch November 26, 2016 22:21
raskchanky pushed a commit that referenced this pull request Apr 16, 2019
- ServiceGroup now has an organization component, and it's from_str
  can parse strings in "foo.bar" and "foo.bar@baz" format
- ConfigFile renamed to GossipFile
- ConfigFileList renamed to GossipFileList
- remove dead key tests in sup
- add retry + exponential backoff for decrypt file writes
- add FileWriteRetries to /gossip sidecar

Signed-off-by: Dave Parfitt <dparfitt@chef.io>

Pull request: #452
Approved by: metadave
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants