Remove dead code in ECPrivateKey.

Remove dead code in ECPrivateKey.

None of these functions are used.

BUG=none

Committed: https://crrev.com/f2331cd02a333399a45e084339532c4bb466fa2a
Cr-Commit-Position: refs/heads/master@{#318117}

[davidben, 2015-02-24 01:08:46]

davidben@chromium.org changed reviewers:
+ rsleevi@chromium.org

[Ryan Sleevi, 2015-02-24 01:30:56]

LGTM, but note the asymmetry with RSAPrivateKey

[wfa511com, 2015-02-24 01:48:38]

بتاريخ Feb 24, 2015 4:08 AM، كتبها <davidben@chromium.org>:

> Reviewers: Ryan Sleevi,
>
> Description:
> Remove dead code in ECPrivateKey.
>
> None of these functions are used.
>
> BUG=none
>
> Please review this at https://codereview.chromium.org/951753002/
>
> Base URL: https://chromium.googlesource.com/chromium/src.git@master
>
> Affected files (+1, -47 lines):
>   M crypto/ec_private_key.h
>   M crypto/ec_private_key_nss.cc
>
>
> Index: crypto/ec_private_key.h
> diff --git a/crypto/ec_private_key.h b/crypto/ec_private_key.h
> index 9fd8ffdfb62230ac2dac10afb56818baba96bd8e..
> cd46d44dbe99c3adf2c1dda5cd6640a8171c4d19 100644
> --- a/crypto/ec_private_key.h
> +++ b/crypto/ec_private_key.h
> @@ -42,13 +42,6 @@ class CRYPTO_EXPORT ECPrivateKey {
>    // TODO(mattm): Add a curve parameter.
>    static ECPrivateKey* Create();
>
> -#if defined(USE_NSS)
> -  // Creates a new random instance in |slot|. Can return NULL if
> initialization
> -  // fails.  The created key is permanent and is not exportable in
> plaintext
> -  // form.
> -  static ECPrivateKey* CreateSensitive(PK11SlotInfo* slot);
> -#endif
> -
>    // Creates a new instance by importing an existing key pair.
>    // The key pair is given as an ASN.1-encoded PKCS #8
> EncryptedPrivateKeyInfo
>    // block and an X.509 SubjectPublicKeyInfo block.
> @@ -58,19 +51,6 @@ class CRYPTO_EXPORT ECPrivateKey {
>        const std::vector<uint8>& encrypted_private_key_info,
>        const std::vector<uint8>& subject_public_key_info);
>
> -#if defined(USE_NSS)
> -  // Creates a new instance in |slot| by importing an existing key pair.
> -  // The key pair is given as an ASN.1-encoded PKCS #8
> EncryptedPrivateKeyInfo
> -  // block and an X.509 SubjectPublicKeyInfo block.
> -  // This can return NULL if initialization fails.  The created key is
> permanent
> -  // and is not exportable in plaintext form.
> -  static ECPrivateKey* CreateSensitiveFromEncryptedPrivateKeyInfo(
> -      PK11SlotInfo* slot,
> -      const std::string& password,
> -      const std::vector<uint8>& encrypted_private_key_info,
> -      const std::vector<uint8>& subject_public_key_info);
> -#endif
> -
>  #if !defined(USE_OPENSSL)
>    // Imports the key pair into |slot| and returns in |public_key| and
> |key|.
>    // Shortcut for code that needs to keep a reference directly to NSS
> types
> @@ -130,8 +110,7 @@ class CRYPTO_EXPORT ECPrivateKey {
>                                          bool permanent,
>                                          bool sensitive);
>
> -  // Shared helper for CreateFromEncryptedPrivateKeyInfo() and
> -  // CreateSensitiveFromEncryptedPrivateKeyInfo().
> +  // Helper for CreateFromEncryptedPrivateKeyInfo().
>    static ECPrivateKey* CreateFromEncryptedPrivateKeyInfoWithParams(
>        PK11SlotInfo* slot,
>        const std::string& password,
> Index: crypto/ec_private_key_nss.cc
> diff --git a/crypto/ec_private_key_nss.cc b/crypto/ec_private_key_nss.cc
> index 2daa0c0f4a38642916981996440a084d09f8adc5..
> 21532a4ae57d7784235464cc1f1ca01b46051981 100644
> --- a/crypto/ec_private_key_nss.cc
> +++ b/crypto/ec_private_key_nss.cc
> @@ -96,14 +96,6 @@ ECPrivateKey* ECPrivateKey::Create() {
>                            false /* not sensitive */);
>  }
>
> -#if defined(USE_NSS)
> -// static
> -ECPrivateKey* ECPrivateKey::CreateSensitive(PK11SlotInfo* slot) {
> -  return CreateWithParams(
> -      slot, true /* permanent */, true /* sensitive */);
> -}
> -#endif
> -
>  // static
>  ECPrivateKey* ECPrivateKey::CreateFromEncryptedPrivateKeyInfo(
>      const std::string& password,
> @@ -121,23 +113,6 @@ ECPrivateKey* ECPrivateKey::
> CreateFromEncryptedPrivateKeyInfo(
>        false /* not sensitive */);
>  }
>
> -#if defined(USE_NSS)
> -// static
> -ECPrivateKey* ECPrivateKey::CreateSensitiveFromEncryptedPrivateKeyInfo(
> -    PK11SlotInfo* slot,
> -    const std::string& password,
> -    const std::vector<uint8>& encrypted_private_key_info,
> -    const std::vector<uint8>& subject_public_key_info) {
> -  return CreateFromEncryptedPrivateKeyInfoWithParams(
> -      slot,
> -      password,
> -      encrypted_private_key_info,
> -      subject_public_key_info,
> -      true /* permanent */,
> -      true /* sensitive */);
> -}
> -#endif
> -
>  // static
>  bool ECPrivateKey::ImportFromEncryptedPrivateKeyInfo(
>      PK11SlotInfo* slot,
>
>
> To unsubscribe from this group and stop receiving emails from it, send an
> email to chromium-reviews+unsubscribe@chromium.org.
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[davidben, 2015-02-24 23:38:59]

davidben@chromium.org changed reviewers:
- wfa511com@gmail.com

[davidben, 2015-02-24 23:44:59]

On 2015/02/24 01:30:56, Ryan Sleevi wrote:
> LGTM, but note the asymmetry with RSAPrivateKey

Uploaded a new version; missed some dead helpers and parameters. Probably wants
a second look.

I think the asymmetry is fine? There isn't an RSA version of
CreateSensitiveFromEncryptedPrivateKeyInfo, and ECPrivateKey doesn't work with
SignatureCreator. We may need to rethink the RSA versions of these anyway.
They're sort of weird functions that nominally are for use when NSS is the
platform crypto library, but, on the RSA side, all users assume that NSS is also
the internal crypto library and pull out key() directly. And for the chimera we
need to split these out.

I'm currently envisioning that code always know whether it's interacting with
NSS the internal crypto library or NSS the platform crypto library. And our
helpers be targeted towards one or the other.

[Ryan Sleevi, 2015-02-25 06:00:27]

On 2015/02/24 23:44:59, David Benjamin wrote:
> I'm currently envisioning that code always know whether it's interacting with
> NSS the internal crypto library or NSS the platform crypto library. And our
> helpers be targeted towards one or the other.

We need better terms. Even I can't keep those straight between internal and
platform :P

[Ryan Sleevi, 2015-02-25 06:04:35]

lgtm

https://codereview.chromium.org/951753002/diff/20001/crypto/ec_private_key_ns...
File crypto/ec_private_key_nss.cc (right):

https://codereview.chromium.org/951753002/diff/20001/crypto/ec_private_key_ns...
crypto/ec_private_key_nss.cc:109: DCHECK_LE(oid_data->oid.len, 127U);
Hrm, this should have been a CHECK, given that line 118 won't crash but do bad
things.

[davidben, 2015-02-25 20:20:50]

https://codereview.chromium.org/951753002/diff/20001/crypto/ec_private_key_ns...
File crypto/ec_private_key_nss.cc (right):

https://codereview.chromium.org/951753002/diff/20001/crypto/ec_private_key_ns...
crypto/ec_private_key_nss.cc:109: DCHECK_LE(oid_data->oid.len, 127U);
On 2015/02/25 06:04:35, Ryan Sleevi wrote:
> Hrm, this should have been a CHECK, given that line 118 won't crash but do bad
> things.

Done. (Bad things are less crash and more generate bogus ASN.1 data, right?)

[davidben, 2015-02-25 20:43:19]

The CQ bit was checked by davidben@chromium.org

[davidben, 2015-02-25 20:43:19]

The patchset sent to the CQ was uploaded after l-g-t-m from rsleevi@chromium.org
Link to the patchset: https://codereview.chromium.org/951753002/#ps60001 (title:
"sleevi comments")

[commit-bot: I haz the power, 2015-02-25 20:44:54]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/951753002/60001

[commit-bot: I haz the power, 2015-02-25 21:17:37]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-02-25 21:19:02]

Patchset 4 (id:??) landed as
https://crrev.com/f2331cd02a333399a45e084339532c4bb466fa2a
Cr-Commit-Position: refs/heads/master@{#318117}