Generate and write public and private key using HPKE. #654

richajaindce · 2023-05-17T02:46:07Z

These will be used to encrypt and decrypt match key shares

martinthomson

I assume that this is only part of the whole. I don't see the use of these keys anywhere.

With that in mind, this is good, but I would avoid using Option<T> for these. It will be easier if we retain a code path where possible.

martinthomson · 2023-05-17T05:34:31Z

src/config.rs

@@ -96,13 +96,16 @@ pub struct PeerConfig {
    /// If a certificate is specified here, only the specified certificate will be accepted. The system
    /// truststore will not be used.
    pub certificate: Option<String>,
+
+    pub matchkey_encryption_key: Option<String>,


This won't be optional in practice, so maybe just make this a String directly.

Sounds good. Although do we need to pass the matchkey public key to the peers?
I think we should also add it to ServerConfig. Will do that

The other peers won't need this, but it probably makes sense to reuse the same configuration (or at least a subset of it) for client configuration.

Yes, I hope ServerConfig is the right place to put it.

I think ServerConfig indeed needs both the private key and the public key (in some formats, the private key is stored with a copy of the public key, but if we're just using raw bytes, that won't be the case).

I also think that the public key should go in NetworkConfig (network.toml). The idea is that network.toml gets distributed to clients of the helper network. test_mpc already takes an argument --network network.toml, and the event generation stuff that @akoshelev is adding to it will soon (possibly immediately, if configuration/use of the report key is mandatory) need to support encrypting the reports.

Within NetworkConfig we have a PeerConfig. Should we add a ClientConfig which will store the public key needed for encryption? There might be other information which would be needed to communicated to clients which can go there. @andyleiserson

I think the public key could go directly in PeerConfig. The only significance that I had in mind for PeerConfig was that it was a container for the configuration that needed to be replicated 3x for the helpers.

Another possibility is to represent the key information in the OHTTP key configuration format mentioned in https://github.com/patcg-individual-drafts/ipa/blob/main/details/encryption.md#helper-key-configuration, but that doesn't seem important to do right now, and even using that format I think we could nest it within PeerConfig.

There are a few things in PeerConfig (e.g. port) which advertisers/publishers don't care about. I will move public_key inside PeerConfig and then we can see how it evolves. The other configuration stuff will be added as we implement decryption

src/config.rs

…behind test-fixture feature

andyleiserson · 2023-05-17T20:48:03Z

With that in mind, this is good, but I would avoid using Option for these. It will be easier if we retain a code path where possible.

I agree that these aren't really optional, in the long term. There is a question of whether we want to support bypassing the match key encryption for some kinds of testing / protocol development work, like how we support HTTP for testing.

Another issue that may come up is how the encryption interacts with the choice between real-world-infra and in-memory-infra. There's no reason that match keys can't be encrypted in both cases, but a couple reasons we might not want that:

It may be useful to skip match key encryption for some kinds of MPC protocol development work where it isn't relevant. (Either for ease of interpreting the test data, or to reduce the number of build dependencies.)
network.toml and ServerConfig (which should soon be server.toml) are currently only used for the real-world-infra. If we do want to use match key encryption with in-memory-infra, that may require some more tinkering with the test setup stuff.

richajaindce · 2023-05-17T23:51:24Z

With that in mind, this is good, but I would avoid using Option for these. It will be easier if we retain a code path where possible.

I agree that these aren't really optional, in the long term. There is a question of whether we want to support bypassing the match key encryption for some kinds of testing / protocol development work, like how we support HTTP for testing.

Another issue that may come up is how the encryption interacts with the choice between real-world-infra and in-memory-infra. There's no reason that match keys can't be encrypted in both cases, but a couple reasons we might not want that:

It may be useful to skip match key encryption for some kinds of MPC protocol development work where it isn't relevant. (Either for ease of interpreting the test data, or to reduce the number of build dependencies.)

network.toml and ServerConfig (which should soon be server.toml) are currently only used for the real-world-infra. If we do want to use match key encryption with in-memory-infra, that may require some more tinkering with the test setup stuff.

I think we should support bypassing encryption for now.

richajaindce · 2023-05-17T23:53:13Z

src/config.rs

@@ -167,24 +183,37 @@ pub struct ServerConfig {

    /// TLS configuration for helper-to-helper communication
    pub tls: Option<TlsConfig>,
+
+    /// Configuration needed for encrypting and decrypting match keys
+    pub matchkey_encryption_info: Option<MatchKeyEncryptionConfig>,


@andyleiserson I have stored public and private keys as optional as MatchKeyEncryptionConfig in ServerConfig. Lets leave it optional for now to support disabling encryption of match keys

Added a flag in net/test.rs for disable_matchkey_encryption but not adding any test for that as yet. Will add it along with coding decryption handling of the reports

andyleiserson · 2023-05-24T21:40:10Z

I resolved the merge conflicts with #653 and #647, you can pull the resolved version from the report_key branch in my fork. I'm not sure how to get a concise rendering of a merge commit out of github, but if you do git show, the lines marked with ++ are the ones that were introduced or changed by my conflict resolutions.

richajaindce · 2023-05-25T00:03:37Z

I resolved the merge conflicts with #653 and #647, you can pull the resolved version from the report_key branch in my fork. I'm not sure how to get a concise rendering of a merge commit out of github, but if you do git show, the lines marked with ++ are the ones that were introduced or changed by my conflict resolutions.

Thanks a lot for that! Will take a look

* HPKE module encapsulates the actual encryption type we use and it should not be visible to the other modules. So now it exposes `KeyPair` type and serialization methods on it. That allows keygen not to worry about the actual encryption type used. * Rename HPKE encryption parameters to make it a bit easier to type and consistent with TLS arguments naming This is how `network.toml` looks like ```toml [client.http_config] version = "http2" [[peers]] certificate = """ -----BEGIN CERTIFICATE----- MIIBZTCCAQugAwIBAgIIbSvJ07OOLJwwCgYIKoZIzj0EAwIwFDESMBAGA1UEAwwJ bG9jYWxob3N0MB4XDTIzMDUyNDE4MzMyOVoXDTIzMDgyMzE4MzMyOVowFDESMBAG A1UEAwwJbG9jYWxob3N0MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAE1hGZwFS6 gMHLsyzKK6voi4sNCMuhyAUkeib8PgDE8Sw1r5hAkuOd582gXaO3hske2NAbx1DB PFNX64l06N2vHqNHMEUwFAYDVR0RBA0wC4IJbG9jYWxob3N0MA4GA1UdDwEB/wQE AwICpDAdBgNVHSUEFjAUBggrBgEFBQcDAQYIKwYBBQUHAwIwCgYIKoZIzj0EAwID SAAwRQIgJzHHbPDNcyA0qeTvX3k6PB7Ecapr4xI8zkn4T/Njyb8CIQDA+IB5QBgI JlilqZIGGNpVqMXBgVK/KdaxurK/NqgJsA== -----END CERTIFICATE----- """ url = "localhost:51126" [peers.hpke] public_key = "5c0376ff2746767deb791219c0853325b49301081a009bcc133582ae8ee24e22" ```

akoshelev · 2023-05-25T22:43:59Z

@richajaindce I made some small changes, lmk what you think, if you are ok with these, we probably can push it

richajaindce · 2023-05-26T04:21:06Z

@richajaindce I made some small changes, lmk what you think, if you are ok with these, we probably can push it

The changes look great!! Thanks for them. Lets ship it!

Generate and write public and private key using HPKE.

991e6b8

These will be used to encrypt and decrypt match key shares

martinthomson reviewed May 17, 2023

View reviewed changes

Adding MatchKeyEncryptionConfig in ServerConfig

8bdcd1d

richajaindce commented May 17, 2023

View reviewed changes

src/config.rs Outdated Show resolved Hide resolved

Hiding TEST_MATCHKEY_ENCRYPTION_KEY and TEST_MATCHKEY_DECRYPTION_KEY …

b5f48ca

…behind test-fixture feature

richajaindce force-pushed the report_key branch from 79e93db to b5f48ca Compare May 17, 2023 18:07

richajaindce commented May 17, 2023

View reviewed changes

richajaindce added 2 commits May 18, 2023 13:42

Storing encryption key in ClientConfig within ServerConfig

f29e376

Moving match key encryption public key inside PeerConfig

d3f8e61

richajaindce mentioned this pull request May 19, 2023

Event generation using test_mpc and IPA integration test + migrate to HTTP2 #647

Merged

andyleiserson mentioned this pull request May 24, 2023

Client cert authentication for inter-helper communication #653

Merged

richajaindce and others added 3 commits May 25, 2023 10:12

Merge branch 'main' into report_key

b79ba71

Update setup_instructions.md

4c1ffa5

akoshelev merged commit 5221bf8 into private-attribution:main May 26, 2023

richajaindce deleted the report_key branch July 21, 2023 04:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Generate and write public and private key using HPKE. #654

Generate and write public and private key using HPKE. #654

richajaindce commented May 17, 2023

martinthomson left a comment

martinthomson May 17, 2023

richajaindce May 17, 2023

martinthomson May 17, 2023

richajaindce May 17, 2023

andyleiserson May 17, 2023

richajaindce May 17, 2023

andyleiserson May 18, 2023

richajaindce May 19, 2023 •

edited

Loading

andyleiserson commented May 17, 2023

richajaindce commented May 17, 2023

richajaindce May 17, 2023

richajaindce May 18, 2023

andyleiserson commented May 24, 2023

richajaindce commented May 25, 2023

akoshelev commented May 25, 2023

richajaindce commented May 26, 2023

Generate and write public and private key using HPKE. #654

Generate and write public and private key using HPKE. #654

Conversation

richajaindce commented May 17, 2023

martinthomson left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

richajaindce May 19, 2023 • edited Loading

Choose a reason for hiding this comment

andyleiserson commented May 17, 2023

richajaindce commented May 17, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andyleiserson commented May 24, 2023

richajaindce commented May 25, 2023

akoshelev commented May 25, 2023

richajaindce commented May 26, 2023

richajaindce May 19, 2023 •

edited

Loading