-
Notifications
You must be signed in to change notification settings - Fork 26
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
Feat/add argon2 and kyber #1291
base: PB-2666-implement-pqc-on-drive
Are you sure you want to change the base?
Feat/add argon2 and kyber #1291
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Quality Gate passedIssues Measures |
@TamaraFinogina Please provide a description |
const messageHex = Buffer.from(message).toString('hex'); | ||
|
||
// message should be the same length as secret, which is 256 bits | ||
const xoredMessage = XORhex(messageHex, secretHex); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will they always be the same length?
Maybe we should check in case they are not, so that it throws an error.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, for XOR to work, both inputs must be the same length. Which error should be thrown?
try { | ||
const publicKeyResponse = await userService.getPublicKeyByEmail(email); | ||
publicKey = publicKeyResponse.publicKey; | ||
publicKyberKey = publicKeyResponse.publicKyberKey; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the backend is already prepared to store this new key?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't think so
isThereAnyNewUser = true; | ||
} | ||
usersList.push({ | ||
email, | ||
userRole, | ||
isNewUser: !publicKey, | ||
isNewUser: !publicKey || !publicKyberKey, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The user will have both keys always?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's the part I'm still thinking about - migration, when we only will have 1 key out of 2
}); | ||
|
||
expect(encryptedMessage).toBeDefined(); | ||
}); | ||
|
||
it('xor should work', async () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is too generic a test, separate it into different tests in which the description of what it has to achieve is a little more specific
"editor.formatOnSave": true, | ||
"editor.defaultFormatter": "esbenp.prettier-vscode", | ||
"editor.codeActionsOnSave": { | ||
"source.fixAll.eslint": "explicit", | ||
"source.fixAll.format": "explicit" | ||
}, | ||
"svg.preview.background": "editor" | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This has to be added to the gitignore, as it is part of the vs code configuration.
"preinstall": "node scripts/use-yarn.js", | ||
"prepare": "husky install", | ||
"dev": "craco start", | ||
"start": "craco start", | ||
"build": "craco build", | ||
"vercel:install": "yarn run add:npmrc && yarn install", | ||
"test:unit": "jest src/ && jest test/unit", | ||
"test:unit": "vitest run test/unit && vitest run src/ ", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting change, with Vitest the problems with global meta variable has gone?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
meta imports you mean? Yes.
}; | ||
} | ||
export function XORhex(a: string, b: string): string { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe it would be good to add jsdoc for this function.
let res = '', | ||
i = a.length, | ||
j = b.length; | ||
while (i-- > 0 && j-- > 0) res = (parseInt(a.charAt(i), 16) ^ parseInt(b.charAt(j), 16)).toString(16) + res; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if the lengths of the chains are different, this will end before they have travelled the longest chain. Is this the desired behaviour?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inputs should always be of identical length
Update
Related to PB-2666