Skip to content

Conversation

@tniessen
Copy link
Member

@tniessen tniessen commented Jan 6, 2018

#17566 added a warning when invalid GCM tag lengths are used. As a preparation for #17825, assign a deprecation code before moving it to end-of-life.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)

crypto

@tniessen tniessen requested a review from joyeecheung January 6, 2018 14:40
@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. crypto Issues and PRs related to the crypto subsystem. labels Jan 6, 2018
@jasnell jasnell added the semver-major PRs that contain breaking changes and should be released in the next major version. label Jan 6, 2018
@tniessen
Copy link
Member Author

tniessen commented Jan 6, 2018

Node.js supports all GCM authentication tag lengths which are accepted by
OpenSSL when calling [`decipher.setAuthTag()`][]. This behavior will change in
a future version at which point only authentication tag lengths of 128, 120,
112, 104, 96, 64 and 32 bits will be allowed. Authentication tags whose length
Copy link
Member

Choose a reason for hiding this comment

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

Tiny nit: serial comma for consistency.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done, thank you!

@jasnell
Copy link
Member

jasnell commented Jan 9, 2018

Ping @nodejs/tsc

Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

LGTM

@joyeecheung
Copy link
Member

Previous CI stopped. New CI: https://ci.nodejs.org/job/node-test-pull-request/12488/

@tniessen tniessen added this to the 10.0.0 milestone Jan 11, 2018
@tniessen
Copy link
Member Author

tniessen added a commit that referenced this pull request Jan 14, 2018
PR-URL: #18017
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
@tniessen
Copy link
Member Author

Landed in 858b48b.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. crypto Issues and PRs related to the crypto subsystem. deprecations Issues and PRs related to deprecations. semver-major PRs that contain breaking changes and should be released in the next major version.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants