crypto,doc: update language regarding key stretching #19810

ryzokuken · 2018-04-04T20:52:26Z

Update the docs to provide clearer instructions regarding the exact scope
of the use (and re-use) of an IV, stating the instructions explicitly with
greater clarity.

Fixes: #19748

Checklist

documentation is changed or added
commit message follows commit guidelines

/cc @bnoordhuis @tniessen

tniessen · 2018-04-04T20:55:50Z

doc/api/crypto.md

@@ -1365,6 +1365,8 @@ The `key` is the raw key used by the `algorithm` and `iv` is an
 [Buffers][`Buffer`], `TypedArray`, or `DataView`s. If the cipher does not need
 an initialization vector, `iv` may be `null`.

+An initialization vector should be unpredictable; ideally, they will be cryptographically random. They do not have to be secret: IVs are typically just added to ciphertext messages in plaintext. It may sound contradictory that something has to be unpredictable, but doesn’t have to be secret; it’s important to remember that an attacker must not be able to predict ahead of time what a given IV will be.


Lines need to be wrapped at 80 characters.

aren't they? strange. I'll do that promptly, sorry.

oh, wait. It's just one single huge line.

tniessen · 2018-04-04T20:55:55Z

doc/api/crypto.md

@@ -1437,6 +1439,8 @@ The `key` is the raw key used by the `algorithm` and `iv` is an
 [Buffers][`Buffer`], `TypedArray`, or `DataView`s. If the cipher does not need
 an initialization vector, `iv` may be `null`.

+An initialization vector should be unpredictable; ideally, they will be cryptographically random. They do not have to be secret: IVs are typically just added to ciphertext messages in plaintext. It may sound contradictory that something has to be unpredictable, but doesn’t have to be secret; it’s important to remember that an attacker must not be able to predict ahead of time what a given IV will be.


vsemozhetbyt · 2018-04-04T21:00:33Z

doc/api/crypto.md

@@ -1365,6 +1365,8 @@ The `key` is the raw key used by the `algorithm` and `iv` is an
 [Buffers][`Buffer`], `TypedArray`, or `DataView`s. If the cipher does not need
 an initialization vector, `iv` may be `null`.

+An initialization vector should be unpredictable; ideally, they will be cryptographically random. They do not have to be secret: IVs are typically just added to ciphertext messages in plaintext. It may sound contradictory that something has to be unpredictable, but doesn’t have to be secret; it’s important to remember that an attacker must not be able to predict ahead of time what a given IV will be.


An initialization vector -> Initialization vectors?
Or
they will be -> it will be, They do not have -> It does not have?

ryzokuken · 2018-04-04T21:07:22Z

@tniessen @vsemozhetbyt fixed.

vsemozhetbyt · 2018-04-04T21:24:52Z

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

tniessen · 2018-04-04T21:43:30Z

doc/api/crypto.md

@@ -1365,6 +1365,13 @@ The `key` is the raw key used by the `algorithm` and `iv` is an
 [Buffers][`Buffer`], `TypedArray`, or `DataView`s. If the cipher does not need
 an initialization vector, `iv` may be `null`.

+Initialization vectors should be unpredictable; ideally, they will be


This really depends on the cipher block mode, some work fine even with predictable IVs. What is much more important is that IVs are unique and that their value cannot be influenced by an attacker.

You're right. How should I better communicate this, in your opinion?

I (among a lot of other people, I'm sure), prefer to keep the IV unpredictable (cryptographically random) anyway. Better be safe than be sorry, I guess.

Maybe should be unpredictable and unique? That's not always true, but as you said, better safe than sorry.

Sure, I'll add unique. I think that was exactly what @bnoordhuis wanted to communicate as well.

tniessen · 2018-04-04T21:44:37Z

doc/api/crypto.md

+cryptographically random. They do not have to be secret: IVs are typically just
+added to ciphertext messages in plaintext. It may sound contradictory that
+something has to be unpredictable, but doesn’t have to be secret; it’s
+importantto remember that an attacker must not be able to predict ahead of time


Missing space in importantto. Actually, I am not sure what this sentence is supposed to say apart from what the previous said.

It may sound contradictory that something has to be unpredictable, but does not have to be secret; it is important to remember that an attacker must not be able to predict ahead of time what a given IV will be.

Emphasis on ahead of time here. While the IV does not have to be (and mostly isn't) secret, It needs to be unpredictable. It can be as obvious as it could be in hindsight, but it's important that the attack should not be able to predict it ahead of time.

I hope that makes my stance clearer? I'm willing to chop this down if you don't find it up to the mark?

No strong opinion here, I am okay with or without the sentence. (Just remember that it is not entirely true for all block cipher modes.)

My personal standard here is that in case of ambiguity (here: the exact block cipher being used), we should go with the most secure solution which would work well with all of the alternatives, and I think the current definition works for all of the ciphers we're supporting.

That said, my knowledge of both the crypto subsystem and the Node crypto ecosystem are substantially less than yours, and I'd trust your instincts on this.

tniessen · 2018-04-04T21:45:44Z

doc/api/crypto.md

+Initialization vectors should be unpredictable; ideally, they will be
+cryptographically random. They do not have to be secret: IVs are typically just
+added to ciphertext messages in plaintext. It may sound contradictory that
+something has to be unpredictable, but doesn’t have to be secret; it’s


doesn't → does not, it's → it is.

bnoordhuis · 2018-04-05T09:10:35Z

doc/api/crypto.md

@@ -1365,6 +1365,13 @@ The `key` is the raw key used by the `algorithm` and `iv` is an
 [Buffers][`Buffer`], `TypedArray`, or `DataView`s. If the cipher does not need
 an initialization vector, `iv` may be `null`.

+Initialization vectors should be unpredictable; ideally, they will be
+cryptographically random. They do not have to be secret: IVs are typically just
+added to ciphertext messages in plaintext. It may sound contradictory that


Maybe don't say 'plaintext' here, that has a specific meaning in crypto speak.

Okay. I meant, it's sent "in the plain", or more precisely, unencrypted. I'll substitute "in plaintext" with "unencrypted". Is that better?

tniessen · 2018-04-05T10:18:27Z

doc/api/crypto.md

+Initialization vectors should be unpredictable; ideally, they will be
+cryptographically random. They do not have to be secret: IVs are typically just
+added to ciphertext messages unencrypted. It may sound contradictory that
+something has to be unpredictable and unique, but does not have to be secret;


Could you move and unique to the beginning (the first sentence)? It really is the most important property of an IV, compared to that, predictability has very limited attack vectors.

It doesn't make too much sense not using the word unique in this sentence, does it? I'll definitely add it to the beginning as well.

It doesn't hurt to have it twice, but it should definitely be in the beginning 👍

Update the docs to provide clearer instructions regarding the exact scope of the use (and re-use) of an IV, stating the instructions explicitly with greater clarity. Fixes: nodejs#19748

tniessen · 2018-04-05T10:53:26Z

(Subsystem should probably just be doc.)

ryzokuken · 2018-04-05T10:56:33Z

I agree. I had been thinking the same, but went with both because of the original issue.

…

On Thu 5 Apr, 2018, 4:24 PM Tobias Nießen, ***@***.***> wrote: (Subsystem should probably just be doc.) — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#19810 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AMg3Mq-Is6P_L3vXoMX2sBtXGDrih-Kcks5tlffZgaJpZM4THe0k> .

tniessen · 2018-04-05T10:59:27Z

New CI: https://ci.nodejs.org/job/node-test-pull-request-lite/433/

ryzokuken · 2018-04-06T16:18:34Z

@tniessen is this landable now?

tniessen · 2018-04-06T17:19:42Z

@ryzokuken Yes, as soon as 48 hours have passed :)

ryzokuken · 2018-04-06T17:25:01Z

Great!

…

On Fri 6 Apr, 2018, 10:50 PM Tobias Nießen, ***@***.***> wrote: @ryzokuken <https://github.com/ryzokuken> Yes, as soon as 48 hours have passed :) — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#19810 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AMg3MkIgInhEV_MEnZYAIPuXciHBpBflks5tl6PzgaJpZM4THe0k> .

tniessen · 2018-04-08T11:33:05Z

Landed in 0a67932.

Update the docs to provide clearer instructions regarding the exact scope of the use (and re-use) of an IV, stating the instructions explicitly with greater clarity. PR-URL: #19810 Fixes: #19748 Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de>

nodejs-github-bot added crypto Issues and PRs related to the crypto subsystem. doc Issues and PRs related to the documentations. labels Apr 4, 2018

tniessen reviewed Apr 4, 2018

View reviewed changes

vsemozhetbyt reviewed Apr 4, 2018

View reviewed changes

ryzokuken force-pushed the docs/crypto-iv branch from 319f357 to 553e1bb Compare April 4, 2018 21:05

vsemozhetbyt approved these changes Apr 4, 2018

View reviewed changes

vsemozhetbyt added fast-track PRs that do not need to wait for 48 hours to land. author ready PRs that have at least one approval, no pending requests for changes, and a CI started. labels Apr 4, 2018

tniessen reviewed Apr 4, 2018

View reviewed changes

tniessen removed author ready PRs that have at least one approval, no pending requests for changes, and a CI started. fast-track PRs that do not need to wait for 48 hours to land. labels Apr 4, 2018

ryzokuken force-pushed the docs/crypto-iv branch 2 times, most recently from b944ecc to e283a51 Compare April 4, 2018 22:05

jasnell approved these changes Apr 4, 2018

View reviewed changes

bnoordhuis reviewed Apr 5, 2018

View reviewed changes

ryzokuken force-pushed the docs/crypto-iv branch from e283a51 to 24bbcdb Compare April 5, 2018 10:09

tniessen reviewed Apr 5, 2018

View reviewed changes

crypto,doc: update language regarding key stretching

7a881d9

Update the docs to provide clearer instructions regarding the exact scope of the use (and re-use) of an IV, stating the instructions explicitly with greater clarity. Fixes: nodejs#19748

ryzokuken force-pushed the docs/crypto-iv branch from 24bbcdb to 7a881d9 Compare April 5, 2018 10:47

tniessen approved these changes Apr 5, 2018

View reviewed changes

tniessen added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Apr 5, 2018

tniessen closed this Apr 8, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

crypto,doc: update language regarding key stretching #19810

crypto,doc: update language regarding key stretching #19810

ryzokuken commented Apr 4, 2018

tniessen Apr 4, 2018

ryzokuken Apr 4, 2018

ryzokuken Apr 4, 2018 •

edited

Loading

tniessen Apr 4, 2018

vsemozhetbyt Apr 4, 2018 •

edited

Loading

ryzokuken commented Apr 4, 2018

vsemozhetbyt commented Apr 4, 2018

tniessen Apr 4, 2018

ryzokuken Apr 4, 2018

tniessen Apr 4, 2018

ryzokuken Apr 4, 2018

tniessen Apr 4, 2018

ryzokuken Apr 4, 2018 •

edited

Loading

tniessen Apr 4, 2018

ryzokuken Apr 4, 2018

tniessen Apr 4, 2018

ryzokuken Apr 4, 2018

bnoordhuis Apr 5, 2018

ryzokuken Apr 5, 2018

tniessen Apr 5, 2018

ryzokuken Apr 5, 2018

tniessen Apr 5, 2018

tniessen commented Apr 5, 2018

ryzokuken commented Apr 5, 2018 via email

tniessen commented Apr 5, 2018

ryzokuken commented Apr 6, 2018

tniessen commented Apr 6, 2018

ryzokuken commented Apr 6, 2018 via email

tniessen commented Apr 8, 2018

crypto,doc: update language regarding key stretching #19810

crypto,doc: update language regarding key stretching #19810

Conversation

ryzokuken commented Apr 4, 2018

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ryzokuken Apr 4, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vsemozhetbyt Apr 4, 2018 • edited Loading

Choose a reason for hiding this comment

ryzokuken commented Apr 4, 2018

vsemozhetbyt commented Apr 4, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ryzokuken Apr 4, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tniessen commented Apr 5, 2018

ryzokuken commented Apr 5, 2018 via email

tniessen commented Apr 5, 2018

ryzokuken commented Apr 6, 2018

tniessen commented Apr 6, 2018

ryzokuken commented Apr 6, 2018 via email

tniessen commented Apr 8, 2018

ryzokuken Apr 4, 2018 •

edited

Loading

vsemozhetbyt Apr 4, 2018 •

edited

Loading

ryzokuken Apr 4, 2018 •

edited

Loading