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.

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

Support authenticating via certfp #1757

Open
wants to merge 29 commits into
base: develop
Choose a base branch
from
Open

Conversation

Half-Shot
Copy link
Contributor

@Half-Shot Half-Shot commented Jul 28, 2023

Fixes #1483, #747

Requires matrix-org/node-irc#110

Most of the work was already done in node-irc, so this just needs the bridge to store and pass through the certificate. I haven't yet implemented a way for users to pass the cert to the bridge. I am wary of passing it through the admin room directly as it would leave the cert in Matrix.

Perhaps we may have a HTTP submission page of some kind.

src/datastore/postgres/PgDataStore.ts Outdated Show resolved Hide resolved
src/datastore/postgres/PgDataStore.ts Outdated Show resolved Hide resolved
src/datastore/postgres/PgDataStore.ts Outdated Show resolved Hide resolved
@Half-Shot Half-Shot marked this pull request as ready for review August 8, 2023 17:15
@Half-Shot Half-Shot requested a review from a team as a code owner August 8, 2023 17:15
src/datastore/StringCrypto.ts Outdated Show resolved Hide resolved
src/datastore/StringCrypto.ts Outdated Show resolved Hide resolved
src/datastore/StringCrypto.ts Show resolved Hide resolved
@Half-Shot Half-Shot enabled auto-merge (squash) August 22, 2023 18:20
Comment on lines +86 to +87
const password = randomBytes(32).toString(ENCRYPTED_ENCODING);
const key = await scrypt(password, 'salt', 32) as Buffer;
Copy link
Member

Choose a reason for hiding this comment

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

As long as you're generating a random password, there's probably not much of a point in passing to scrypt, as it can be used directly as an AES key.

Comment on lines +97 to +107
const streamPromise = new Promise<string>((resolve, reject) => {
cipher.on('error', (err) => reject(err));
cipher.on('end', () => resolve(
`lg:${secret}:${encrypted}`
));
});

cipher.on('data', (chunk) => { encrypted += chunk });
cipher.write(plaintext);
cipher.end();
return streamPromise;
Copy link
Member

Choose a reason for hiding this comment

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

Since the data to be written into the cipher is known to be just the provided plaintext, it'd be more direct to use cipher.update and cipher.final instead of bothering with stream callbacks.

Comment on lines +114 to +115
const [, keyPlusIvEnc, data] = encryptedString.split(':', 3);
const [keyB64, ivB64] = this.decrypt(keyPlusIvEnc).split('_');
Copy link
Member

Choose a reason for hiding this comment

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

Is it guaranteed that the ciphertexts of encrypt() or the AES cipher won't contain underscores or colons that would interfere with these splits?

@programmerjake
Copy link
Contributor

are you still intending on working on this? having storepass work on oftc would be really nice!

@progval
Copy link
Contributor

progval commented Jun 2, 2024

This is an alternative to !storepass, it won't make !storepass itself work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

!storepass doesn't work at all on OFTC
5 participants