Only enable EC groups that are supported by the token

RESOLVED FIXED in 3.28

Status

NSS
Libraries
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: mt, Assigned: mt)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 obsolete attachment)

(Assignee)

Description

a year ago
We discovered that the only part of softtoken that knows about keys is the bit that imports a key.  Therefore, if we import a key of a specific type, then it will succeed if the group is supported and fail otherwise.  We can use this to detect what groups are available to us.

Unfortunately, this is a little involved and probably slow, so it's probably best to only do this once.

This can be replaced by something less kludgy if PKCS#11 ever gets a better way to enumerate support for named groups.
(Assignee)

Comment 1

a year ago
Created attachment 8792461 [details] [diff] [review]
bug1303648-2.patch

Not sure why we are getting some of the curves show up as being present when they shouldn't be.  Investigation required, but that shouldn't stop this from getting a first pass.
Assignee: nobody → martin.thomson
Attachment #8792461 - Flags: review?(rrelyea)
Attachment #8792461 - Flags: review?(franziskuskiefer)
Comment on attachment 8792461 [details] [diff] [review]
bug1303648-2.patch

Review of attachment 8792461 [details] [diff] [review]:
-----------------------------------------------------------------

lgtm

::: lib/ssl/filterec.c
@@ +9,5 @@
> +#include "pk11func.h"
> +#include "ssl.h"
> +#include "sslimpl.h"
> +
> +/* This table contains valid public keys for each of the support EC groups we

... supported EC groups we support.

@@ +17,5 @@
> +    SSLNamedGroup name;
> +    const PRUint8 data[145];
> +    unsigned int len;
> +} ssl_test_public_keys[SSL_NAMED_GROUP_COUNT] = {
> +    { ssl_grp_ec_secp256r1,

add 25519

::: lib/ssl/ssl3ecc.c
@@ +64,5 @@
>  
> +    PORT_Assert(params);
> +    PORT_Assert(ecGroup);
> +    PORT_Assert(ecGroup->type == group_type_ec);
> +    if ((oidData = SECOID_FindOIDByTag(ecGroup->oidTag)) == NULL) {

I think we should keep those null checks. opt builds can run in a null ecGroup here otherwise.
Attachment #8792461 - Flags: review?(franziskuskiefer) → review+
(Assignee)

Comment 3

a year ago
Reviewed here: https://nss-dev.phacility.com/D6

https://hg.mozilla.org/projects/nss/rev/0f1a09d967b3
Status: NEW → RESOLVED
Last Resolved: a year ago
Resolution: --- → FIXED
Target Milestone: --- → 3.28
(Assignee)

Comment 4

a year ago
Comment on attachment 8792461 [details] [diff] [review]
bug1303648-2.patch

Overtaken by events.  This isn't that great.
Attachment #8792461 - Attachment is obsolete: true
Attachment #8792461 - Flags: review?(rrelyea)
You need to log in before you can comment on or make changes to this bug.