Skip to content
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.

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

SOFT: Check the EC Key on C_CreateObject and C_DeriveKey #402

Merged
merged 1 commit into from May 11, 2021

Conversation

ifranzki
Copy link
Contributor

@ifranzki ifranzki commented May 5, 2021

When constructing an OpenSSL EC public or private key from PKCS#11 attributes or ECDH public data, check that the key is valid, i.e. that the point is on the curve.

This prevents one from creating an EC key object via C_CreateObject with invalid key data. It also prevents C_DeriveKey to derive a secret using ECDH with an EC public key (public data) that uses a different curve
or is invalid by other means.

Copy link
Contributor

@juergenchrist juergenchrist left a comment

Choose a reason for hiding this comment

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

lgtm

@@ -4365,6 +4365,19 @@ static CK_RV fill_ec_key_from_pubkey(EC_KEY *ec_key, const CK_BYTE *data,
goto out;
}

if (EC_POINT_is_on_curve(EC_KEY_get0_group(ec_key),
EC_KEY_get0_public_key(ec_key), NULL) != 1) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This check is also done by EC_KEY_check_type as far as I can see in the code. However, since this is not documented (or I just could not find it), I would keep this check here and below.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right, that was also my thinking when I coded that. EC_KEY_check_key should actually also check if the public key point is on the curve, but the documentation does not state that explicitly. So I also thought that its better to leave it in, even if the same thing is checked twice.

When constructing an OpenSSL EC public or private key from PKCS#11
attributes or ECDH public data, check that the key is valid, i.e. that
the point is on the curve.

This prevents one from creating an EC key object via C_CreateObject with
invalid key data. It also prevents C_DeriveKey to derive a secret using
ECDH with an EC public key (public data) that uses a different curve
or is invalid by other means.

Signed-off-by: Ingo Franzki <ifranzki@linux.ibm.com>
@ifranzki
Copy link
Contributor Author

ifranzki commented May 7, 2021

According to @p-steuer EC_KEY_check_key() is sufficient, and we should not do an additional EC_POINT_is_on_curve() check, since there is a cost associated with doing checking this twice.

I have changed the code accordingly.

@p-steuer p-steuer merged commit 4e3b43c into opencryptoki:master May 11, 2021
@ifranzki ifranzki deleted the soft-ec-point-check branch July 12, 2021 11:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants