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.

Sign up for GitHub

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

Jump to bottom

Refactor CBD API #231

Merged
merged 9 commits into from
Aug 2, 2023
Merged

Refactor CBD API #231

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
b3d2f90
simplify cohort-strategy relationship
piotr-roslaniec Jun 28, 2023
fcd7fc0
feat!: replaced configuration with raw porter uri
piotr-roslaniec Jun 29, 2023
d911481
feat!: porter uris are based on networks, not chain ids
piotr-roslaniec Jun 29, 2023
f975994
feat! porter is not a character; renamed to porter client
piotr-roslaniec Jun 29, 2023
a2c1677
feat! remove porter uri and web3 provider from character constructors
piotr-roslaniec Jun 29, 2023
69f474f
apply pr suggestions
piotr-roslaniec Jun 29, 2023
64db30f
refactor: simplify equals method in protocol objects
piotr-roslaniec Jun 30, 2023
6d513bb
apply pr suggestions
piotr-roslaniec Jul 5, 2023
300696b
fix after rebase
piotr-roslaniec Aug 2, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 20 additions & 23 deletions src/characters/alice.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,29 +6,22 @@ import {
} from '@nucypher/nucypher-core';
import { ethers } from 'ethers';

import { Configuration } from '../config';
import { Keyring } from '../keyring';
import {
BlockchainPolicy,
BlockchainPolicyParameters,
EnactedPolicy,
PreEnactedPolicy,
} from '../policies/policy';
import { PorterClient } from '../porter';
import { ChecksumAddress } from '../types';

import { RemoteBob } from './bob';
import { Porter } from './porter';

export class Alice {
private readonly porter: Porter;
private readonly keyring: Keyring;

private constructor(
config: Configuration,
secretKey: SecretKey,
public readonly web3Provider: ethers.providers.Web3Provider
) {
this.porter = new Porter(config.porterUri);
private constructor(secretKey: SecretKey) {
this.keyring = new Keyring(secretKey);
}

Expand All @@ -40,43 +33,45 @@ export class Alice {
return this.keyring.signer;
}

public static fromSecretKey(
config: Configuration,
secretKey: SecretKey,
web3Provider: ethers.providers.Web3Provider
): Alice {
return new Alice(config, secretKey, web3Provider);
public static fromSecretKey(secretKey: SecretKey): Alice {
return new Alice(secretKey);
}

public getPolicyEncryptingKeyFromLabel(label: string): PublicKey {
return this.keyring.getPublicKeyFromLabel(label);
}

public async grant(
web3Provider: ethers.providers.Web3Provider,
porterUri: string,
policyParameters: BlockchainPolicyParameters,
includeUrsulas?: readonly ChecksumAddress[],
excludeUrsulas?: readonly ChecksumAddress[]
): Promise<EnactedPolicy> {
const ursulas = await this.porter.getUrsulas(
const porter = new PorterClient(porterUri);
const ursulas = await porter.getUrsulas(
policyParameters.shares,
excludeUrsulas,
includeUrsulas
);
const policy = await this.createPolicy(policyParameters);
return await policy.enact(ursulas);
const policy = await this.createPolicy(web3Provider, policyParameters);
return await policy.enact(web3Provider, ursulas);
}

public async generatePreEnactedPolicy(
web3Provider: ethers.providers.Web3Provider,
porterUri: string,
policyParameters: BlockchainPolicyParameters,
includeUrsulas?: readonly ChecksumAddress[],
excludeUrsulas?: readonly ChecksumAddress[]
): Promise<PreEnactedPolicy> {
const ursulas = await this.porter.getUrsulas(
const porter = new PorterClient(porterUri);
const ursulas = await porter.getUrsulas(
policyParameters.shares,
excludeUrsulas,
includeUrsulas
);
const policy = await this.createPolicy(policyParameters);
const policy = await this.createPolicy(web3Provider, policyParameters);
return await policy.generatePreEnactedPolicy(ursulas);
}

Expand All @@ -99,10 +94,11 @@ export class Alice {
}

private async createPolicy(
web3Provider: ethers.providers.Web3Provider,
rawParameters: BlockchainPolicyParameters
): Promise<BlockchainPolicy> {
const { bob, label, threshold, shares, startDate, endDate } =
await this.validatePolicyParameters(rawParameters);
await this.validatePolicyParameters(web3Provider, rawParameters);
const { delegatingKey, verifiedKFrags } = this.generateKFrags(
bob,
label,
Expand All @@ -123,6 +119,7 @@ export class Alice {
}

private async validatePolicyParameters(
web3Provider: ethers.providers.Web3Provider,
rawParams: BlockchainPolicyParameters
): Promise<BlockchainPolicyParameters> {
const startDate = rawParams.startDate ?? new Date();
Expand All @@ -144,8 +141,8 @@ export class Alice {
);
}

const blockNumber = await this.web3Provider.getBlockNumber();
const block = await this.web3Provider.getBlock(blockNumber);
const blockNumber = await web3Provider.getBlockNumber();
const block = await web3Provider.getBlock(blockNumber);
const blockTime = new Date(block.timestamp * 1000);
if (endDate < blockTime) {
throw new Error(
Expand Down
21 changes: 9 additions & 12 deletions src/characters/bob.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,12 @@ import {
Signer,
} from '@nucypher/nucypher-core';

import { Configuration } from '../config';
import { Keyring } from '../keyring';
import { PolicyMessageKit } from '../kits/message';
import { RetrievalResult } from '../kits/retrieval';
import { PorterClient } from '../porter';
import { zip } from '../utils';

import { Porter } from './porter';

export class RemoteBob {
private constructor(
public readonly decryptingKey: PublicKey,
Expand All @@ -37,11 +35,9 @@ export class RemoteBob {
}

export class Bob {
private readonly porter: Porter;
private readonly keyring: Keyring;

constructor(config: Configuration, secretKey: SecretKey) {
this.porter = new Porter(config.porterUri);
constructor(secretKey: SecretKey) {
this.keyring = new Keyring(secretKey);
}

Expand All @@ -57,24 +53,23 @@ export class Bob {
return this.keyring.signer;
}

public static fromSecretKey(
config: Configuration,
secretKey: SecretKey
): Bob {
return new Bob(config, secretKey);
public static fromSecretKey(secretKey: SecretKey): Bob {
return new Bob(secretKey);
}

public decrypt(messageKit: MessageKit | PolicyMessageKit): Uint8Array {
return this.keyring.decrypt(messageKit);
}

public async retrieveAndDecrypt(
porterUri: string,
policyEncryptingKey: PublicKey,
publisherVerifyingKey: PublicKey,
messageKits: readonly MessageKit[],
encryptedTreasureMap: EncryptedTreasureMap
): Promise<readonly Uint8Array[]> {
const policyMessageKits = await this.retrieve(
porterUri,
policyEncryptingKey,
publisherVerifyingKey,
messageKits,
Expand Down Expand Up @@ -103,6 +98,7 @@ export class Bob {
}

public async retrieve(
porterUri: string,
policyEncryptingKey: PublicKey,
publisherVerifyingKey: PublicKey,
messageKits: readonly MessageKit[],
Expand All @@ -122,7 +118,8 @@ export class Bob {
);

const retrievalKits = policyMessageKits.map((pk) => pk.asRetrievalKit());
const retrieveCFragsResponses = await this.porter.retrieveCFrags(
const porter = new PorterClient(porterUri);
const retrieveCFragsResponses = await porter.retrieveCFrags(
treasureMap,
retrievalKits,
publisherVerifyingKey,
Expand Down
27 changes: 15 additions & 12 deletions src/characters/cbd-recipient.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,28 +25,27 @@ import {
getCombineDecryptionSharesFunction,
getVariantClass,
} from '../dkg';
import { PorterClient } from '../porter';
import { fromJSON, toJSON } from '../utils';

import { Porter } from './porter';

export type CbdTDecDecrypterJSON = {
export type ThresholdDecrypterJSON = {
porterUri: string;
ritualId: number;
threshold: number;
};

export class CbdTDecDecrypter {
export class ThresholdDecrypter {
// private readonly verifyingKey: Keyring;

private constructor(
private readonly porter: Porter,
private readonly porter: PorterClient,
private readonly ritualId: number,
private readonly threshold: number
) {}

public static create(porterUri: string, dkgRitual: DkgRitual) {
return new CbdTDecDecrypter(
new Porter(porterUri),
return new ThresholdDecrypter(
new PorterClient(porterUri),
dkgRitual.id,
dkgRitual.dkgParams.threshold
);
Expand Down Expand Up @@ -223,7 +222,7 @@ export class CbdTDecDecrypter {
return SessionStaticSecret.random();
}

public toObj(): CbdTDecDecrypterJSON {
public toObj(): ThresholdDecrypterJSON {
return {
porterUri: this.porter.porterUrl.toString(),
ritualId: this.ritualId,
Expand All @@ -239,15 +238,19 @@ export class CbdTDecDecrypter {
porterUri,
ritualId,
threshold,
}: CbdTDecDecrypterJSON) {
return new CbdTDecDecrypter(new Porter(porterUri), ritualId, threshold);
}: ThresholdDecrypterJSON) {
return new ThresholdDecrypter(
new PorterClient(porterUri),
ritualId,
threshold
);
}

public static fromJSON(json: string) {
return CbdTDecDecrypter.fromObj(fromJSON(json));
return ThresholdDecrypter.fromObj(fromJSON(json));
}

public equals(other: CbdTDecDecrypter): boolean {
public equals(other: ThresholdDecrypter): boolean {
return (
this.porter.porterUrl.toString() === other.porter.porterUrl.toString()
);
Expand Down
45 changes: 20 additions & 25 deletions src/characters/pre-recipient.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,23 +12,22 @@ import { ConditionContext, ConditionExpression } from '../conditions';
import { Keyring } from '../keyring';
import { PolicyMessageKit } from '../kits/message';
import { RetrievalResult } from '../kits/retrieval';
import { base64ToU8Receiver, bytesEquals, toJSON, zip } from '../utils';
import { PorterClient } from '../porter';
import { base64ToU8Receiver, toJSON, zip } from '../utils';

import { Porter } from './porter';

export type PreTDecDecrypterJSON = {
export type PreDecrypterJSON = {
porterUri: string;
policyEncryptingKeyBytes: Uint8Array;
encryptedTreasureMapBytes: Uint8Array;
publisherVerifyingKeyBytes: Uint8Array;
bobSecretKeyBytes: Uint8Array;
Copy link
Member

Choose a reason for hiding this comment

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

If this is no longer PreTDecDecrypter, and just PreDecrypter, then bob's secret keys should no longer be needed, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How does the PreDecrypter decrypt without a secret key?

Copy link
Member

@derekpierre derekpierre Jun 29, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

For PRE-tDEC Bob's secret key bytes were publicly known anyway so I thought that's why they were passed in here i.e. the Universal Bob concept.

However, for just PRE, Bob's secret bytes should continue to be a secret to everyone except for Bob. So a few things stand out:

  1. Should a PREDecrypter be handling Bob's secret bytes at all - that seems like a security hole. More philosophically, what is a "decrypter" in the context of PRE? For example, does a PRE decrypter return re-encrypted bytes (encrypted for Bob) that Bob can then decrypt himself instead? Basically it gets the data re-encrypted and is separated from decryption. It's kind of like the line between Porter and Bob or in Python the line between PRERetrievalClient and Bob. In which case, it's not a "decrypter" per se.

  2. If PREDecrypters are persistable they should not be persisting bob's secret bytes. Bob should maintain control of his own secret key, which brings me back to point 1) above.

I might need to go back and look at the existing code, maybe I'm just missing something - I think this is where the API becomes very tricky 😅

Copy link
Contributor Author

@piotr-roslaniec piotr-roslaniec Jun 30, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I think I have more clarity now on where to go from there: It feels to me like PREDecrypter is no longer a satisfactory abstraction for decryption, and that job should be delegated to Bob. And in that case, maybe we should also revisit the shape and function of PreStrategy.

These items look like a big job, and I'd rather sketch a refactoring in a separate issue (#166), discuss it, and then fix it in another PR.

};

export class PreTDecDecrypter {
export class PreDecrypter {
// private readonly verifyingKey: Keyring;

constructor(
private readonly porter: Porter,
private readonly porter: PorterClient,
private readonly keyring: Keyring,
private readonly policyEncryptingKey: PublicKey,
private readonly publisherVerifyingKey: PublicKey,
Expand All @@ -41,9 +40,9 @@ export class PreTDecDecrypter {
policyEncryptingKey: PublicKey,
publisherVerifyingKey: PublicKey,
encryptedTreasureMap: EncryptedTreasureMap
): PreTDecDecrypter {
return new PreTDecDecrypter(
new Porter(porterUri),
): PreDecrypter {
return new PreDecrypter(
new PorterClient(porterUri),
new Keyring(secretKey),
policyEncryptingKey,
publisherVerifyingKey,
Expand Down Expand Up @@ -149,7 +148,7 @@ export class PreTDecDecrypter {
});
}

public toObj(): PreTDecDecrypterJSON {
public toObj(): PreDecrypterJSON {
return {
porterUri: this.porter.porterUrl.toString(),
policyEncryptingKeyBytes: this.policyEncryptingKey.toCompressedBytes(),
Expand All @@ -170,9 +169,9 @@ export class PreTDecDecrypter {
encryptedTreasureMapBytes,
publisherVerifyingKeyBytes,
bobSecretKeyBytes,
}: PreTDecDecrypterJSON) {
return new PreTDecDecrypter(
new Porter(porterUri),
}: PreDecrypterJSON) {
return new PreDecrypter(
new PorterClient(porterUri),
new Keyring(SecretKey.fromBEBytes(bobSecretKeyBytes)),
PublicKey.fromCompressedBytes(policyEncryptingKeyBytes),
PublicKey.fromCompressedBytes(publisherVerifyingKeyBytes),
Expand All @@ -182,19 +181,15 @@ export class PreTDecDecrypter {

public static fromJSON(json: string) {
const config = JSON.parse(json, base64ToU8Receiver);
return PreTDecDecrypter.fromObj(config);
return PreDecrypter.fromObj(config);
}

public equals(other: PreTDecDecrypter): boolean {
return (
this.porter.porterUrl.toString() === other.porter.porterUrl.toString() &&
this.policyEncryptingKey.equals(other.policyEncryptingKey) &&
// TODO: Replace with `equals` after https://github.com/nucypher/nucypher-core/issues/56 is fixed
bytesEquals(
this.encryptedTreasureMap.toBytes(),
other.encryptedTreasureMap.toBytes()
) &&
this.publisherVerifyingKey.equals(other.publisherVerifyingKey)
);
public equals(other: PreDecrypter): boolean {
return [
this.porter.porterUrl.toString() === other.porter.porterUrl.toString(),
this.policyEncryptingKey.equals(other.policyEncryptingKey),
this.encryptedTreasureMap.equals(other.encryptedTreasureMap),
this.publisherVerifyingKey.equals(other.publisherVerifyingKey),
].every(Boolean);
}
}
8 changes: 4 additions & 4 deletions src/conditions/condition-expr.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,9 +95,9 @@ export class ConditionExpression {
}

public equals(other: ConditionExpression): boolean {
return (
this.version === other.version &&
objectEquals(this.condition.toObj(), other.condition.toObj())
);
return [
this.version === other.version,
objectEquals(this.condition.toObj(), other.condition.toObj()),
].every(Boolean);
}
}
Loading
Loading