From 9b1a210a06ea1176ec1f0a1ddf83ad7463b8ea3e Mon Sep 17 00:00:00 2001 From: Alex Vaghin Date: Fri, 16 Dec 2016 14:31:18 +0100 Subject: [PATCH] acme/autocert: add an option for RSA-based certs Currently, autocert.Manager always generates EC-based certificates. This change adds an optional field forcing the Manager to use RSA instead. An alternative idea, a "double" certificate, where the Manager presents either RSA or EC certificate based on client's compatibility, doesn't seem to be worth the implementation time given the constant increase in Elliptic Curve cryptography. Fixes golang/go#17744 Change-Id: Idc68abfc698bcff4aad99715baefc06f8fae50ad Reviewed-on: https://go-review.googlesource.com/34570 Reviewed-by: Brad Fitzpatrick --- acme/autocert/autocert.go | 18 +++++++++++++- acme/autocert/autocert_test.go | 43 ++++++++++++++++++++++++---------- 2 files changed, 48 insertions(+), 13 deletions(-) diff --git a/acme/autocert/autocert.go b/acme/autocert/autocert.go index 877bfba..4b15816 100644 --- a/acme/autocert/autocert.go +++ b/acme/autocert/autocert.go @@ -141,6 +141,12 @@ type Manager struct { // If the Client's account key is already registered, Email is not used. Email string + // ForceRSA makes the Manager generate certificates with 2048-bit RSA keys. + // + // If false, a default is used. Currently the default + // is EC-based keys using the P-256 curve. + ForceRSA bool + clientMu sync.Mutex client *acme.Client // initialized by acmeClient method @@ -385,11 +391,21 @@ func (m *Manager) certState(domain string) (*certState, error) { if state, ok := m.state[domain]; ok { return state, nil } + // new locked state - key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + var ( + err error + key crypto.Signer + ) + if m.ForceRSA { + key, err = rsa.GenerateKey(rand.Reader, 2048) + } else { + key, err = ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + } if err != nil { return nil, err } + state := &certState{ key: key, locked: true, diff --git a/acme/autocert/autocert_test.go b/acme/autocert/autocert_test.go index 295f702..4bcd6d5 100644 --- a/acme/autocert/autocert_test.go +++ b/acme/autocert/autocert_test.go @@ -108,18 +108,41 @@ func decodePayload(v interface{}, r io.Reader) error { } func TestGetCertificate(t *testing.T) { - testGetCertificate(t, false) + man := &Manager{Prompt: AcceptTOS} + defer man.stopRenew() + hello := &tls.ClientHelloInfo{ServerName: "example.org"} + testGetCertificate(t, man, "example.org", hello) } func TestGetCertificate_trailingDot(t *testing.T) { - testGetCertificate(t, true) -} - -func testGetCertificate(t *testing.T, trailingDot bool) { - const domain = "example.org" man := &Manager{Prompt: AcceptTOS} defer man.stopRenew() + hello := &tls.ClientHelloInfo{ServerName: "example.org."} + testGetCertificate(t, man, "example.org", hello) +} +func TestGetCertificate_ForceRSA(t *testing.T) { + man := &Manager{ + Prompt: AcceptTOS, + Cache: make(memCache), + ForceRSA: true, + } + defer man.stopRenew() + hello := &tls.ClientHelloInfo{ServerName: "example.org"} + testGetCertificate(t, man, "example.org", hello) + + cert, err := man.cacheGet("example.org") + if err != nil { + t.Fatalf("man.cacheGet: %v", err) + } + if _, ok := cert.PrivateKey.(*rsa.PrivateKey); !ok { + t.Errorf("cert.PrivateKey is %T; want *rsa.PrivateKey", cert.PrivateKey) + } +} + +// tests man.GetCertificate flow using the provided hello argument. +// The domain argument is the expected domain name of a certificate request. +func testGetCertificate(t *testing.T, man *Manager, domain string, hello *tls.ClientHelloInfo) { // echo token-02 | shasum -a 256 // then divide result in 2 parts separated by dot tokenCertName := "4e8eb87631187e9ff2153b56b13a4dec.13a35d002e485d60ff37354b32f665d9.token.acme.invalid" @@ -212,14 +235,10 @@ func testGetCertificate(t *testing.T, trailingDot bool) { // simulate tls.Config.GetCertificate var tlscert *tls.Certificate done := make(chan struct{}) - go func(serverName string) { - if trailingDot { - serverName += "." - } - hello := &tls.ClientHelloInfo{ServerName: serverName} + go func() { tlscert, err = man.GetCertificate(hello) close(done) - }(domain) + }() select { case <-time.After(time.Minute): t.Fatal("man.GetCertificate took too long to return")