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/elliptic: add A to CurveParams, allowing some cases of A != -3 #26776

Closed
cag opened this issue Aug 2, 2018 · 17 comments
Closed
Labels
FrozenDueToAge Proposal Proposal-Crypto Proposal related to crypto packages or other security issues
Milestone

Comments

@cag
Copy link

cag commented Aug 2, 2018

The following addition to CurveParams is proposed:

type CurveParams struct {
        // ...
        A       *big.Int // the coefficient of x in the curve equation
        // ...
}

The groups from bn256 can be considered instances of elliptic.Curve with this modification, being Koblitz curves with A = 0, B = 3. I suspect the G1 implementation can be repurposed as a general variable-time implementation for A = 0 curves, as the implementation derives from the study of this class of curves, which only assumes that A = 0.

Some support for the named Koblitz curves in SEC 2 can also be added (so A = 0 or A = -1 (mod P) also possible).

Parsing X.509 keys which use these curves with crypto/x509 should also work. This means keys and certificates which use the Koblitz curves will be supported by the tls module. Refer to RFC 4492 Section 5.1.1 for their corresponding enum values.

See also #6782 and #26187

Also would like to remark that all Montgomery curves admit a Weierstrass form (and the computational speedup for computing operations on e.g. Curve25519, can be thought of as implementation details). In fact, all elliptic curves could be correctly implemented by elliptic.Curve with this modification. Curve/Ed25519 is isomorphic to a curve group defined by a short Weierstrass form equation, but this is not the case in general.

@FiloSottile FiloSottile added Proposal Proposal-Crypto Proposal related to crypto packages or other security issues labels Aug 2, 2018
@FiloSottile FiloSottile added this to the Proposal milestone Aug 2, 2018
@ianlancetaylor ianlancetaylor changed the title crypto/elliptic: Add A to CurveParams, allowing some cases of A != -3 proposal: crypto/elliptic: add A to CurveParams, allowing some cases of A != -3 Aug 2, 2018
@sanguohot
Copy link

@cag Hey.I wonder if your go fork has this curve supported.

@cag
Copy link
Author

cag commented Aug 6, 2018

No, still exploring this issue, and whatever I implement first will likely not be timing side-channel secure.


Also, wrt Curve25519, there is an IETF draft talking about the compatibility of that, Ed25519, and an equivalent Weierstrass form of it: https://tools.ietf.org/id/draft-struik-lwig-curve-representations-02.html

In fact, all elliptic curves could be correctly implemented by elliptic.Curve with this modification.

I was wrong. To paraphrase Wikipedia:

If the characteristic of K[, the field over which the curve is defined,] is neither 2 nor 3, then every elliptic curve over K can be written in the [short Weierstrass form.]


Regarding prime Koblitz curves: those weren't recommended by the NIST in FIPS 186. I got them from SECG's SEC 2. FIPS 186 calls secp256r1 the P-256 curve, which is what the name P256() is derived from. I'm using Secp256k1() to represent the secp256k1 curve. There are K-xxx curves in FIPS 186 which denote Koblitz as well, but those are binary curves (i.e. curves over a finite field K with characteristic 2). These don't have a representation in the short Weierstrass form.

What I'd like to know is whether any of the following provides enough motivation for an elliptic.Curve expansion to the class of general short-form Weierstrass curves to be implemented in the golang standard lib:

  • prime Koblitz curves like secp256k1
  • Curve/Ed25519
  • BN curves from x/crypto/bn256

@cag
Copy link
Author

cag commented Aug 8, 2018

Thanks to @collinc97 for implementing this for me!

Note that allowing A to vary means changing only the doubling algorithm from this to this, as the addition algo is identical to the current implementation.

This changes the cost from 3M + 5S to 1M + 8S + 1*a, where M stands for big.Int multiplication, S stands for big.Int squaring (more or less equivalent here in time complexity to big.Int multiplication), and *a stands for multiplying by A. What this practically means is that for this generalization, we go from eight big.Int multiplies to ten in a generic implementation which is vulnerable to side-channels. P-256 (probably the recipient of the lion's share of modern elliptic.Curve use) is mostly not affected by this, having an optimized time-invariant implementation.

Anyway, secp256k1 was added as an elliptic.Curve in order to verify the implementation is correct, but I still wish to hear from maintainers whether the addition of curves like this belongs in the Golang standard library.

@sanguohot
Copy link

@cag It is really a good news from you, and I really thank you for the contribuction.
So I want to build it and do my test.
But I got this error when using your curve-param-a branch.

package main

import (
	"crypto/tls"
	"fmt"
)

func main() {
	fmt.Println(`openssl req -x509 -sha256 -nodes -newkey ec:<(openssl ecparam -name secp256k1) -keyout tls.key -out tls.crt -days 3650 -subj "/O=Bar/CN=Foo"`)
	fmt.Println(tls.X509KeyPair([]byte(`
-----BEGIN CERTIFICATE-----
MIIBfDCCASKgAwIBAgIJAOlLpHqkoFZOMAoGCCqGSM49BAMCMBwxDDAKBgNVBAoM
A0JhcjEMMAoGA1UEAwwDRm9vMB4XDTE4MDgwOTAyNTIxN1oXDTI4MDgwNjAyNTIx
N1owHDEMMAoGA1UECgwDQmFyMQwwCgYDVQQDDANGb28wVjAQBgcqhkjOPQIBBgUr
gQQACgNCAAQWvVJ+HOjfOvb14bN1O3iWUl2vXVk9WWg77TC6FGiuXkKU6/1URakD
oTfR/PXU87+U7VozOYA4/SW0yQKu9W4Uo1AwTjAdBgNVHQ4EFgQUFSBA+z4N+/02
OMZqIUKSwi9mBCUwHwYDVR0jBBgwFoAUFSBA+z4N+/02OMZqIUKSwi9mBCUwDAYD
VR0TBAUwAwEB/zAKBggqhkjOPQQDAgNIADBFAiBr11aPIg93TJN/mjv+WlvygFcF
MMlsUlWNHSbaXQB6SAIhAKA+IrhEeXJe6KQQqpTPp3mnNL2icaxSHp/AH5Jx7L1B
-----END CERTIFICATE-----
`), []byte(`
-----BEGIN PRIVATE KEY-----
MIGEAgEAMBAGByqGSM49AgEGBSuBBAAKBG0wawIBAQQgGTQNnM0BCTDGUVlrMJaZ
cwwYYTS+Z9Rj/+kFP9JMg1KhRANCAAQWvVJ+HOjfOvb14bN1O3iWUl2vXVk9WWg7
7TC6FGiuXkKU6/1URakDoTfR/PXU87+U7VozOYA4/SW0yQKu9W4U
-----END PRIVATE KEY-----
`)))
}

Then i ran it and got the output:

[root@blockchainmedicalimaging53 ~]# go run tls.secp256k1.go
openssl req -x509 -sha256 -nodes -newkey ec:<(openssl ecparam -name secp256k1) -keyout tls.key -out tls.crt -days 3650 -subj "/O=Bar/CN=Foo"
{[] [] [] } x509: unsupported elliptic curve

May be my mistake?

@cag
Copy link
Author

cag commented Aug 9, 2018

@sanguohot No mistake. Just that this is only the elliptic.Curve implementation stuff, not any of the PKI stuff supporting it. Thanks for the test case though!

I suspect what's left is a simple matter of mapping the named curve enum to an ECDSA thingy backed by the appropriate curve, but I haven't looked into doing this yet. Sorry to get your hopes up so soon.

Edit: with that said, be warned:

I still wish to hear from maintainers whether the addition of curves like this belongs in the Golang standard library.

@sanguohot
Copy link

@cag Oh, it is a pity.
I am interesting in your words, so I will take a look at the source code, but I am not sure to figure it out.
Anyway I am still ready to test it.

@collinc97
Copy link

@sanguohot I have just added tls support for secp256k1 on #26873
When I run your example I get:

openssl req -x509 -sha256 -nodes -newkey ec:<(openssl ecparam -name secp256k1) -keyout tls.key -out tls.crt -days 3650 -subj "/O=Bar/CN=Foo"
{[[48 130 1 124 48 130 1 34 160 3 2 1 2 2 9 0 233 75 164 122 164 160 86 78 4810 6 8 42 134 72
 206 61 4 3 2 48 28 49 12 48 10 6 3 85 4 10 12 3 66 97 114 4912 48 10 6 3 85 4 3 12 3 70 111 111
 48 30 23 13 49 56 48 56 48 57 48 50 53 5049 55 90 23 13 50 56 48 56 48 54 48 50 53 50 49
 55 90 48 28 49 12 48 10 6 3 85 4 10 12 3 66 97 114 49 12 48 10 6 3 85 4 3 12 3 70 111 111 48 86
 48 16 6 7 42 134 72 206 61 2 1 6 5 43 129 4 0 10 3 66 0 4 22 189 82 126 28 232 223 58 246245
 225 179 117 59 120 150 82 93 175 93 89 61 89 104 59 237 48 186 20 104 17494 66 148 235 253
 84 69 169 3 161 55 209 252 245 212 243 191 148 237 90 51 57128 56 253 37 180 201 2 174 245
 110 20 163 80 48 78 48 29 6 3 85 29 14 4 22 420 21 32 64 251 62 13 251 253 54 56 198 106 33
 66 146 194 47 102 4 37 48 31 63 85 29 35 4 24 48 22 128 20 21 32 64 251 62 13 251 253 54 56
 198 106 33 66 146 194 47 102 4 37 48 12 6 3 85 29 19 4 5 48 3 1 1 255 48 10 6 8 42 134 72 
20661 4 3 2 3 72 0 48 69 2 32 107 215 86 143 34 15 119 76 147 127 154 59 254 90 91 242 128 87
 5 48 201 108 82 85 141 29 38 218 93 0 122 72 2 33 0 160 62 34 184 68 121 114 94 232 164 16 
170 148 207 167 121 167 52 189 162 113 172 82 30 159 192 31 146 113 236 189 65]] 
0xc000096960 [] [] <nil>} <nil>

Let me know if you have any issues.

@sanguohot
Copy link

@collinc97 Oh my god!
You can not imagine how excited I was when received the email.
Thank you so much!
I simply test it and it seems ok.
But I can not do another test until monday for several reasons.
I will feedback a little late.

@sanguohot
Copy link

sanguohot commented Aug 14, 2018

@collinc97 @cag I got a problem.
First setup the server by openssl command.

[root@blockchainmedicalimaging54 ~]# openssl s_server -key /opt/conf/sdk.key -CAfile /opt/conf/ca.crt -cert /opt/conf/sdk.crt -accept 8822 -named_curve secp256k1
Using default temp DH parameters
ACCEPT

I list the server's cipher suites

[root@blockchainmedicalimaging53 ~]# sslscan 10.6.250.54:8822|grep Accepted
Accepted SSLv3 256 bits ECDHE-ECDSA-AES256-SHA
Accepted SSLv3 128 bits ECDHE-ECDSA-AES128-SHA
Accepted SSLv3 112 bits ECDHE-ECDSA-DES-CBC3-SHA
Accepted SSLv3 112 bits ECDHE-ECDSA-RC4-SHA
Accepted TLSv1 256 bits ECDHE-ECDSA-AES256-SHA
Accepted TLSv1 128 bits ECDHE-ECDSA-AES128-SHA
Accepted TLSv1 112 bits ECDHE-ECDSA-DES-CBC3-SHA
Accepted TLSv1 112 bits ECDHE-ECDSA-RC4-SHA
Accepted TLS11 256 bits ECDHE-ECDSA-AES256-SHA
Accepted TLS11 128 bits ECDHE-ECDSA-AES128-SHA
Accepted TLS11 112 bits ECDHE-ECDSA-DES-CBC3-SHA
Accepted TLS11 112 bits ECDHE-ECDSA-RC4-SHA
Accepted TLS12 256 bits ECDHE-ECDSA-AES256-GCM-SHA384
Accepted TLS12 256 bits ECDHE-ECDSA-AES256-SHA384
Accepted TLS12 256 bits ECDHE-ECDSA-AES256-SHA
Accepted TLS12 128 bits ECDHE-ECDSA-AES128-GCM-SHA256
Accepted TLS12 128 bits ECDHE-ECDSA-AES128-SHA256
Accepted TLS12 128 bits ECDHE-ECDSA-AES128-SHA
Accepted TLS12 112 bits ECDHE-ECDSA-DES-CBC3-SHA
Accepted TLS12 112 bits ECDHE-ECDSA-RC4-SHA

And the openssl supported
[root@blockchainmedicalimaging55 ~]# openssl ciphers -V | column -t|grep ECDSA

0xC0,0x2C - ECDHE-ECDSA-AES256-GCM-SHA384 TLSv1.2 Kx=ECDH Au=ECDSA Enc=AESGCM(256) Mac=AEAD
0xC0,0x24 - ECDHE-ECDSA-AES256-SHA384 TLSv1.2 Kx=ECDH Au=ECDSA Enc=AES(256) Mac=SHA384
0xC0,0x0A - ECDHE-ECDSA-AES256-SHA SSLv3 Kx=ECDH Au=ECDSA Enc=AES(256) Mac=SHA1
0xC0,0x2E - ECDH-ECDSA-AES256-GCM-SHA384 TLSv1.2 Kx=ECDH/ECDSA Au=ECDH Enc=AESGCM(256) Mac=AEAD
0xC0,0x26 - ECDH-ECDSA-AES256-SHA384 TLSv1.2 Kx=ECDH/ECDSA Au=ECDH Enc=AES(256) Mac=SHA384
0xC0,0x05 - ECDH-ECDSA-AES256-SHA SSLv3 Kx=ECDH/ECDSA Au=ECDH Enc=AES(256) Mac=SHA1
0xC0,0x2B - ECDHE-ECDSA-AES128-GCM-SHA256 TLSv1.2 Kx=ECDH Au=ECDSA Enc=AESGCM(128) Mac=AEAD
0xC0,0x23 - ECDHE-ECDSA-AES128-SHA256 TLSv1.2 Kx=ECDH Au=ECDSA Enc=AES(128) Mac=SHA256
0xC0,0x09 - ECDHE-ECDSA-AES128-SHA SSLv3 Kx=ECDH Au=ECDSA Enc=AES(128) Mac=SHA1
0xC0,0x2D - ECDH-ECDSA-AES128-GCM-SHA256 TLSv1.2 Kx=ECDH/ECDSA Au=ECDH Enc=AESGCM(128) Mac=AEAD
0xC0,0x25 - ECDH-ECDSA-AES128-SHA256 TLSv1.2 Kx=ECDH/ECDSA Au=ECDH Enc=AES(128) Mac=SHA256
0xC0,0x04 - ECDH-ECDSA-AES128-SHA SSLv3 Kx=ECDH/ECDSA Au=ECDH Enc=AES(128) Mac=SHA1
0xC0,0x08 - ECDHE-ECDSA-DES-CBC3-SHA SSLv3 Kx=ECDH Au=ECDSA Enc=3DES(168) Mac=SHA1
0xC0,0x03 - ECDH-ECDSA-DES-CBC3-SHA SSLv3 Kx=ECDH/ECDSA Au=ECDH Enc=3DES(168) Mac=SHA1
0xC0,0x07 - ECDHE-ECDSA-RC4-SHA SSLv3 Kx=ECDH Au=ECDSA Enc=RC4(128) Mac=SHA1
0xC0,0x02 - ECDH-ECDSA-RC4-SHA SSLv3 Kx=ECDH/ECDSA Au=ECDH Enc=RC4(128) Mac=SHA1

I test it using openssl s_client, and this is my code.

#!/usr/bin/env bash

# OpenSSL requires the port number.
SERVER=$1
DELAY=1
ciphers=$(openssl ciphers 'ALL:eNULL' | sed -e 's/:/ /g')

echo Obtaining cipher list from $(openssl version).

for cipher in ${ciphers[@]}
do
echo -n Testing $cipher...
result=$(echo -n | openssl s_client -cipher "$cipher" -connect $SERVER 2>&1)
if [[ "$result" =~ ":error:" ]] ; then
  error=$(echo -n $result | cut -d':' -f6)
  echo NO \($error\)
else
  if [[ "$result" =~ "Cipher is ${cipher}" || "$result" =~ "Cipher    :" ]] ; then
    echo YES
  else
    echo UNKNOWN RESPONSE
    echo $result
  fi
fi
sleep $DELAY
done

Everything seems ok.

[root@blockchainmedicalimaging53 ~]# ./cipher_test.sh 10.6.250.54:8822|grep YES
Testing ECDHE-ECDSA-AES256-GCM-SHA384...YES
Testing ECDHE-ECDSA-AES256-SHA384...YES
Testing ECDHE-ECDSA-AES256-SHA...YES
Testing ECDHE-ECDSA-AES128-GCM-SHA256...YES
Testing ECDHE-ECDSA-AES128-SHA256...YES
Testing ECDHE-ECDSA-AES128-SHA...YES

Then I test it with the cipher suites supported.

package main
import (
	"crypto/tls"
	"log"
	"io/ioutil"
	"crypto/x509"
	"flag"
	"os"
)
var (
	certFile = flag.String("cert", "/opt/conf/sdk.crt", "A PEM eoncoded certificate file.")
	keyFile  = flag.String("key", "/opt/conf/sdk.key", "A PEM encoded private key file.")
	caFile   = flag.String("CA", "/opt/conf/ca.crt", "A PEM eoncoded CA's certificate file.")
)
func main() {
	// Load client cert
	// Load client cert
	cert, err := tls.LoadX509KeyPair(*certFile, *keyFile)
	if err != nil {
		log.Fatal(err)
	}

	// Load CA cert
	caCert, err := ioutil.ReadFile(*caFile)
	if err != nil {
		log.Fatal(err)
	}
	caCertPool := x509.NewCertPool()
	caCertPool.AppendCertsFromPEM(caCert)

	tlsConfig := &tls.Config{
		Certificates: []tls.Certificate{cert},
		RootCAs:      caCertPool,
		InsecureSkipVerify: true,
		MinVersion: tls.VersionSSL30,
		CipherSuites: []uint16{
			tls.TLS_ECDHE_ECDSA_WITH_RC4_128_SHA,
			tls.TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA,
			tls.TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA,
			tls.TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256,
			tls.TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256,
			tls.TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384,
		},
	}
	conn, err := tls.Dial("tcp", "10.6.250.54:8822", tlsConfig)
	if err != nil {
		log.Println(err)
		return
	}
	defer conn.Close()
	n, err := conn.Write([]byte("hello\n"))
	if err != nil {
		log.Println(n, err)
		return
	}
	buf := make([]byte, 100)
	n, err = conn.Read(buf)
	if err != nil {
		log.Println(n, err)
		return
	}
	println(string(buf[:n]))
	os.Stdin.Read(make([]byte,1))
}

Unluckly I get the error

[root@blockchainmedicalimaging53 ~]# go run tls.client.go
2018/08/14 10:32:23 remote error: tls: handshake failure

And the server said:

ERROR
139830577534864:error:1408A0C1:SSL routines:ssl3_get_client_hello:no shared cipher:s3_srvr.c:1435:
shutting down SSL
CONNECTION CLOSED

cert files:

[root@blockchainmedicalimaging53 ~]# cat /opt/conf/sdk.key
-----BEGIN PRIVATE KEY-----
MIGEAgEAMBAGByqGSM49AgEGBSuBBAAKBG0wawIBAQQgiwGaJ0Wpf6tHtINzoOL3
kN9SydBc6HFPKewV4Cw+nQ+hRANCAAQqQOPXT6YHVmzv+IrdISJfjAqPe+GtrwoM
OxwS2/BvSyCKXLQkgBCK79moFvaFi81BKm8r1MMWpV7uQQwdsdgj
-----END PRIVATE KEY-----
[root@blockchainmedicalimaging53 ~]# cat /opt/conf/sdk.crt
-----BEGIN CERTIFICATE-----
MIICrTCCAZWgAwIBAgIJAId9WK/1W0QMMA0GCSqGSIb3DQEBCwUAMGYxCzAJBgNV
BAMMAkNOMQswCQYDVQQGEwJDTjESMBAGA1UECAwJR3Vhbmdkb25nMREwDwYDVQQH
DAhTaGVuemhlbjETMBEGA1UECgwKRklTQ08tQkNPUzEOMAwGA1UECwwFYWdlbnQw
HhcNMTgwODAyMDkzMzQ0WhcNMTgwOTAxMDkzMzQ0WjB4MS0wKwYDVQQDDCRDTi9l
bWFpbEFkZHJlc3M9c2VydmljZUBmaXNjby5jb20uY24xCzAJBgNVBAYTAkNOMRIw
EAYDVQQIDAlHdWFuZ2RvbmcxETAPBgNVBAcMCFNoZW56aGVuMRMwEQYDVQQKDApG
SVNDTy1CQ09TMFYwEAYHKoZIzj0CAQYFK4EEAAoDQgAEKkDj10+mB1Zs7/iK3SEi
X4wKj3vhra8KDDscEtvwb0sgily0JIAQiu/ZqBb2hYvNQSpvK9TDFqVe7kEMHbHY
I6MaMBgwCQYDVR0TBAIwADALBgNVHQ8EBAMCBeAwDQYJKoZIhvcNAQELBQADggEB
AC5oCYmU/LtZ7DszzDlQjmoDIegiC9v5u7RFP9eWRaVxS8DX2XOBWwYemFgsmr6q
7D/6qsUpw35pRRnXp8TEGo6i5GFOED8VmuZPbmfGRfUg4u4XmJPYuRa9IAQqGnIT
p1t49CFr3GnIvGpvmJrMXg8ZHfPSo7Bbnz5PUOXwjWSQHfrbc/S0TsarA9WIWeEE
7mbJOaGwFhzNzJtUW3k0doc2L3KSZfMm21uDvnZHqeRbDy2KMEl60p56wY9V2tgo
J4jj+P+EDUqcLfx1aO+GjLf5fDf4aQBZhxWPJQirY/tB84R8u6wN7j/DXgbYcgZS
psYHgZLCq8yb38ruFRYqw08=
-----END CERTIFICATE-----
[root@blockchainmedicalimaging53 ~]# cat /opt/conf/ca.crt
-----BEGIN CERTIFICATE-----
MIIDnzCCAoegAwIBAgIJALsPI3cqXzWEMA0GCSqGSIb3DQEBCwUAMGYxCzAJBgNV
BAMMAkNOMQswCQYDVQQGEwJDTjESMBAGA1UECAwJR3Vhbmdkb25nMREwDwYDVQQH
DAhTaGVuemhlbjETMBEGA1UECgwKRklTQ08tQkNPUzEOMAwGA1UECwwFY2hhaW4w
HhcNMTgwODAyMDkzMzQzWhcNMjgwNzMwMDkzMzQzWjBmMQswCQYDVQQDDAJDTjEL
MAkGA1UEBhMCQ04xEjAQBgNVBAgMCUd1YW5nZG9uZzERMA8GA1UEBwwIU2hlbnpo
ZW4xEzARBgNVBAoMCkZJU0NPLUJDT1MxDjAMBgNVBAsMBWNoYWluMIIBIjANBgkq
hkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAsJOQ6Aljnl6mP/p1wWitF6Aob/Gx/Sg/
vgPWwfVCkgVlVLHHbGg4rlAvwSKSNhTwUf6s3FE0ZAfKfeeEJiqFkk+yyJgDXVfE
gZ9ez0Jw4uDWrSEDmPPiU3p2L59w3UaFAbpE6IBYon+JmooWg0rNBx964lzxQ0vC
V+3g1uNE4xoCY4uvSPfgyuBLcalmsyyrmfkmUkPLcpykbcGUnU5uQQ6osmjBm4A1
+PGH9QzkqDt0kCWm3kuOAe7tQCwoGAhfIKpBo/3BJ/+DfW9kHZISPPNlz72HBFIw
5gWBZZjYkm+eLNFjCp7gnUAIe9ly8WfRuJp7g0n5NA7f8aBhWKW/rwIDAQABo1Aw
TjAdBgNVHQ4EFgQUIyT75hA2a9XdAwTg1Jd6T7lXKGwwHwYDVR0jBBgwFoAUIyT7
5hA2a9XdAwTg1Jd6T7lXKGwwDAYDVR0TBAUwAwEB/zANBgkqhkiG9w0BAQsFAAOC
AQEAGSE8PwIVUuRAJYEGuP62OJz1NxTCVOYPKkWFFlZagDTm4+WFaiR56TcvRkYN
9+VOnd8wOCUETk12ZQ0CBUar45+g/hWcS5bhPY8ixb/YCocumTSEZ3kA6kXoyZXy
YKNrqUPR9Kf4CD2imqwgEWz623sRU0Sxihqq21sS8nxs9Nyz3l0W+twfi03qH/mW
Jf2lBAC6dSuc3mSG8sQDX7/NCJf5BcenxkduFwN+RqnvYD6Pg5xh0GDPYrlUJ1z2
pbYQ41a5qvhVLi6OmAiL9DF32S968uofl4muFYsDz8JMq5FBkxvjHD7wFpKjWCyl
dT4Wir63GEp5VhuySwN/Pw2CpA==
-----END CERTIFICATE-----
-----BEGIN CERTIFICATE-----
MIIDXzCCAkegAwIBAgIJAJxeXjau5/iOMA0GCSqGSIb3DQEBCwUAMGYxCzAJBgNV
BAMMAkNOMQswCQYDVQQGEwJDTjESMBAGA1UECAwJR3Vhbmdkb25nMREwDwYDVQQH
DAhTaGVuemhlbjETMBEGA1UECgwKRklTQ08tQkNPUzEOMAwGA1UECwwFY2hhaW4w
HhcNMTgwODAyMDkzMzQzWhcNMjgwNzMwMDkzMzQzWjBmMQswCQYDVQQDDAJDTjEL
MAkGA1UEBhMCQ04xEjAQBgNVBAgMCUd1YW5nZG9uZzERMA8GA1UEBwwIU2hlbnpo
ZW4xEzARBgNVBAoMCkZJU0NPLUJDT1MxDjAMBgNVBAsMBWFnZW50MIIBIjANBgkq
hkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEA0q0TuhuFJqLlxlHL35Lj9HLJtpyUboad
FmrrCIgwOUVp5AaNFo/uc4BK/c1VtPY+DOoeI48TJIPTmJJEI02E6cce6PKrPYGf
n4gU1ypkgHgijmpOu4hZ7UGfjFwNmjddkp0BIeBzY+kawhNKK/r7DCEzcV0mA+tk
Gxvu1457ebkofWsEw4s9npqTzU29i0ADw9/Vw6A6RF6J6rGt6fKz277Fl3vjeHyo
Zpa23qZ4F13eKqhUwQSbkClQJvkSqi6+6FzgzMrgpOdi+KH2kvaMZ4b04Wp7aXOh
p1bRxM4IkTsTh873n848N6BtyE0tn+mR9G+kSWzdOeD9rSXs5e/9DwIDAQABoxAw
DjAMBgNVHRMEBTADAQH/MA0GCSqGSIb3DQEBCwUAA4IBAQBlbCpCDrPsWulNmg2C
cnY2vDveC1fkbauBFDGV0IEPelxzPNBKa9hqN5FJyqsjo5CXh2MgySfvMVks8aHV
UYU3V9H/4s3DfqdegEW3gyJDAHoSNHN5HQ3yCx9zEuIRozSEUpZpMx1gZy9ZMK7k
O7kpwYhgDNBvKcnq5PTEngPjXg/Cpw+EB8NMFTqASDFZ53fkXc22OMu6F35Vkm2D
0egfk2hcwEApl8W34xgV++/RMExGFiJ7HAqx2LCciz1EsrF2/lLfJdeEW0KYyfo4
D4uzaBNgGK20nQcNqJrmjIX9Ea1/P0s97tT5hwWoVggGVki9qRRHWptXO6sGbjp5
FtSZ
-----END CERTIFICATE-----

I have no idea now, would you give me some help please?

@aajtodd
Copy link

aajtodd commented Oct 25, 2018

I would be interested in this as it would allow ed25519 to be used as an elliptic.Curve with the appropriate conversion to Weierstrass form (if I understood correctly).

The ed25519 package from x/crypto only exposes the signature interfaces, it does no expose point operations required for things like PAKE. At the very least it would be nice if the implementation wasn't nested in an internal package so that these operations could be built on top.

@ritesh
Copy link

ritesh commented Oct 30, 2018

Hello! I see that there's a PR to expose the A param - will this get merged at some point?

@paocalvi
Copy link

I'd also love to have some news on this topic. It seems an easy fix, why is not merged yet?
Thanks, Paolo

@FiloSottile
Copy link
Contributor

There seems to be two separate proposals here: making the generic CurveParams implementation more flexible, and adding support for more curves (or curve operations).

I believe the generic CurveParams type was a mistake. Custom curves don't belong in the standard library, as most users should never ever use them. As a way to implement known curves, the CurveParams approach is suboptimal, because it requires generic, slow and non-constant time implementations. Indeed, the only elliptic.Curve you should use (NIST P-256) has a completely separate implementation. (But you can still shoot yourself in the foot by using elliptic.P256().Params().) So no, I don't think we should double down on the mistake by expanding CurveParams in the standard library.

As for adding curves, I'm not convinced they need to be in the standard library either. Surely there is a good secp256k1 implementation in the ecosystem due to its use in Bitcoin. As for Ed25519 curve operations, that might be worth doing if there's enough demand. Please open a new issue to discuss that specifically, as it would likely go into x/crypto/ed25519. (Although I would recommend using https://github.com/gtank/ristretto255 in new protocols instead.) Finally, BN256 is broken, so we can ignore it.

There is an argument for providing a curve interface, but unfortunately elliptic.Curve hardcoded *big.Int in its method definitions, so I'm afraid it's not the way forward, and I'll document that it's frozen to the NIST curves. (@gtank tried making an Ed25519 elliptic.Curve, and it was awkward and slow due to the API.)

@paocalvi
Copy link

paocalvi commented Jun 2, 2019 via email

@FiloSottile
Copy link
Contributor

I am saying the standard library should provide specialized implementations of an opinionated small set of curves, not support for all possible curves. The latter belongs in a third-party project (which can absolutely start from the stdlib code, as it's BSD licensed).

@eliwjones
Copy link

As for adding curves, I'm not convinced they need to be in the standard library either. Surely there is a good secp256k1 implementation in the ecosystem due to its use in Bitcoin.

I went ahead and pulled the code from the PR out and put a usable version here (tests and example included):

https://github.com/eliwjones/crypto

The main secp256k1 stuff I see out and about are wrappers of C code.

I have not seen a satisfactorily trustworthy native Go implementation.. so, if anyone knows of one, please post it here.

@baha-ai
Copy link

baha-ai commented Nov 20, 2019

As for adding curves, I'm not convinced they need to be in the standard library either. Surely there is a good secp256k1 implementation in the ecosystem due to its use in Bitcoin.

I went ahead and pulled the code from the PR out and put a usable version here (tests and example included):

https://github.com/eliwjones/crypto

The main secp256k1 stuff I see out and about are wrappers of C code.

I have not seen a satisfactorily trustworthy native Go implementation.. so, if anyone knows of one, please post it here.

Apparently there is one here: https://github.com/btcsuite/btcd/tree/master/btcec
I posted an issue to request extracting this package into it's own repo (for easier referencing).

AnnaShaleva added a commit to nspcc-dev/neo-go that referenced this issue Jun 2, 2020
Part of #1003

We have to implement Koblitz-related math to recover keys properly
with Neo.Cryptography.Secp256k1Recover interop as far as standard
go elliptic package supports short-form Weierstrass curve with a=-3
only (see golang/go#26776 for details).
However, it's not the best choise to have a lot of such math in our
project, so it would be better to use ready-made solution for
Koblitz-related cryptography.
AnnaShaleva added a commit to nspcc-dev/neo-go that referenced this issue Jun 2, 2020
Part of #1003

We have to implement Koblitz-related math to recover keys properly
with Neo.Cryptography.Secp256k1Recover interop as far as standard
go elliptic package supports short-form Weierstrass curve with a=-3
only (see golang/go#26776 for details).
However, it's not the best choise to have a lot of such math in our
project, so it would be better to use ready-made solution for
Koblitz-related cryptography.
AnnaShaleva added a commit to nspcc-dev/neo-go that referenced this issue Jun 2, 2020
Part of #1003

We have to implement Koblitz-related math to recover keys properly
with Neo.Cryptography.Secp256k1Recover interop as far as standard
go elliptic package supports short-form Weierstrass curve with a=-3
only (see golang/go#26776 for details).
However, it's not the best choise to have a lot of such math in our
project, so it would be better to use ready-made solution for
Koblitz-related cryptography.
AnnaShaleva added a commit to nspcc-dev/neo-go that referenced this issue Jun 2, 2020
Part of #1003

We have to implement Koblitz-related math to recover keys properly
with Neo.Cryptography.Secp256k1Recover interop as far as standard
go elliptic package supports short-form Weierstrass curve with a=-3
only (see golang/go#26776 for details).
However, it's not the best choise to have a lot of such math in our
project, so it would be better to use ready-made solution for
Koblitz-related cryptography.
AnnaShaleva added a commit to nspcc-dev/neo-go that referenced this issue Jun 2, 2020
Implement secp256k1 and secp256r1 recover interops, closes #1003.

Note:

We have to implement Koblitz-related math to recover keys properly
with Neo.Cryptography.Secp256k1Recover interop as far as standard
go elliptic package supports short-form Weierstrass curve with a=-3
only (see golang/go#26776 for details).
However, it's not the best choise to have a lot of such math in our
project, so it would be better to use ready-made solution for
Koblitz-related cryptography.
AnnaShaleva added a commit to nspcc-dev/neo-go that referenced this issue Jun 2, 2020
Implement secp256k1 and secp256r1 recover interops, closes #1003.

Note:

We have to implement Koblitz-related math to recover keys properly
with Neo.Cryptography.Secp256k1Recover interop as far as standard
go elliptic package supports short-form Weierstrass curve with a=-3
only (see golang/go#26776 for details).
However, it's not the best choise to have a lot of such math in our
project, so it would be better to use ready-made solution for
Koblitz-related cryptography.
AnnaShaleva added a commit to nspcc-dev/neo-go that referenced this issue Jun 2, 2020
Implement secp256k1 and secp256r1 recover interops, closes #1003.

Note:

We have to implement Koblitz-related math to recover keys properly
with Neo.Cryptography.Secp256k1Recover interop as far as standard
go elliptic package supports short-form Weierstrass curve with a=-3
only (see golang/go#26776 for details).
However, it's not the best choise to have a lot of such math in our
project, so it would be better to use ready-made solution for
Koblitz-related cryptography.
AnnaShaleva added a commit to nspcc-dev/neo-go that referenced this issue Jun 3, 2020
Implement secp256k1 and secp256r1 recover interops, closes #1003.

Note:

We have to implement Koblitz-related math to recover keys properly
with Neo.Cryptography.Secp256k1Recover interop as far as standard
go elliptic package supports short-form Weierstrass curve with a=-3
only (see golang/go#26776 for details).
However, it's not the best choise to have a lot of such math in our
project, so it would be better to use ready-made solution for
Koblitz-related cryptography.
AnnaShaleva added a commit to nspcc-dev/neo-go that referenced this issue Jun 3, 2020
Implement secp256k1 and secp256r1 recover interops, closes #1003.

Note:

We have to implement Koblitz-related math to recover keys properly
with Neo.Cryptography.Secp256k1Recover interop as far as standard
go elliptic package supports short-form Weierstrass curve with a=-3
only (see golang/go#26776 for details).
However, it's not the best choise to have a lot of such math in our
project, so it would be better to use ready-made solution for
Koblitz-related cryptography.
AnnaShaleva added a commit to nspcc-dev/neo-go that referenced this issue Jun 3, 2020
Implement secp256k1 and secp256r1 recover interops, closes #1003.

Note:

We have to implement Koblitz-related math to recover keys properly
with Neo.Cryptography.Secp256k1Recover interop as far as standard
go elliptic package supports short-form Weierstrass curve with a=-3
only (see golang/go#26776 for details).
However, it's not the best choise to have a lot of such math in our
project, so it would be better to use ready-made solution for
Koblitz-related cryptography.
@golang golang locked and limited conversation to collaborators Nov 19, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge Proposal Proposal-Crypto Proposal related to crypto packages or other security issues
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants