Add certificatesRequests store validation

[leblowl]

We should verify that each CSR is signed by pubKey (see: #1892) and validate usernames, etc.

This prevents users from forging other users' CSRs, overwriting other users' username requests and from adding invalid usernames.

Currently, with the additions of #1843, CSRs are replicated:

quiet/packages/backend/src/nest/storage/storage.service.ts

Line 404 in e019b86

this.emit(StorageEvents.REPLICATED_CSR, { csrs: allCsrs, certificates: allCertificates })

And via RESPONSE_GET_CSRS, these CSRs are sent to the frontend:

quiet/packages/backend/src/nest/connections-manager/connections-manager.service.ts

Lines 562 to 569 in e019b86

	this.storageService.on(
	StorageEvents.REPLICATED_CSR,
	async (payload: { csrs: string[]; certificates: string[] }) => {
	console.log(`On ${StorageEvents.REPLICATED_CSR}`)
	this.serverIoProvider.io.emit(SocketActionTypes.RESPONSE_GET_CSRS, { csrs: payload.csrs })
	this.registrationService.emit(RegistrationEvents.REGISTER_USER_CERTIFICATE, payload)
	}
	)

CSRs are indexed by public key:

quiet/packages/state-manager/src/sagas/users/users.selectors.ts

Line 48 in e019b86

export const csrsMapping = createSelector(csrs, csrs => {

Where the public key is extracted from the decoded cert:

quiet/packages/identity/src/extractPubKey.ts

Line 14 in 277f966

export const parseCertificationRequest = (pem: string): CertificationRequest => {

quiet/packages/identity/src/extractPubKey.ts

Line 22 in e019b86

return Buffer.from(certificate.subjectPublicKeyInfo.subjectPublicKey.valueBlock.valueHex).toString('base64')

These CSRs are then used for associating username with users:

quiet/packages/state-manager/src/sagas/users/users.selectors.ts

Line 76 in e019b86

export const allUsers = createSelector(csrsMapping, certificatesMapping, (csrs, certs) => {

It appears that the CSR isn't validated in this example, so you could BER encode a CSR with someone else's public key and since we don't check the signature, we wouldn't notice.

Following the other path that CSRs take, they get signed by the owner via REGISTER_USER_CERTIFICATE. In this path, first they are decoded:

quiet/packages/identity/src/common.ts

Lines 92 to 95 in 277f966

	export const loadCSR = async (csr: string): Promise<CertificationRequest> => {
	const certBuffer = stringToArrayBuffer(fromBase64(csr))
	const asn1 = fromBER(certBuffer)
	return new CertificationRequest({ schema: asn1.result })

They are then filtered so only CSRs with unregistered names are prepared for signing:

quiet/packages/backend/src/nest/registration/registration.functions.ts

Line 29 in e019b86

export const extractPendingCsrs = async (payload: { csrs: string[]; certificates: string[] }) => {

Then each filtered CSR is signed:

quiet/packages/backend/src/nest/registration/registration.functions.ts

Line 72 in e019b86

export const issueCertificate = async (userCsr: string, permsData: PermsData): Promise<RegistrarResponse> => {

Inside issueCertificate, there is a validateCsr function, but that only appears to validate the BER encoding and basic structural properties:

quiet/packages/backend/src/nest/registration/registration.functions.ts

Lines 62 to 67 in e019b86

	export const validateCsr = async (csr: string) => {
	const userData = new UserCsrData()
	userData.csr = csr
	const validationErrors = await validate(userData)
	return validationErrors
	}

quiet/packages/backend/src/nest/registration/registration.functions.ts

Lines 11 to 16 in e019b86

	class UserCsrData {
	@IsNotEmpty()
	@IsBase64()
	@IsCsr()
	@CsrContainsFields()
	csr: string

generateUserCert also doesn't appear to validate the CSR signature before signing:

quiet/packages/identity/src/generateUserCertificate.ts

Line 24 in 277f966

export const createUserCert = async (

So it looks like anyone could send a CSR with another user's public key in the CSR and an invalid signature (because they don't control that public key) and thus change another user's username.

[holmesworcester]

How would overwriting others' usernames work? Do you mean submitting a request for the name alice right after someone else submits a request for the name alice? I think we can't stop that with an access controller. Or do you mean something else?

[leblowl]

@holmesworcester I added additional details to the issue. Basically, if we don't check CSR signatures then I think anyone can put someone else's public key and a random username in a CSR (the CSR signature won't be valid, but we don't check that) then get a certificate issued for it, thus changing another user's username. The signature of the CSR needs to match the CSR data and be signed by the public key included in the CSR data, otherwise you can just include any public key in the CSR.

[holmesworcester]

Note: we would do this together with #1892

[leblowl]

In order to validate a CSR, we need to check:

• that the CSR is valid (parses correctly, formatted correctly, contains required fields, etc.)
• that the CSR is signed by the public key included in the CSR
• that the username in the CSR is valid (ASCII a-z, dashes and numbers)
• TODO: validate other fields in CSR?

To implement this validation, we could use an access controller, but it looks like access controllers only check heads and we need to validate each entry. As an alternative, we can create a new validation function validateCsrEntry that, given an entry, validates the entry according the specs above (and calls Entry.verify) and returns true/false. Then we can create a new function, getCsrs that iterates over the certificatesRequests log entries, and filters the logs with validateCsrEntry such that it only returns valid entries. We can then use getCsrs to send valid CSRs to other parts of the application for processing. Also logic from the state-manager that retrieves the latest CSR for each public key should be moved into getCsrs on the backend.

With this approach we will still be caching and replicating invalid entries in IPFS, however when we implement permanent deletion, we can revisit that.

This approach also involves re-validating invalid entries when iterating over the log successively. That's not ideal, but I think we can optimize it in the future.

It might be nice to encapsulate everything in a CertificateRequestStore class. See also: #1923

[holmesworcester]

I think the validation should lowercase letters only, hyphens allowed, and numbers

[kingalg]

Desktop: [email protected]
Mobile: [email protected], ios 344