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

proposal: crypto/x509: ability to add custom curve when parsing X509 certificate #32874

Open
trung opened this issue Jul 1, 2019 · 4 comments

Comments

@trung
Copy link

commented Jul 1, 2019

Per #26776, using a third party library for custom curve is advised.

However, when parsing x509 certificate (x509.ParseCetificate()), it is not possible to supply custom curve.

My proposal is to offer a configuration that can be used to supply a function to return elliptic.Curve from asn1.ObjectIdentifier to complement the default namedCurveFromIOD()

@trung trung changed the title cyrpto/x509: ability to add custom curve when parsing X509 certificate crypto/x509: ability to add custom curve when parsing X509 certificate Jul 1, 2019

@trung trung changed the title crypto/x509: ability to add custom curve when parsing X509 certificate proposal: crypto/x509: ability to add custom curve when parsing X509 certificate Jul 1, 2019

@gopherbot gopherbot added this to the Proposal milestone Jul 1, 2019

@gopherbot gopherbot added the Proposal label Jul 1, 2019

@trung

This comment has been minimized.

Copy link
Author

commented Jul 2, 2019

From the current flow:

go/src/crypto/x509/x509.go

Lines 519 to 553 in fbde753

var (
oidNamedCurveP224 = asn1.ObjectIdentifier{1, 3, 132, 0, 33}
oidNamedCurveP256 = asn1.ObjectIdentifier{1, 2, 840, 10045, 3, 1, 7}
oidNamedCurveP384 = asn1.ObjectIdentifier{1, 3, 132, 0, 34}
oidNamedCurveP521 = asn1.ObjectIdentifier{1, 3, 132, 0, 35}
)
func namedCurveFromOID(oid asn1.ObjectIdentifier) elliptic.Curve {
switch {
case oid.Equal(oidNamedCurveP224):
return elliptic.P224()
case oid.Equal(oidNamedCurveP256):
return elliptic.P256()
case oid.Equal(oidNamedCurveP384):
return elliptic.P384()
case oid.Equal(oidNamedCurveP521):
return elliptic.P521()
}
return nil
}
func oidFromNamedCurve(curve elliptic.Curve) (asn1.ObjectIdentifier, bool) {
switch curve {
case elliptic.P224():
return oidNamedCurveP224, true
case elliptic.P256():
return oidNamedCurveP256, true
case elliptic.P384():
return oidNamedCurveP384, true
case elliptic.P521():
return oidNamedCurveP521, true
}
return nil, false
}

I think of injecting custom functions something like:

type OIDToCurveFunc func(oid asn1.ObjectIdentifier) elliptic.Curve
type CurveToOIDFunc func(curve elliptic.Curve) (asn1.ObjectIdentifier, bool)

var (
	CustomIOIDToCurveFunc OIDToCurveFunc
	CustomCurveToOIDFunc CurveToOIDFunc
)

func namedCurveFromOID(oid asn1.ObjectIdentifier) elliptic.Curve {
	switch {
	case oid.Equal(oidNamedCurveP224):
		return elliptic.P224()
	case oid.Equal(oidNamedCurveP256):
		return elliptic.P256()
	case oid.Equal(oidNamedCurveP384):
		return elliptic.P384()
	case oid.Equal(oidNamedCurveP521):
		return elliptic.P521()
	}
	if CustomIOIDToCurveFunc != nil {
		return CustomIOIDToCurveFunc(oid)
	}
	return nil
}

func oidFromNamedCurve(curve elliptic.Curve) (asn1.ObjectIdentifier, bool) {
	switch curve {
	case elliptic.P224():
		return oidNamedCurveP224, true
	case elliptic.P256():
		return oidNamedCurveP256, true
	case elliptic.P384():
		return oidNamedCurveP384, true
	case elliptic.P521():
		return oidNamedCurveP521, true
	}
	if CustomCurveToOIDFunc != nil {
		return CustomCurveToOIDFunc(curve)
	}
	return nil, false
}
@rsc

This comment has been minimized.

Copy link
Contributor

commented Jul 2, 2019

Globals are probably not the right answer here. Perhaps there should be an x509.Parser that can be configured with extra hooks, a bit like there is an xml.Decoder to configure the XML parser.

Or perhaps unrecognized curves should be exposed in some general way and client code can just post-process.

/cc @FiloSottile

@rsc rsc added the Proposal-Crypto label Jul 2, 2019

@trung

This comment has been minimized.

Copy link
Author

commented Jul 3, 2019

Agreed with globals. There might be breaking changes once introducing x509.Parser as x509.ParseCertifcate() is used in number of places.

@FiloSottile

This comment has been minimized.

Copy link
Member

commented Jul 10, 2019

crypto/x509 primarily serves the WebPKI, and there is no use of custom curves there. X.509 is a sprawling standard, so it's critical for the crypto/x509 package to stay focused on its use case.

I will keep this open to collect feedback about use cases, as things of course change over time, but we are not going to support this for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.