Skip to content

Commit

Permalink
Explicitly free some Rust-side objects (#3911)
Browse files Browse the repository at this point in the history 
* Explicitly `free` stuff returned by `OlmMachine.getIdentity()`

* Explicitly `free` stuff returned by `OlmMachine.getDevice()`

* one more
  • Loading branch information
@richvdh
richvdh committed Nov 28, 2023
1 parent 8ef2ca6 commit a749662
Show file tree
Hide file tree
Showing 3 changed files with 120 additions and 69 deletions.
3 changes: 2 additions & 1 deletion spec/unit/rust-crypto/rust-crypto.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -645,6 +645,7 @@ describe("RustCrypto", () => {

it("should call getDevice", async () => {
olmMachine.getDevice.mockResolvedValue({
free: jest.fn(),
isCrossSigningTrusted: jest.fn().mockReturnValue(false),
isLocallyTrusted: jest.fn().mockReturnValue(false),
isCrossSignedByOwner: jest.fn().mockReturnValue(false),
Expand Down Expand Up @@ -871,7 +872,7 @@ describe("RustCrypto", () => {
});

it("returns a verified UserVerificationStatus when the UserIdentity is verified", async () => {
olmMachine.getIdentity.mockResolvedValue({ isVerified: jest.fn().mockReturnValue(true) });
olmMachine.getIdentity.mockResolvedValue({ free: jest.fn(), isVerified: jest.fn().mockReturnValue(true) });

const userVerificationStatus = await rustCrypto.getUserVerificationStatus(testData.TEST_USER_ID);
expect(userVerificationStatus.isVerified()).toBeTruthy();
Expand Down
11 changes: 7 additions & 4 deletions src/rust-crypto/CrossSigningIdentity.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -91,10 +91,13 @@ export class CrossSigningIdentity {
this.olmMachine.userId,
this.olmMachine.deviceId,
);

// Sign the device with our cross-signing key and upload the signature
const request: RustSdkCryptoJs.SignatureUploadRequest = await device.verify();
await this.outgoingRequestProcessor.makeOutgoingRequest(request);
try {
// Sign the device with our cross-signing key and upload the signature
const request: RustSdkCryptoJs.SignatureUploadRequest = await device.verify();
await this.outgoingRequestProcessor.makeOutgoingRequest(request);
} finally {
device.free();
}
} else {
logger.log(
"bootStrapCrossSigning: Cross-signing private keys not found locally or in secret storage, creating new keys",
Expand Down
175 changes: 111 additions & 64 deletions src/rust-crypto/rust-crypto.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -425,6 +425,7 @@ export class RustCrypto extends TypedEventEmitter<RustCryptoEvents, RustCryptoEv
await this.outgoingRequestProcessor.makeOutgoingRequest(request);
}
const userIdentity = await this.olmMachine.getIdentity(rustTrackedUser);
userIdentity?.free();
return userIdentity !== undefined;
} else if (downloadUncached) {
// Download the cross signing keys and check if the master key is available
Expand Down Expand Up @@ -562,7 +563,13 @@ export class RustCrypto extends TypedEventEmitter<RustCryptoEvents, RustCryptoEv
if (!device) {
throw new Error(`Unknown device ${userId}|${deviceId}`);
}
await device.setLocalTrust(verified ? RustSdkCryptoJs.LocalTrust.Verified : RustSdkCryptoJs.LocalTrust.Unset);
try {
await device.setLocalTrust(
verified ? RustSdkCryptoJs.LocalTrust.Verified : RustSdkCryptoJs.LocalTrust.Unset,
);
} finally {
device.free();
}
}

/**
Expand All @@ -578,13 +585,16 @@ export class RustCrypto extends TypedEventEmitter<RustCryptoEvents, RustCryptoEv
);

if (!device) return null;

return new DeviceVerificationStatus({
signedByOwner: device.isCrossSignedByOwner(),
crossSigningVerified: device.isCrossSigningTrusted(),
localVerified: device.isLocallyTrusted(),
trustCrossSignedDevices: this._trustCrossSignedDevices,
});
try {
return new DeviceVerificationStatus({
signedByOwner: device.isCrossSignedByOwner(),
crossSigningVerified: device.isCrossSigningTrusted(),
localVerified: device.isLocallyTrusted(),
trustCrossSignedDevices: this._trustCrossSignedDevices,
});
} finally {
device.free();
}
}

/**
Expand All @@ -596,7 +606,9 @@ export class RustCrypto extends TypedEventEmitter<RustCryptoEvents, RustCryptoEv
if (userIdentity === undefined) {
return new UserVerificationStatus(false, false, false);
}
return new UserVerificationStatus(userIdentity.isVerified(), false, false);
const verified = userIdentity.isVerified();
userIdentity.free();
return new UserVerificationStatus(verified, false, false);
}

/**
Expand All @@ -621,42 +633,51 @@ export class RustCrypto extends TypedEventEmitter<RustCryptoEvents, RustCryptoEv
const userIdentity: RustSdkCryptoJs.OwnUserIdentity | undefined = await this.olmMachine.getIdentity(
new RustSdkCryptoJs.UserId(this.userId),
);

const crossSigningStatus: RustSdkCryptoJs.CrossSigningStatus = await this.olmMachine.crossSigningStatus();
const privateKeysOnDevice =
crossSigningStatus.hasMaster && crossSigningStatus.hasUserSigning && crossSigningStatus.hasSelfSigning;

if (!userIdentity || !privateKeysOnDevice) {
// The public or private keys are not available on this device
if (!userIdentity) {
// The public keys are not available on this device
return null;
}

if (!userIdentity.isVerified()) {
// We have both public and private keys, but they don't match!
return null;
}
try {
const crossSigningStatus: RustSdkCryptoJs.CrossSigningStatus = await this.olmMachine.crossSigningStatus();

let key: string;
switch (type) {
case CrossSigningKey.Master:
key = userIdentity.masterKey;
break;
case CrossSigningKey.SelfSigning:
key = userIdentity.selfSigningKey;
break;
case CrossSigningKey.UserSigning:
key = userIdentity.userSigningKey;
break;
default:
// Unknown type
const privateKeysOnDevice =
crossSigningStatus.hasMaster && crossSigningStatus.hasUserSigning && crossSigningStatus.hasSelfSigning;

if (!privateKeysOnDevice) {
// The private keys are not available on this device
return null;
}
}

const parsedKey: CrossSigningKeyInfo = JSON.parse(key);
// `keys` is an object with { [`ed25519:${pubKey}`]: pubKey }
// We assume only a single key, and we want the bare form without type
// prefix, so we select the values.
return Object.values(parsedKey.keys)[0];
if (!userIdentity.isVerified()) {
// We have both public and private keys, but they don't match!
return null;
}

let key: string;
switch (type) {
case CrossSigningKey.Master:
key = userIdentity.masterKey;
break;
case CrossSigningKey.SelfSigning:
key = userIdentity.selfSigningKey;
break;
case CrossSigningKey.UserSigning:
key = userIdentity.userSigningKey;
break;
default:
// Unknown type
return null;
}

const parsedKey: CrossSigningKeyInfo = JSON.parse(key);
// `keys` is an object with { [`ed25519:${pubKey}`]: pubKey }
// We assume only a single key, and we want the bare form without type
// prefix, so we select the values.
return Object.values(parsedKey.keys)[0];
} finally {
userIdentity.free();
}
}

/**
Expand Down Expand Up @@ -800,6 +821,8 @@ export class RustCrypto extends TypedEventEmitter<RustCryptoEvents, RustCryptoEv
Boolean(userIdentity?.masterKey) &&
Boolean(userIdentity?.selfSigningKey) &&
Boolean(userIdentity?.userSigningKey);
userIdentity?.free();

const privateKeysInSecretStorage = await secretStorageContainsCrossSigningKeys(this.secretStorage);
const crossSigningStatus: RustSdkCryptoJs.CrossSigningStatus | null =
await this.getOlmMachineOrThrow().crossSigningStatus();
Expand Down Expand Up @@ -917,23 +940,31 @@ export class RustCrypto extends TypedEventEmitter<RustCryptoEvents, RustCryptoEv

if (!userIdentity) throw new Error(`unknown userId ${userId}`);

// Transform the verification methods into rust objects
const methods = this._supportedVerificationMethods.map((method) =>
verificationMethodIdentifierToMethod(method),
);
// Get the request content to send to the DM room
const verificationEventContent: string = await userIdentity.verificationRequestContent(methods);
try {
// Transform the verification methods into rust objects
const methods = this._supportedVerificationMethods.map((method) =>
verificationMethodIdentifierToMethod(method),
);
// Get the request content to send to the DM room
const verificationEventContent: string = await userIdentity.verificationRequestContent(methods);

// Send the request content to send to the DM room
const eventId = await this.sendVerificationRequestContent(roomId, verificationEventContent);
// Send the request content to send to the DM room
const eventId = await this.sendVerificationRequestContent(roomId, verificationEventContent);

// Get a verification request
const request: RustSdkCryptoJs.VerificationRequest = await userIdentity.requestVerification(
new RustSdkCryptoJs.RoomId(roomId),
new RustSdkCryptoJs.EventId(eventId),
methods,
);
return new RustVerificationRequest(request, this.outgoingRequestProcessor, this._supportedVerificationMethods);
// Get a verification request
const request: RustSdkCryptoJs.VerificationRequest = await userIdentity.requestVerification(
new RustSdkCryptoJs.RoomId(roomId),
new RustSdkCryptoJs.EventId(eventId),
methods,
);
return new RustVerificationRequest(
request,
this.outgoingRequestProcessor,
this._supportedVerificationMethods,
);
} finally {
userIdentity.free();
}
}

/**
Expand Down Expand Up @@ -995,12 +1026,20 @@ export class RustCrypto extends TypedEventEmitter<RustCryptoEvents, RustCryptoEv
throw new Error("cannot request verification for this device when there is no existing cross-signing key");
}

const [request, outgoingRequest]: [RustSdkCryptoJs.VerificationRequest, RustSdkCryptoJs.ToDeviceRequest] =
await userIdentity.requestVerification(
this._supportedVerificationMethods.map(verificationMethodIdentifierToMethod),
try {
const [request, outgoingRequest]: [RustSdkCryptoJs.VerificationRequest, RustSdkCryptoJs.ToDeviceRequest] =
await userIdentity.requestVerification(
this._supportedVerificationMethods.map(verificationMethodIdentifierToMethod),
);
await this.outgoingRequestProcessor.makeOutgoingRequest(outgoingRequest);
return new RustVerificationRequest(
request,
this.outgoingRequestProcessor,
this._supportedVerificationMethods,
);
await this.outgoingRequestProcessor.makeOutgoingRequest(outgoingRequest);
return new RustVerificationRequest(request, this.outgoingRequestProcessor, this._supportedVerificationMethods);
} finally {
userIdentity.free();
}
}

/**
Expand All @@ -1025,12 +1064,20 @@ export class RustCrypto extends TypedEventEmitter<RustCryptoEvents, RustCryptoEv
throw new Error("Not a known device");
}

const [request, outgoingRequest]: [RustSdkCryptoJs.VerificationRequest, RustSdkCryptoJs.ToDeviceRequest] =
await device.requestVerification(
this._supportedVerificationMethods.map(verificationMethodIdentifierToMethod),
try {
const [request, outgoingRequest]: [RustSdkCryptoJs.VerificationRequest, RustSdkCryptoJs.ToDeviceRequest] =
await device.requestVerification(
this._supportedVerificationMethods.map(verificationMethodIdentifierToMethod),
);
await this.outgoingRequestProcessor.makeOutgoingRequest(outgoingRequest);
return new RustVerificationRequest(
request,
this.outgoingRequestProcessor,
this._supportedVerificationMethods,
);
await this.outgoingRequestProcessor.makeOutgoingRequest(outgoingRequest);
return new RustVerificationRequest(request, this.outgoingRequestProcessor, this._supportedVerificationMethods);
} finally {
device.free();
}
}

/**
Expand Down

0 comments on commit a749662

Please sign in to comment.