Skip to content

Conversation

kun6fup4nd4
Copy link

@kun6fup4nd4 kun6fup4nd4 commented Jun 10, 2025

PR Type

Enhancement, Tests


Description

  • Added full support for detached Ed25519 signatures (signing and verification)

  • Introduced new functions: signDetached, verifyDetached, signObjDetached, verifyObjDetached

  • Updated sign and signObj to default to detached signatures, with backward compatibility

  • Added comprehensive unit tests for detached and legacy signatures


Changes walkthrough 📝

Relevant files
Enhancement
sodium-native.d.ts
Add typings for detached signature sodium-native functions

sodium-native.d.ts

  • Added type declarations for crypto_sign_detached and
    crypto_sign_verify_detached
  • Extended sodium-native typings for detached signature support
  • +2/-0     
    index.ts
    Implement detached signature support and update signing API

    src/index.ts

  • Added signDetached, verifyDetached, signObjDetached, verifyObjDetached
    functions
  • Updated sign and signObj to support and default to detached signatures
  • Implemented auto-detection of signature type in verification
  • Improved error handling and documentation for new and existing
    functions
  • +159/-10
    Tests
    detachedSignatures.test.ts
    Add unit tests for detached and legacy signature support 

    test/unit/detachedSignatures.test.ts

  • Added comprehensive tests for detached and legacy signature flows
  • Tested new API functions and backward compatibility
  • Verified default behavior and signature length differences
  • Checked error handling for mixed and invalid signature types
  • +162/-0 

    Need help?
  • Type /help how to ... in the comments thread for any questions about PR-Agent usage.
  • Check out the documentation for more information.
  • Copy link

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
    🏅 Score: 93
    🧪 PR contains tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Backward Compatibility

    The default behavior of the sign and signObj functions now returns detached signatures. Review all usages and documentation to ensure this change will not break existing integrations expecting the legacy (non-detached) signature format.

    export function sign(input: hexstring | Buffer, sk: secretKey | Buffer, detached = true): string {
      let inputBuf: Buffer
      let skBuf: Buffer
      if (typeof input !== 'string') {
        if (Buffer.isBuffer(input)) {
          inputBuf = input
        } else {
          throw new TypeError('Input must be a hex string or buffer.')
        }
      } else {
        try {
          inputBuf = Buffer.from(input, 'hex')
        } catch (e) {
          throw new TypeError('Input string must be in hex format.')
        }
      }
      if (typeof sk !== 'string') {
        if (Buffer.isBuffer(sk)) {
          skBuf = sk
        } else {
          throw new TypeError('Secret key must be a hex string or buffer.')
        }
      } else {
        try {
          skBuf = Buffer.from(sk, 'hex')
        } catch (e) {
          throw new TypeError('Secret key string must be in hex format')
        }
      }
      if (detached) {
        // Use detached signature
        const sig = Buffer.allocUnsafe(sodium.crypto_sign_BYTES)
        try {
          sodium.crypto_sign_detached(sig, inputBuf, skBuf)
        } catch (e) {
          throw new Error('Failed to sign input with provided secret key.')
        }
        return sig.toString('hex')
      } else {
        // Use non-detached signature (legacy behavior)
        const sig = Buffer.allocUnsafe(inputBuf.length + sodium.crypto_sign_BYTES)
        try {
          sodium.crypto_sign(sig, inputBuf, skBuf)
        } catch (e) {
          throw new Error('Failed to sign input with provided secret key.')
        }
        return sig.toString('hex')
      }
    }
    Signature Type Detection

    The verify function auto-detects signature type based on length. Ensure this logic is robust and cannot be bypassed or confused by malformed input, especially if message or signature lengths are manipulated.

    // Auto-detect signature type based on length
    // Detached signatures are exactly 64 bytes
    // Non-detached signatures are 64 bytes + message length
    const msgBuf = Buffer.from(msg, 'hex')
    const expectedNonDetachedLength = sodium.crypto_sign_BYTES + msgBuf.length
    
    if (sigBuf.length === sodium.crypto_sign_BYTES) {
      // This is a detached signature
      try {
        return sodium.crypto_sign_verify_detached(sigBuf as Buffer, msgBuf, pkBuf as Buffer)
      } catch (e) {
        return false
      }
    } else if (sigBuf.length === expectedNonDetachedLength) {
      // This is a non-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
      } catch (e) {
        return false
      }
    } else {
      // Invalid signature length
      throw new Error('Invalid signature length. Expected either detached (64 bytes) or non-detached signature.')
    }

    Comment on lines +523 to 540
    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
    }
    }

    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]

    Suggested change
    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 {

    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.
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    2 participants