Skip to content

Commit

Permalink
fix(multipart upload): lock down multipart upload
Browse files Browse the repository at this point in the history 
Added checks to prevent overwriting of files with multi part upload and that the correct zKey index
is uploaded
  • Loading branch information
@ctrlc03 @0xjei
ctrlc03 authored and 0xjei committed Mar 21, 2023
1 parent 5305a01 commit 6ca3963
Show file tree
Hide file tree
Showing 2 changed files with 240 additions and 20 deletions.
169 changes: 165 additions & 4 deletions packages/actions/test/unit/security.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ import {
import { where } from "firebase/firestore"
import { createOAuthDeviceAuth } from "@octokit/auth-oauth-device"
import { randomBytes } from "crypto"
import { fakeCeremoniesData, fakeCircuitsData, fakeUsersData } from "../data/samples"
import { CircuitDocumentReferenceAndData } from "src/types"
import { fakeCeremoniesData, fakeCircuitsData, fakeParticipantsData, fakeUsersData } from "../data/samples"
import {
deleteAdminApp,
envType,
Expand All @@ -22,18 +23,29 @@ import {
cleanUpMockUsers,
getAuthenticationConfiguration,
cleanUpMockCeremony,
createMockCeremony
createMockCeremony,
createMockParticipant,
cleanUpMockParticipant,
getStorageConfiguration,
sleep,
deleteBucket
} from "../utils"
import {
commonTerms,
formatZkeyIndex,
generateGetObjectPreSignedUrl,
genesisZkeyIndex,
getBucketName,
getCurrentFirebaseAuthUser,
getZkeyStorageFilePath,
isCoordinator,
signInToFirebaseWithCredentials
} from "../../src"
import { TestingEnvironment } from "../../src/types/enums"
import { getDocumentById, queryCollection } from "../../src/helpers/database"
import { getCircuitsCollectionPath, getDocumentById, queryCollection } from "../../src/helpers/database"
import { simulateOnVerification } from "../utils/authentication"
import { generateFakeCircuit } from "../data/generators"
import { createS3Bucket, openMultiPartUpload } from "../../src/helpers/functions"

chai.use(chaiAsPromised)

Expand All @@ -49,6 +61,8 @@ describe("Security", () => {
const users = [fakeUsersData.fakeUser1, fakeUsersData.fakeUser2, fakeUsersData.fakeUser3]
const passwords = generateUserPasswords(users.length)

const { ceremonyBucketPostfix } = getStorageConfiguration()

/// @note pre conditions for production tests
if (envType === TestingEnvironment.PRODUCTION) {
if (
Expand All @@ -63,7 +77,8 @@ describe("Security", () => {
throw new Error("Missing environment variables for Firebase tests.")
}

// create users for all tests
let circuitsCurrentContributor: CircuitDocumentReferenceAndData

beforeAll(async () => {
for (let i = 0; i < users.length; i++) {
users[i].uid = await createMockUser(
Expand All @@ -74,6 +89,63 @@ describe("Security", () => {
adminAuth
)
}

circuitsCurrentContributor = generateFakeCircuit({
uid: "000000000000000000A4",
data: {
name: "Circuit Small",
description: "Short description of Circuit Small for testing",
prefix: "circuit-small",
sequencePosition: 1,
fixedTimeWindow: 10,
zKeySizeInBytes: 45020,
lastUpdated: Date.now(),
metadata: {
constraints: 65,
curve: "bn-128",
labels: 79,
outputs: 1,
pot: 7,
privateInputs: 0,
publicInputs: 2,
wires: 67
},
template: {
commitHash: "295d995802b152a1dc73b5d0690ce3f8ca5d9b23",
paramsConfiguration: ["2"],
source: "https://github.com/0xjei/circom-starter/blob/dev/circuits/exercise/checkAscendingOrder.circom"
},
waitingQueue: {
completedContributions: 0,
contributors: [users[0].uid, users[1].uid],
currentContributor: users[0].uid, // fake user 1
failedContributions: 0
},
files: {
initialZkeyBlake2bHash:
"eea0a468524a984908bff6de1de09867ac5d5b0caed92c3332fd5ec61004f79505a784df9d23f69f33efbfef016ad3138871fa8ad63b6e8124a9d0721b0e9e32",
initialZkeyFilename: "circuit_small_00000.zkey",
initialZkeyStoragePath: "circuits/circuit_small/contributions/circuit_small_00000.zkey",
potBlake2bHash:
"34379653611c22a7647da22893c606f9840b38d1cb6da3368df85c2e0b709cfdb03a8efe91ce621a424a39fe4d5f5451266d91d21203148c2d7d61cf5298d119",
potFilename: "powersOfTau28_hez_final_07.ptau",
potStoragePath: "pot/powersOfTau28_hez_final_07.ptau",
r1csBlake2bHash:
"0739198d5578a4bdaeb2fa2a1043a1d9cac988472f97337a0a60c296052b82d6cecb6ae7ce503ab9864bc86a38cdb583f2d33877c41543cbf19049510bca7472",
r1csFilename: "circuit_small.r1cs",
r1csStoragePath: "circuits/circuit_small/circuit_small.r1cs"
},
avgTimings: {
contributionComputation: 0,
fullContribution: 0,
verifyCloudFunction: 0
},
compiler: {
commitHash: "ed807764a17ce06d8307cd611ab6b917247914f5",
version: "2.0.5"
}
}
})
})

describe("GeneratePreSignedURL", () => {
Expand Down Expand Up @@ -188,6 +260,95 @@ describe("Security", () => {
})
})

if (envType === TestingEnvironment.PRODUCTION) {
// Tests related to multi part upload security
// @note We want to make sure that a contributor can
// 1. only upload when in contributing status (current contributor)
// 2. only upload a file with the correct name
// 3. only upload a valid zkey file and previous zkey
describe("Multipart upload", () => {
const participant = fakeParticipantsData.fakeParticipantCurrentContributorUploading
const ceremonyNotContributor = fakeCeremoniesData.fakeCeremonyOpenedFixed
const ceremonyContributor = fakeCeremoniesData.fakeCeremonyOpenedDynamic
const circuitsNotCurrentContributor = fakeCircuitsData.fakeCircuitSmallContributors
const bucketName = getBucketName(ceremonyContributor.data.prefix!, ceremonyBucketPostfix)
beforeAll(async () => {
// we need the pre conditions to meet
await createMockCeremony(adminFirestore, ceremonyNotContributor, circuitsNotCurrentContributor)
await createMockCeremony(adminFirestore, ceremonyContributor, circuitsCurrentContributor)
await createMockParticipant(adminFirestore, ceremonyNotContributor.uid, users[0].uid, participant)
await createMockParticipant(adminFirestore, ceremonyContributor.uid, users[0].uid, participant)
await signInWithEmailAndPassword(userAuth, users[2].data.email, passwords[2])
await createS3Bucket(userFunctions, bucketName)
await sleep(2000)
})

afterAll(async () => {
// we need to delete the pre conditions
await cleanUpMockCeremony(adminFirestore, ceremonyNotContributor.uid, circuitsNotCurrentContributor.uid)
await cleanUpMockCeremony(adminFirestore, ceremonyContributor.uid, circuitsCurrentContributor.uid)
await cleanUpMockParticipant(adminFirestore, ceremonyNotContributor.uid, users[0].uid)
await cleanUpMockParticipant(adminFirestore, ceremonyContributor.uid, users[0].uid)
await deleteBucket(bucketName)
})

it("should succeed when the user is the current contributor and is upload valid zkey index file", async () => {
await signInWithEmailAndPassword(userAuth, users[0].data.email, passwords[0])
// we need to set the waiting queue because initEmptyWaitingQueue might
// mess up with us and reset it before we call
await adminFirestore
.collection(getCircuitsCollectionPath(ceremonyContributor.uid))
.doc(circuitsCurrentContributor.uid)
.set({
prefix: circuitsCurrentContributor.data.prefix,
waitingQueue: {
completedContributions: 0,
contributors: [users[0].uid, users[1].uid],
currentContributor: users[0].uid, // fake user 1
failedContributions: 0
}
})

await expect(
openMultiPartUpload(
userFunctions,
getBucketName(ceremonyContributor.data.prefix!, ceremonyBucketPostfix),
getZkeyStorageFilePath(
circuitsCurrentContributor.data.prefix!,
`${circuitsCurrentContributor.data.prefix}_${formatZkeyIndex(1)}.zkey`
),
ceremonyContributor.uid
)
).to.be.fulfilled
})
it("should revert when the user is not a contributor for this ceremony circuit", async () => {
await signInWithEmailAndPassword(userAuth, users[0].data.email, passwords[0])
await expect(
openMultiPartUpload(
userFunctions,
getBucketName(ceremonyNotContributor.data.prefix!, ceremonyBucketPostfix),
`${circuitsNotCurrentContributor.data.prefix}_${genesisZkeyIndex}.zkey`,
ceremonyNotContributor.uid
)
).to.be.rejectedWith(
"Unable to interact with a multi-part upload (start, create pre-signed urls or complete)."
)
})
it("should fail when the user is trying to upload a file to a bucket not part of a ceremony", async () => {
await signInWithEmailAndPassword(userAuth, users[0].data.email, passwords[0])
await expect(
openMultiPartUpload(
userFunctions,
"not-a-ceremony-bucket",
`${circuitsNotCurrentContributor.data.prefix}_${formatZkeyIndex(1)}.zkey`,
ceremonyNotContributor.uid
)
// @todo discuss whether this error name should be changed to be more general?
).to.be.rejectedWith("Unable to generate a pre-signed url for the given object in the provided bucket.")
})
})
}

// Tests related to authentication security
// @note It is recommended to run these tests
// on their own, as they take a long time
Expand Down
91 changes: 75 additions & 16 deletions packages/backend/src/functions/storage.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@ import {
} from "@aws-sdk/client-s3"
import { getSignedUrl } from "@aws-sdk/s3-request-presigner"
import dotenv from "dotenv"
import { commonTerms, getParticipantsCollectionPath } from "@zkmpc/actions/src"
import { commonTerms, formatZkeyIndex, getParticipantsCollectionPath, getZkeyStorageFilePath } from "@zkmpc/actions/src"
import { ParticipantStatus, ParticipantContributionStep } from "@zkmpc/actions/src/types/enums"
import { getDocumentById } from "../lib/utils"
import { getCeremonyCircuits, getDocumentById } from "../lib/utils"
import { COMMON_ERRORS, logAndThrowError, makeError, printLog, SPECIFIC_ERRORS } from "../lib/errors"
import { LogLevel } from "../../types/enums"
import { getS3Client } from "../lib/services"
Expand Down Expand Up @@ -53,6 +53,68 @@ const checkPreConditionForCurrentContributorToInteractWithMultiPartUpload = asyn
logAndThrowError(SPECIFIC_ERRORS.SE_STORAGE_CANNOT_INTERACT_WITH_MULTI_PART_UPLOAD)
}

/**
* Helper function to check whether a contributor is uploading a file related to its contribution.
* @param contributorId <string> - the unique identifier of the contributor.
* @param ceremonyId <string> - the unique identifier of the ceremony.
* @param objectKey <string> - the object key of the file being uploaded.
*/
const checkUploadedFileValidity = async (contributorId: string, ceremonyId: string, objectKey: string) => {
// Get the circuits for the ceremony
const circuits = await getCeremonyCircuits(ceremonyId)

// We need to have at least 1 circuit
if (circuits.length === 0) logAndThrowError(SPECIFIC_ERRORS.SE_CONTRIBUTE_NO_CEREMONY_CIRCUITS)

// Loop through the circuits until we find the one we are contributing to
for (const circuit of circuits) {
// Extract the data we need
const { prefix, waitingQueue } = circuit.data()!
const { completedContributions, currentContributor } = waitingQueue

// If we are not a contributor to this circuit, continue looping
if (currentContributor === contributorId) {
// Get the index of the zKey
const contributorZKeyIndex = formatZkeyIndex(completedContributions + 1)
// The uploaded file must be the expected one
const zkeyNameContributor = `${prefix}_${contributorZKeyIndex}.zkey`
const contributorZKeyStoragePath = getZkeyStorageFilePath(prefix, zkeyNameContributor)

// If the object key is not one of the two zkeys, throw an error
if (objectKey !== contributorZKeyStoragePath) {
logAndThrowError(SPECIFIC_ERRORS.SE_STORAGE_CANNOT_INTERACT_WITH_MULTI_PART_UPLOAD)
}

// void return if we found a match and the contributor can upload the zkey
return
}
}

// if there was no match for the circuit current contributor, then throw an error
logAndThrowError(SPECIFIC_ERRORS.SE_STORAGE_CANNOT_INTERACT_WITH_MULTI_PART_UPLOAD)
}

/**
* Helper function that confirms whether a bucket is used for a ceremony.
* @dev this helps to prevent unauthorized access to coordinator's buckets.
* @param bucketName
*/
const checkIfBucketDedicatedToCeremony = async (bucketName: string) => {
// Get Firestore DB.
const firestoreDatabase = admin.firestore()

// Extract ceremony prefix from bucket name.
const ceremonyPrefix = bucketName.replace(String(process.env.AWS_CEREMONY_BUCKET_POSTFIX), "")

// Query the collection.
const ceremonyCollection = await firestoreDatabase
.collection(commonTerms.collections.ceremonies.name)
.where(commonTerms.collections.ceremonies.fields.prefix, "==", ceremonyPrefix)
.get()

if (ceremonyCollection.empty) logAndThrowError(SPECIFIC_ERRORS.SE_STORAGE_BUCKET_NOT_CONNECTED_TO_CEREMONY)
}

/**
* Create a new AWS S3 bucket for a particular ceremony.
* @notice the S3 bucket is used to store all the ceremony artifacts and contributions.
Expand Down Expand Up @@ -166,19 +228,8 @@ export const generateGetObjectPreSignedUrl = functions.https.onCall(
// Prepare input data.
const { objectKey, bucketName } = data

// Get Firestore DB.
const firestoreDatabase = admin.firestore()

// Extract ceremony prefix from bucket name.
const ceremonyPrefix = bucketName.replace(String(process.env.AWS_CEREMONY_BUCKET_POSTFIX), "")

// Query the collection.
const ceremonyCollection = await firestoreDatabase
.collection(commonTerms.collections.ceremonies.name)
.where(commonTerms.collections.ceremonies.fields.prefix, "==", ceremonyPrefix)
.get()

if (ceremonyCollection.empty) logAndThrowError(SPECIFIC_ERRORS.SE_STORAGE_BUCKET_NOT_CONNECTED_TO_CEREMONY)
// Check whether the bucket for which we are generating the pre-signed url is dedicated to a ceremony.
await checkIfBucketDedicatedToCeremony(bucketName)

// Connect to S3 client.
const S3 = await getS3Client()
Expand Down Expand Up @@ -227,6 +278,12 @@ export const startMultiPartUpload = functions.https.onCall(
if (context.auth?.token.participant && !!ceremonyId) {
// Check pre-condition.
await checkPreConditionForCurrentContributorToInteractWithMultiPartUpload(userId!, ceremonyId)

// Check whether the bucket where the object for which we are generating the pre-signed url is dedicated to a ceremony.
await checkIfBucketDedicatedToCeremony(bucketName)

// Check the validity of the uploaded file.
await checkUploadedFileValidity(userId!, ceremonyId!, objectKey)
}

// Connect to S3 client.
Expand All @@ -238,7 +295,6 @@ export const startMultiPartUpload = functions.https.onCall(
try {
// Execute S3 command.
const response = await S3.send(command)

if (response.$metadata.httpStatusCode === 200 && !!response.UploadId) {
printLog(
`The multi-part upload identifier is ${response.UploadId}. Requested by ${userId}`,
Expand Down Expand Up @@ -358,6 +414,9 @@ export const completeMultiPartUpload = functions.https.onCall(
await checkPreConditionForCurrentContributorToInteractWithMultiPartUpload(userId!, ceremonyId)
}

// Check if the bucket is dedicated to a ceremony.
await checkIfBucketDedicatedToCeremony(bucketName)

// Connect to S3.
const S3 = await getS3Client()

Expand Down

0 comments on commit 6ca3963

Please sign in to comment.