Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

crypto: assign deprecation code for setAuthTag/GCM #18017

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.

Sign up for GitHub

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

Jump to bottom
Closed
tniessen wants to merge 2 commits into from
Closed

crypto: assign deprecation code for setAuthTag/GCM #18017

tniessen wants to merge 2 commits into from

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

CI: https://ci.nodejs.org/job/node-test-pull-request/12437/

lpinca
lpinca approved these changes Jan 7, 2018
View reviewed changes
doc/api/deprecations.md Outdated
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

@lpinca lpinca Jan 7, 2018

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

@tniessen tniessen Jan 7, 2018

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

mcollina
mcollina approved these changes Jan 10, 2018
View reviewed changes
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

joyeecheung commented Jan 10, 2018

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 commented Jan 14, 2018

Pre-land CI: https://ci.nodejs.org/job/node-test-pull-request/12525/

tniessen added a commit that referenced this pull request Jan 14, 2018
@tniessen
crypto: assign deprecation code for setAuthTag/GCM
858b48b 
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

tniessen commented Jan 14, 2018

Landed in 858b48b.

@tniessen tniessen closed this Jan 14, 2018
This was referenced Jan 17, 2018
Closed
Closed
@tniessen tniessen mentioned this pull request Feb 4, 2018
Closed
4 tasks
@tniessen tniessen added the deprecations Issues and PRs related to deprecations. label Apr 28, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mcollina mcollina mcollina approved these changes

@jasnell jasnell jasnell approved these changes

@lpinca lpinca lpinca approved these changes

@targos targos targos approved these changes

@joyeecheung joyeecheung joyeecheung approved these changes

Assignees

No one assigned

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

Milestone

10.0.0

Development

Successfully merging this pull request may close these issues.

7 participants

@tniessen @jasnell @joyeecheung @mcollina @lpinca @targos @nodejs-github-bot
Morty Proxy This is a proxified and sanitized view of the page, visit original site.