Skip to content

Runtime-deprecate calling digest() on HMAC more than once #62838

Description

@ChALkeR

(Upd: detected by the scanner behind @deepview-autofix)

Hash behavior is reasonable:

> hash = require('crypto').createHash('sha256').update('data')
> hash.digest()
<Buffer 3a 6e b0 79 0f 39 ac 87 c9 4f 38 56 b2 dd 2c 5d 11 0e 68 11 60 22 61 a9 a9 23 d3 bb 23 ad c8 b7>
> hash.digest()
Uncaught Error [ERR_CRYPTO_HASH_FINALIZED]: Digest already called
    at Hash.digest (node:internal/crypto/hash:155:11) {
  code: 'ERR_CRYPTO_HASH_FINALIZED'
}

But HMAC, on the other hand, returns empty buffers on further .digest() calls, likely for compat reasons:

> hmac = require('crypto').createHmac('sha256', 'key').update('data')
> hmac.digest()
<Buffer 50 31 fe 3d 98 9c 6d 15 37 a0 13 fa 6e 73 9d a2 34 63 fd ae c3 b7 01 37 d8 28 e3 6a ce 22 1b d0>
> hmac.digest()
<Buffer >

This is a footgun with potential security risks, and should be first runtime-deprecated, then removed if no breakage is detected.


Image

Activity

  1. added
    securityIssues and PRs related to security.
    cryptoIssues and PRs related to the crypto subsystem.
    on Apr 20, 2026
  2. panva commented on Apr 20, 2026

    @panva
    Member

    Given the lack of usefulness of the second digest() call's return (and in so the likelyhood of it being depended on), shouldn't we treat this as a bug fix?

  3. panva commented on Apr 20, 2026

    @panva
    Member

    The behaviour looks entirely deliberate

    if (state[kFinalized]) {
    const buf = Buffer.from('');
    if (outputEncoding && outputEncoding !== 'buffer')
    return buf.toString(outputEncoding);
    return buf;
    }

    but looking at the blame its introduction traces back to #15231 by @jasnell, its parent was just throwing the same as Hash

    Hmac.prototype.update = Hash.prototype.update;

  4. jasnell commented on Apr 20, 2026

    @jasnell
    Member

    Oh, we reaching way back. I don't remember what the reasoning here was. I'm +1 on deprecation of the inconsistent behavior

  5. panva commented on Apr 20, 2026

    @panva
    Member

    Test Wayback machine: #14122, a3b9f4b

    Add a test that ensures the second call to .digest() returns an empty
    HMAC, like it did before. No comment on whether that is the right
    behavior or not.

  6. panva commented on Apr 20, 2026

    @panva
    Member

    Yeah, +1 deprecate it.

  7. ChALkeR commented on Apr 20, 2026

    @ChALkeR
    MemberAuthor

    @panva this was also auto-detected by the scanner behind @deepview-autofix btw.

    But this in fact looks intentional so I would recommend removing this in a major as a safeguard, following a deprecation cycle.

    Upd: github glitched, I replied to #62838 (comment), all the other comments (end the edit on that one) just loaded. I agree with those.

  8. panva commented on Apr 20, 2026

    @panva
    Member

    Enough time to land a semver-major PRs that contain breaking changes and should be released in the next major version. runtime-deprecation for v27.x and then soonest with v28.x EOL the deprecation.

  9. mizoz commented on Apr 28, 2026

    @mizoz

    Hi! I'd love to contribute. Could you assign this issue to me?

  10. Anshikakalpana commented on May 4, 2026

    @Anshikakalpana
    Contributor

    Hi! I’d like to work on this issue. I’m preparing a PR for it.

  11. soulee-dev commented on May 13, 2026

    @soulee-dev
    Contributor

    @Anshikakalpana Left review suggestions for the CI failure.

    expectWarning is keyed by warning name, so the inline DEP0181 block at the bottom of test-crypto-hmac.js overwrites the new DEP0206 registration. Merging them at the top resolves it.

    #63162 (review)

  12. Aanushka001 commented on May 24, 2026

    @Aanushka001

    Fetching comments for issue #62838...

  13. removed
    good first issueIssues that are suitable for first-time contributors.
    on Jun 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    cryptoIssues and PRs related to the crypto subsystem.securityIssues and PRs related to security.

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions