runtime: HKDF: Expand and DeriveKey throw invalid exceptions when outputLength is negative

Methods Expand and DeriveKey of the System.Security.Cryptography.HKDF class throw invalid exceptions when the argument outputLength has negative value.

In this example Expand throws ArgumentOutOfRangeException with the message Output keying material length can be at most 8160 bytes (255 * hash length).:

HKDF.Expand(HashAlgorithmName.SHA256, prk: new byte[32], outputLength: -1);

Instead the exception message should say that outputLength can’t be negative (or that it must be positive - depends on whether 0 is considered a valid input).

Here DeriveKey throws OverflowException with the message Arithmetic operation resulted in an overflow.:

HKDF.DeriveKey(HashAlgorithmName.SHA256, ikm: new byte[32], outputLength: -1);

Instead the type of exception should be ArgumentOutOfRangeException and the message should say that the outputLength can’t be negative (or that it must be positive - depends on whether 0 is considered a valid input).

Windows 10 x64 Pro, dotnet 5.0.0-preview.8.

About this issue

Original URL
State: closed
Created 4 years ago
Comments: 15 (12 by maintainers)

Most upvoted comments

@tonycimaglia Excellent!

Have a look at the Workflow Guide for instructions about getting dotnet/runtime building and running locally, the OS table has a link to building requirements. If everything is set up correctly, you should be able to do build.cmd -rc Release -s clr+libs at the repository root. (Substitute build.cmd for build.sh if you are on macOS / Linux). You need to do this build from the command line once before the Visual Studio solution files will work.

If you plan to develop and test in Visual Studio, see the Visual Studio Workflow for some info on getting tests running.

Also, take a look at the Building Libraries for more info about building and testing libraries from the Command Line.

The HKDF implementation is here:

https://github.com/dotnet/runtime/blob/a1f9226e7b7689aa153861b7bb5011e6f272ccc2/src/libraries/System.Security.Cryptography.Algorithms/src/System/Security/Cryptography/HKDF.cs#L49-L51

and the tests are here:

https://github.com/dotnet/runtime/blob/a1f9226e7b7689aa153861b7bb5011e6f272ccc2/src/libraries/System.Security.Cryptography.Algorithms/tests/HKDFTests.cs#L13

Before you start making changes it’d be a good idea to make sure that all of the tests in ‘System.Security.Cryptography.Algorithms’ are green.

vcsjones on Sep 17, 2020

Hi, I am a first-time contributor and I would love to take a shot at this.

tonycimaglia on Sep 17, 2020

@ADustyOldMuffin Just in case you missed the in the scroll, fixing #42230 will likely address this one at the same time (but we should make sure tests cover both cases).

vcsjones on May 4, 2021

Here’s an example of another method checking that a parameter is positive:

https://github.com/dotnet/runtime/blob/1b491f603275a0d943d613da1e914502b9102e0f/src/libraries/System.Security.Cryptography.Algorithms/src/System/Security/Cryptography/RandomNumberGenerator.cs#L143-L144

For testing ArgumentExceptions and derived exceptions, we have a custom assertion helper that should be used that also asserts the ParamName of the exception. Here is an example:

https://github.com/dotnet/runtime/blob/0ed3f3311e4d07cf6840b78c32055ab1b16d092c/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/DSA/DsaFamilySignatureFormatTests.cs#L315-L317

It’s new-ish so it isn’t used consistently in all of the unit tests, but it should be used going forward.

vcsjones on Sep 19, 2020

@bartonjs / @krwq (I don’t have assign rights)

vcsjones on Sep 17, 2020