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

crypto/x509, crypto/elliptic: support P-192 #41035

Closed
cespare opened this issue Aug 25, 2020 · 5 comments
Closed

crypto/x509, crypto/elliptic: support P-192 #41035

cespare opened this issue Aug 25, 2020 · 5 comments
Labels
FeatureRequest FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@cespare
Copy link
Contributor

cespare commented Aug 25, 2020

To work with some Apple APIs, we need to use ECDSA keys with the NIST P-192 curve (secp192r1). It would be convenient if this curve were implemented by crypto/x509 and crypto/elliptic such that functions like x509.ParseECPrivateKey and x509.ParsePKIXPublicKey worked with such keys. The crypto/x509 interface can't be used directly with our own curve definitions; in order to support P-192 we have to copy-paste various bits of crypto/x509 code.

It's not clear to me from my googling whether this curve is considered deficient or should be avoided for some reason, nor could I find any prior discussion about its inclusion in the stdlib crypto packages. If there is a reason that this curve should not be used then I understand not including it in crypto/*; in that case, this issue can serve as a record of that decision.

/cc @FiloSottile

@narqo
Copy link
Contributor

narqo commented Aug 26, 2020

To add to that, Apple's verification for SKAdNetwork postbacks, that they expected to start sending with iOS14 (releasing in September), requires checking against Apple's public key, which is P-192 (see https://developer.apple.com/documentation/storekit/skadnetwork/verifying_an_install_validation_postback).

Currently, it's impossible to verify the postback using Go's standard library because parsing Apple's public key returns "x509: unsupported elliptic curve": https://play.golang.org/p/oRhFhwYYVX_q

@cagedmantis cagedmantis added this to the Backlog milestone Aug 26, 2020
@cagedmantis cagedmantis added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. FeatureRequest labels Aug 26, 2020
@cagedmantis
Copy link
Contributor

/cc @FiloSottile

@FiloSottile
Copy link
Contributor

Sorry, but no. Currently, parsing keys only supports secure curves, so the application doesn't have to check if the curve is acceptable after parsing. P-192 is weaker than generally accepted security levels, so it shouldn't be allowed through the same paths as secure curves.

You can still instantiate a elliptic.CurveParams for it (which itself is a mistake, see #34648, but it is what it is) and probably use it with ecdsa.Verify, but you'll have to rip stuff out of crypto/x509 for parsing. It probably belongs in an external module, if a lot of people need this I might drop an implementation on my GitHub.

Really, it's disappointing to see Apple introduce P-192 in the ecosystem, and people should push back on it. Without optimized implementations, which no one carries because it's a weak curve, it will be slower than P-256 anyway.

@matipan
Copy link

matipan commented Oct 20, 2020

In case anybody lands here again: Apple now supports P256. They've updated their documentation for ad networks: https://developer.apple.com/documentation/storekit/skadnetwork/registering_an_ad_network. They now require to generate a P256 key and register that instead.

@narqo
Copy link
Contributor

narqo commented Oct 20, 2020

@matipan the docs for the validation of the Apple's own postbacks is still referring old Apple's own public key, which is P-192 :(

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FeatureRequest FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

6 participants