Page MenuHomePhabricator

Allow Conduit requests to be signed with a public/private keypair
ClosedPublic

Authored by epriestley on Sep 2 2014, 10:49 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Nov 24, 5:38 AM
Unknown Object (File)
Fri, Nov 22, 2:50 PM
Unknown Object (File)
Fri, Nov 22, 9:05 AM
Unknown Object (File)
Sun, Nov 17, 5:12 AM
Unknown Object (File)
Fri, Nov 15, 7:16 AM
Unknown Object (File)
Fri, Nov 1, 6:11 PM
Unknown Object (File)
Thu, Oct 31, 1:35 AM
Unknown Object (File)
Oct 24 2024, 9:28 AM
Subscribers

Details

Summary

This allows callers (in the future, servers in a cluster or instances) to sign Conduit requests with an asymmetric keypair instead of a certificate or token.

Overall we could get away without this, but it seems worth doing for a few reasons:

  • By binding Device identity to SSH keys, we can also authorize them over (real) SSH easily, and not need separate conduit / SSH keys.
  • Asymmetric key cryptography is strong and well understood, and we never have to share or transmit private keys.
  • This is potentially useful to third parties for device identity, in a way that custom Conduit stuff wouldn't be.
Test Plan
  • Added unit tests.
  • Will actually test once I mess with the other half of this.

Diff Detail

Repository
rPHU libphutil
Branch
D10402
Lint
Lint Passed
SeverityLocationCodeMessage
Auto-Fixsrc/conduit/ConduitClient.php:46TXT6Trailing Whitespace
Auto-Fixsrc/conduit/ConduitClient.php:51TXT6Trailing Whitespace
Auto-Fixsrc/conduit/ConduitClient.php:73TXT6Trailing Whitespace
Auto-Fixsrc/conduit/ConduitClient.php:79TXT6Trailing Whitespace
Auto-Fixsrc/conduit/ConduitClient.php:143TXT6Trailing Whitespace
Auto-Fixsrc/conduit/ConduitClient.php:156TXT6Trailing Whitespace
Advicesrc/conduit/ConduitClient.php:161XHP48Array Separator
Unit
No Test Coverage
Build Status
Buildable 2733
Build 2737: [Placeholder Plan] Wait for 30 Seconds

Event Timeline

hach-que retitled this revision from to Implement setServerSignature on ConduitClient for cross-server calls.
hach-que updated this object.
hach-que edited the test plan for this revision. (Show Details)
hach-que added a reviewer: epriestley.
hach-que edited edge metadata.

This looks fine, except that all of this stuff should presumably be signed -- maybe signing needs to move into the client?

hach-que edited edge metadata.

Update to perform signing / verification in libphutil

Remove unnecessary signature field

epriestley edited edge metadata.

We need to shore up the actual signature algorithm at some point, but this is seems reasonable for now. Thanks!

This revision is now accepted and ready to land.Oct 3 2014, 2:17 PM
hach-que edited edge metadata.

Fix parameter and exclude stuff during getEncodedParameters

epriestley edited reviewers, added: hach-que; removed: epriestley.

I'm going to yank this and maybe go halfway toward the longer term in T5955.

This revision now requires review to proceed.Nov 12 2014, 8:37 PM
  • Guessing at how to mostly future-proof this without over-engineering too much.
  • Notes in a sec.
epriestley retitled this revision from Implement setServerSignature on ConduitClient for cross-server calls to Allow Conduit requests to be signed with a public/private keypair.Nov 12 2014, 9:59 PM
epriestley updated this object.
epriestley edited the test plan for this revision. (Show Details)
epriestley added a reviewer: btrahan.

This is mostly a judgement call on what the simplest thing we can get away with for now while still doing SSH key auth is. This is slightly more formal than the original implementation was because I think a couple of pieces of it weren't quite heavyweight enough to survive to SAAS. I think this is about the least we can get away with.

The possibly-overengineered part here is the "Consign" signature algorithm. There's some discussion of this and reasoning in D10401. The previous version of this just signed JSON. I picked this by balancing these concerns:

  • Cons
    • Custom algorithm that I might have screwed up.
    • Third parties will have to implement this if they want to do SSH signing (but we don't expect anyone to use SSH keys for now).
  • Pros
    • Works for binary data, which signing JSON doesn't.
    • Signature is indepenent of key order in dictionaries, which signing JSON isn't.
    • Works for nested dictionaries, which other algorithms like the AWS algorithm don't. I think allowing nested objects/lists in the API is generally good (e.g., apply a list of transaction dictionaries).
    • Relatively small signature algorithm (e.g., no dependency on msgpack or BSON).
    • Signature input is human-readable, which signing msgpack or something like that isn't.
    • If this is stupid, the "Consign1.0/" prefix lets us unstupid it later.

Overall doing a custom thing seems not-that-terrible so I went with it. It has a unit test! This algorithm is structurally similar to serialization schemes like msgpack.

I also made host + port part of the signature.

btrahan edited edge metadata.

I mean, duh. =P I dig the test coverage / future test coverage to come if / when bugs emerge.

This revision is now accepted and ready to land.Nov 12 2014, 10:52 PM
epriestley edited edge metadata.
  • Use PhutilErrorTrap to produce more informative signing failures.
  • Accept public key in PCKS8 format. This key handling code will probably move into libphutil eventually.
  • Make host part of the request explicitly so we can raise more useful errors on the far side.
This revision was automatically updated to reflect the committed changes.