-
Notifications
You must be signed in to change notification settings - Fork 4
removed unused suffix from signature #21
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
base: SHARD-2023
Are you sure you want to change the base?
Conversation
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
export function verifyDetached(msg: string, sig: hexstring | Buffer, pk: publicKey | Buffer): boolean { | ||
if (typeof msg !== 'string') { | ||
throw new TypeError('Message to compare must be a string.') | ||
} | ||
const msgBuf = Buffer.from(msg, 'hex') | ||
const sigBuf = _ensureBuffer(sig) | ||
const pkBuf = _ensureBuffer(pk) | ||
|
||
if (sigBuf.length !== sodium.crypto_sign_BYTES) { | ||
throw new Error('Invalid signature length for detached signature.') | ||
} | ||
|
||
try { | ||
const opened = Buffer.allocUnsafe(sigBuf.length - sodium.crypto_sign_BYTES) | ||
sodium.crypto_sign_open(opened, sigBuf as Buffer, pkBuf as Buffer) | ||
const verified = opened.toString('hex') | ||
return verified === msg | ||
return sodium.crypto_sign_verify_detached(sigBuf as Buffer, msgBuf, pkBuf as Buffer) | ||
} catch (e) { | ||
throw new Error('Unable to verify provided signature with provided public key.') | ||
return false | ||
} | ||
} |
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.
Suggestion: Throwing an error for invalid signature length may allow attackers to distinguish between valid and invalid signatures, potentially leaking information. Instead, return false for invalid signature lengths to avoid information leakage. [security, importance: 8]
export function verifyDetached(msg: string, sig: hexstring | Buffer, pk: publicKey | Buffer): boolean { | |
if (typeof msg !== 'string') { | |
throw new TypeError('Message to compare must be a string.') | |
} | |
const msgBuf = Buffer.from(msg, 'hex') | |
const sigBuf = _ensureBuffer(sig) | |
const pkBuf = _ensureBuffer(pk) | |
if (sigBuf.length !== sodium.crypto_sign_BYTES) { | |
throw new Error('Invalid signature length for detached signature.') | |
} | |
try { | |
const opened = Buffer.allocUnsafe(sigBuf.length - sodium.crypto_sign_BYTES) | |
sodium.crypto_sign_open(opened, sigBuf as Buffer, pkBuf as Buffer) | |
const verified = opened.toString('hex') | |
return verified === msg | |
return sodium.crypto_sign_verify_detached(sigBuf as Buffer, msgBuf, pkBuf as Buffer) | |
} catch (e) { | |
throw new Error('Unable to verify provided signature with provided public key.') | |
return false | |
} | |
} | |
export function verifyDetached(msg: string, sig: hexstring | Buffer, pk: publicKey | Buffer): boolean { | |
if (typeof msg !== 'string') { | |
throw new TypeError('Message to compare must be a string.') | |
} | |
const msgBuf = Buffer.from(msg, 'hex') | |
const sigBuf = _ensureBuffer(sig) | |
const pkBuf = _ensureBuffer(pk) | |
if (sigBuf.length !== sodium.crypto_sign_BYTES) { | |
return false | |
} | |
try { | |
return sodium.crypto_sign_verify_detached(sigBuf as Buffer, msgBuf, pkBuf as Buffer) | |
} catch (e) { | |
return false | |
} | |
} |
* @param detached - If true (default), returns only the signature (64 bytes). If false, returns signature + message | ||
*/ | ||
export function sign(input: hexstring | Buffer, sk: secretKey | Buffer): string { | ||
export function sign(input: hexstring | Buffer, sk: secretKey | Buffer, detached = true): 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.
Suggestion: The function currently returns a hex string for both detached and non-detached signatures, but the non-detached signature contains both the signature and the message. If the input is sensitive, ensure that returning the concatenated signature and message does not leak unintended information. Consider documenting this behavior clearly to prevent misuse. [general, importance: 6]
Refactors the signing and verification logic to default to detached signatures for improved security and efficiency. Adds functionality to manually convert non-detached signatures to detached signatures, ensuring compatibility and flexibility in signature management. Updates tests to reflect the new default behavior and conversion capabilities.
7714612
to
14f302d
Compare
PR Type
Enhancement, Tests
Description
Added full support for detached Ed25519 signatures (signing and verification)
Introduced new functions:
signDetached
,verifyDetached
,signObjDetached
,verifyObjDetached
Updated
sign
andsignObj
to default to detached signatures, with backward compatibilityAdded comprehensive unit tests for detached and legacy signatures
Changes walkthrough 📝
sodium-native.d.ts
Add typings for detached signature sodium-native functions
sodium-native.d.ts
crypto_sign_detached
andcrypto_sign_verify_detached
index.ts
Implement detached signature support and update signing API
src/index.ts
signDetached
,verifyDetached
,signObjDetached
,verifyObjDetached
functions
sign
andsignObj
to support and default to detached signaturesfunctions
detachedSignatures.test.ts
Add unit tests for detached and legacy signature support
test/unit/detachedSignatures.test.ts