[go] crypto/tls: implement crypto.Signer and crypto.Decrypter

[Nick Sullivan (Gerrit)]

Nick Sullivan uploaded a change:
https://go-review.googlesource.com/3900

crypto/tls: implement crypto.Signer and crypto.Decrypter

Decrypter is a new interface to support opaque private keys that perform
decryption operations. This interface is analogous to the crypto.Signer
interface.

This change introduces the crypto.Decrypter interface and implements
the crypto.Decrypter interface for rsa.PrivateKey with both OAEP and
PKCS#1 v1.5 padding modes.

This change also implements crypto.Signer and crypto.Decrypter in tls so
opaque RSA and ECDSA keys can be used in both client and server
handshakes.

Change-Id: I433f649f84ed3c2148337d735cafd75f1d94a904
---
M src/crypto/crypto.go
M src/crypto/rsa/pkcs1v15.go
M src/crypto/rsa/rsa.go
M src/crypto/tls/common.go
M src/crypto/tls/handshake_server.go
M src/crypto/tls/key_agreement.go
6 files changed, 80 insertions(+), 19 deletions(-)



diff --git a/src/crypto/crypto.go b/src/crypto/crypto.go
index 59b23e9..d96d985 100644
--- a/src/crypto/crypto.go
+++ b/src/crypto/crypto.go
@@ -124,3 +124,19 @@
// hashing was done.
HashFunc() Hash
}
+
+// Decrypter is an interface for an opaque private key that can be used for
+// asymmetric decryption operations. For example, an RSA key kept in a
hardware
+// module.
+type Decrypter interface {
+ // Public returns the public key corresponding to the opaque,
+ // private key.
+ Public() PublicKey
+
+ // Decrypt decrypts a ciphertext into an existing slice. Behavior
+ // depends on the DecrypterOpts.
+ Decrypt(rand io.Reader, ciphertext, msg []byte, opts DecrypterOpts) (err
error)
+}
+
+// DecrypterOpts contains options for a Decrypter.
+type DecrypterOpts interface{}
diff --git a/src/crypto/rsa/pkcs1v15.go b/src/crypto/rsa/pkcs1v15.go
index 59e8bb5..e461473 100644
--- a/src/crypto/rsa/pkcs1v15.go
+++ b/src/crypto/rsa/pkcs1v15.go
@@ -14,6 +14,12 @@

// This file implements encryption and decryption using PKCS#1 v1.5
padding.

+// PKCS1v15Options implements crypto.DecrypterOpts and provides an option
to
+// perform TLS session key decryption in Decrypt.
+type PKCS1v15Options struct {
+ SessionKey bool
+}
+
// EncryptPKCS1v15 encrypts the given message with RSA and the padding
scheme from PKCS#1 v1.5.
// The message must be no longer than the length of the public modulus
minus 11 bytes.
// WARNING: use of this function to encrypt plaintexts other than session
keys
diff --git a/src/crypto/rsa/rsa.go b/src/crypto/rsa/rsa.go
index 2702311..56f2fb4 100644
--- a/src/crypto/rsa/rsa.go
+++ b/src/crypto/rsa/rsa.go
@@ -24,6 +24,14 @@
E int // public exponent
}

+// OAEPOptions provides the hash and the label to be used in OAEP
+// decryption through the crypto.Decrypter interface as a
+// crypto.DecrypterOpts
+type OAEPOptions struct {
+ hash hash.Hash
+ label []byte
+}
+
var (
errPublicModulus = errors.New("crypto/rsa: missing public modulus")
errPublicExponentSmall = errors.New("crypto/rsa: public exponent too
small")
@@ -77,6 +85,29 @@
return SignPKCS1v15(rand, priv, opts.HashFunc(), msg)
}

+// Decrypt decrypts with an RSA private key. If opts is of type
*OAEPOptions
+// then OAEP padding is expected, otherwise PKCS#1 v1.5 padding is
expected.
+func (priv *PrivateKey) Decrypt(rand io.Reader, ciphertext []byte, msg
[]byte, opts crypto.DecrypterOpts) (err error) {
+ switch o := opts.(type) {
+ case *OAEPOptions:
+ out, err := DecryptOAEP(o.hash, rand, priv, ciphertext, o.label)
+ if err != nil {
+ copy(msg, out)
+ }
+ return err
+ case *PKCS1v15Options:
+ if o.SessionKey {
+ return DecryptPKCS1v15SessionKey(rand, priv, ciphertext, msg)
+ } else {
+ out, err := DecryptPKCS1v15(rand, priv, ciphertext)
+ if err != nil {
+ copy(msg, out)
+ }
+ }
+ }
+ return
+}
+
type PrecomputedValues struct {
Dp, Dq *big.Int // D mod (P-1) (or mod Q-1)
Qinv *big.Int // Q^-1 mod P
diff --git a/src/crypto/tls/common.go b/src/crypto/tls/common.go
index e3c6004..14858ae 100644
--- a/src/crypto/tls/common.go
+++ b/src/crypto/tls/common.go
@@ -488,10 +488,10 @@
type Certificate struct {
Certificate [][]byte
// PrivateKey contains the private key corresponding to the public key
- // in Leaf. For a server, this must be a *rsa.PrivateKey or
- // *ecdsa.PrivateKey. For a client doing client authentication, this
- // can be any type that implements crypto.Signer (which includes RSA
- // and ECDSA private keys).
+ // in Leaf. For a server, this must implement either crypto.Decrypter
+ // (implemented by RSA private keys) or crypto.Signer (which includes
+ // RSA and ECDSA private keys). For a client doing client authentication,
+ // this can be any type that implements crypto.Signer.
PrivateKey crypto.PrivateKey
// OCSPStaple contains an optional OCSP response which will be served
// to clients that request it.
diff --git a/src/crypto/tls/handshake_server.go
b/src/crypto/tls/handshake_server.go
index a461334..20bcf6d 100644
--- a/src/crypto/tls/handshake_server.go
+++ b/src/crypto/tls/handshake_server.go
@@ -197,7 +197,11 @@
}
}

- _, hs.ecdsaOk = hs.cert.PrivateKey.(*ecdsa.PrivateKey)
+ priv, ok := hs.cert.PrivateKey.(crypto.Signer)
+ if !ok {
+ err = errors.New("tls: certificate private key does not implement
crypto.Signer")
+ }
+ _, hs.ecdsaOk = priv.Public().(*ecdsa.PublicKey)

if hs.checkForResumption() {
return true, nil
diff --git a/src/crypto/tls/key_agreement.go
b/src/crypto/tls/key_agreement.go
index 0974fc6..92d61ff 100644
--- a/src/crypto/tls/key_agreement.go
+++ b/src/crypto/tls/key_agreement.go
@@ -50,7 +50,11 @@
ciphertext = ckx.ciphertext[2:]
}

- err = rsa.DecryptPKCS1v15SessionKey(config.rand(),
cert.PrivateKey.(*rsa.PrivateKey), ciphertext, preMasterSecret)
+ priv, ok := cert.PrivateKey.(crypto.Decrypter)
+ if !ok {
+ return nil, errors.New("tls: certificate private key does not implement
crypto.Decrypter")
+ }
+ err = priv.Decrypt(config.rand(), ciphertext, preMasterSecret,
rsa.PKCS1v15Options{SessionKey: true})
if err != nil {
return nil, err
}
@@ -239,30 +243,30 @@
if err != nil {
return nil, err
}
+
+ priv, ok := cert.PrivateKey.(crypto.Signer)
+ if !ok {
+ err = errors.New("tls: certificate private key does not implement
crypto.Signer")
+ }
var sig []byte
switch ka.sigType {
case signatureECDSA:
- privKey, ok := cert.PrivateKey.(*ecdsa.PrivateKey)
+ _, ok := priv.Public().(*ecdsa.PublicKey)
if !ok {
- return nil, errors.New("ECDHE ECDSA requires an ECDSA server private
key")
+ return nil, errors.New("ECDHE ECDSA requires an ECDSA server key")
}
- r, s, err := ecdsa.Sign(config.rand(), privKey, digest)
- if err != nil {
- return nil, errors.New("failed to sign ECDHE parameters: " +
err.Error())
- }
- sig, err = asn1.Marshal(ecdsaSignature{r, s})
case signatureRSA:
- privKey, ok := cert.PrivateKey.(*rsa.PrivateKey)
+ _, ok := priv.Public().(*rsa.PublicKey)
if !ok {
- return nil, errors.New("ECDHE RSA requires a RSA server private key")
- }
- sig, err = rsa.SignPKCS1v15(config.rand(), privKey, hashFunc, digest)
- if err != nil {
- return nil, errors.New("failed to sign ECDHE parameters: " +
err.Error())
+ return nil, errors.New("ECDHE RSA requires a RSA server key")
}
default:
return nil, errors.New("unknown ECDHE signature algorithm")
}
+ sig, err = priv.Sign(config.rand(), digest, hashFunc)
+ if err != nil {
+ return nil, errors.New("failed to sign ECDHE parameters: " + err.Error())
+ }

skx := new(serverKeyExchangeMsg)
sigAndHashLen := 0

--
https://go-review.googlesource.com/3900

[Jacob Haven (Gerrit)]

Jacob Haven has posted comments on this change.

crypto/tls: implement crypto.Signer and crypto.Decrypter

Patch Set 1:

(1 comment)

https://go-review.googlesource.com/#/c/3900/1/src/crypto/rsa/rsa.go
File src/crypto/rsa/rsa.go:

Line 106: }
Should this return an error for unexpected opts? i.e.:

default: err = errors.New("rsa: invalid DecrypterOpts")


--
https://go-review.googlesource.com/3900
Gerrit-Reviewer: Jacob Haven <ja...@cloudflare.com>
Gerrit-HasComments: Yes

[Nick Sullivan (Gerrit)]

Nick Sullivan uploaded a new patch set:

https://go-review.googlesource.com/3900

crypto/tls: implement crypto.Signer and crypto.Decrypter

Decrypter is a new interface to support opaque private keys that perform
decryption operations. This interface is analogous to the crypto.Signer
interface.

This change introduces the crypto.Decrypter interface and implements
the crypto.Decrypter interface for rsa.PrivateKey with both OAEP and
PKCS#1 v1.5 padding modes.

This change also implements crypto.Signer and crypto.Decrypter in tls so
opaque RSA and ECDSA keys can be used in both client and server
handshakes.

Change-Id: I433f649f84ed3c2148337d735cafd75f1d94a904
---

A doc/progs/defer
A doc/progs/defer2
A doc/progs/eff_bytesize
A doc/progs/eff_qr
A doc/progs/eff_sequence
A doc/progs/eff_unused2
A doc/progs/error
A doc/progs/error2
A doc/progs/error3
A doc/progs/error4
A doc/progs/gobs2
A doc/progs/interface
A doc/progs/interface2
A doc/progs/json1
A doc/progs/json2
A doc/progs/json3
M src/crypto/crypto.go
M src/crypto/rsa/pkcs1v15.go
M src/crypto/rsa/pkcs1v15_test.go
M src/crypto/rsa/rsa.go
M src/crypto/rsa/rsa_test.go

M src/crypto/tls/common.go
M src/crypto/tls/handshake_server.go
M src/crypto/tls/key_agreement.go

24 files changed, 158 insertions(+), 19 deletions(-)

[Nick Sullivan (Gerrit)]

Nick Sullivan uploaded a new patch set:

https://go-review.googlesource.com/3900

crypto/tls: implement crypto.Signer and crypto.Decrypter

Decrypter is a new interface to support opaque private keys that perform
decryption operations. This interface is analogous to the crypto.Signer
interface.

This change introduces the crypto.Decrypter interface and implements
the crypto.Decrypter interface for rsa.PrivateKey with both OAEP and
PKCS#1 v1.5 padding modes.

This change also implements crypto.Signer and crypto.Decrypter in tls so
opaque RSA and ECDSA keys can be used in both client and server
handshakes.

Change-Id: I433f649f84ed3c2148337d735cafd75f1d94a904
---
M src/crypto/crypto.go
M src/crypto/rsa/pkcs1v15.go

M src/crypto/rsa/pkcs1v15_test.go
M src/crypto/rsa/rsa.go
M src/crypto/rsa/rsa_test.go

M src/crypto/tls/common.go
M src/crypto/tls/handshake_server.go
M src/crypto/tls/key_agreement.go

8 files changed, 158 insertions(+), 19 deletions(-)

[Adam Langley (Gerrit)]

Adam Langley uploaded a new patch set:
https://go-review.googlesource.com/3900

crypto/tls: implement crypto.Decrypter

Decrypter is an interface to support opaque private keys that perform

decryption operations. This interface is analogous to the crypto.Signer
interface.

This change introduces the crypto.Decrypter interface and implements
the crypto.Decrypter interface for rsa.PrivateKey with both OAEP and
PKCS#1 v1.5 padding modes.

Change-Id: I433f649f84ed3c2148337d735cafd75f1d94a904
---
M src/crypto/crypto.go
M src/crypto/rsa/pkcs1v15.go

M src/crypto/rsa/pkcs1v15_test.go
M src/crypto/rsa/rsa.go
4 files changed, 102 insertions(+), 8 deletions(-)


--
https://go-review.googlesource.com/3900
Gerrit-Reviewer: Adam Langley <a...@golang.org>
Gerrit-Reviewer: Jacob Haven <ja...@cloudflare.com>

[Adam Langley (Gerrit)]

Adam Langley has posted comments on this change.

crypto/tls: implement crypto.Decrypter

Patch Set 4: Run-TryBot+1

I went around on this for a while, but here's something that I think is at
least close.

1) I pondered the difference between have a struct or interface for the
options for a long time. Signer has an interface because it tries to be
somewhat detached from the underlying primitive and being able to pass the
crypto.Hash value is nice. If Decrypter was getting added at the same time
then I might have used structs for Signer.

However, Signer has shipped, so that's not changing. The question is when
whether Decrypter should use interfaces and my feeling is no. You can pass
nil now and get default behaviour but I don't see any commonality between
different Decrypter implementations so an interface does seem to make
sense. That this is different between Signer and Decrypter is a small wart.

2) I've removed the crypto/tls changes for now. Let's get this done first
and do that in a different change.


Please pull this change into a local branch (see the Cherry Pick under
Download at the top-right), make any tweaks and then `git commit -a --amend
--reset-author && git push origin HEAD:refs/for/master` to set the author
back to yourself.

--
https://go-review.googlesource.com/3900
Gerrit-Reviewer: Adam Langley <a...@golang.org>
Gerrit-Reviewer: Jacob Haven <ja...@cloudflare.com>

Gerrit-HasComments: No

[Gobot Gobot (Gerrit)]

Gobot Gobot has posted comments on this change.

crypto/tls: implement crypto.Decrypter

Patch Set 4:

TryBots beginning. Status page: http://107.178.219.46/

--
https://go-review.googlesource.com/3900
Gerrit-Reviewer: Adam Langley <a...@golang.org>

Gerrit-Reviewer: Gobot Gobot <go...@golang.org>

[Gobot Gobot (Gerrit)]

Gobot Gobot has posted comments on this change.

crypto/tls: implement crypto.Decrypter

Patch Set 4:

This change failed on plan9-386-gcepartial:
See
https://storage.googleapis.com/go-build-log/fc0da3cb/plan9-386-gcepartial_00eb3a74.log

Consult https://build.golang.org/ to see whether it's a new failure.

[Gobot Gobot (Gerrit)]

Gobot Gobot has posted comments on this change.

crypto/tls: implement crypto.Decrypter

Patch Set 4: TryBot-Result-1

1 of 12 TryBots failed: plan9-386-gcepartial

[Nick Sullivan (Gerrit)]

Nick Sullivan has posted comments on this change.

crypto/tls: implement crypto.Decrypter

Patch Set 5:

Changes look good. I have updated and rebased.

--
https://go-review.googlesource.com/3900
Gerrit-Reviewer: Adam Langley <a...@golang.org>
Gerrit-Reviewer: Gobot Gobot <go...@golang.org>
Gerrit-Reviewer: Jacob Haven <ja...@cloudflare.com>

Gerrit-Reviewer: Nick Sullivan <nicholas...@gmail.com>
Gerrit-HasComments: No

[Nick Sullivan (Gerrit)]

Reviewers: Gobot Gobot, Adam Langley

Nick Sullivan uploaded a new patch set:
https://go-review.googlesource.com/3900

crypto/rsa: implement crypto.Decrypter

Decrypter is an interface to support opaque private keys that perform

decryption operations. This interface is analogous to the crypto.Signer
interface.

This change introduces the crypto.Decrypter interface and implements
the crypto.Decrypter interface for rsa.PrivateKey with both OAEP and
PKCS#1 v1.5 padding modes.

Change-Id: I433f649f84ed3c2148337d735cafd75f1d94a904
---
M src/crypto/crypto.go
M src/crypto/rsa/pkcs1v15.go

M src/crypto/rsa/pkcs1v15_test.go
M src/crypto/rsa/rsa.go
4 files changed, 102 insertions(+), 8 deletions(-)

[Ben Burkert (Gerrit)]

Ben Burkert has posted comments on this change.

crypto/rsa: implement crypto.Decrypter

Patch Set 6:

Perhaps self-referential functions could remove some of the wonkiness of
DecrypterOpts as an empty interface:
http://commandcenter.blogspot.nl/2014/01/self-referential-functions-and-design.html

(here's a more applicable example:
http://godoc.org/github.com/benburkert/net.tune#ex-FastOpen)

--
https://go-review.googlesource.com/3900
Gerrit-Reviewer: Adam Langley <a...@golang.org>

Gerrit-Reviewer: Ben Burkert <b...@benburkert.com>

Gerrit-Reviewer: Gobot Gobot <go...@golang.org>
Gerrit-Reviewer: Jacob Haven <ja...@cloudflare.com>
Gerrit-Reviewer: Nick Sullivan <nicholas...@gmail.com>

Gerrit-HasComments: No

[Nick Sullivan (Gerrit)]

Nick Sullivan has posted comments on this change.

crypto/rsa: implement crypto.Decrypter

Patch Set 6:

Hi Ben,

Thanks for the suggestion. I'm not sure it makes a lot of sense here to
apply that design pattern. OAEPOptions and PKCS1v15DecryptOptions have
different semantics. Unless you have a code suggestion for how to
meaningfully do this, I would rather stick with the current submission.

Nick

[Jacob Haven (Gerrit)]

Jacob Haven has posted comments on this change.

crypto/rsa: implement crypto.Decrypter

Patch Set 6:

@agl What's the current status of this change? Are there any further
comments, or is the current patch set acceptable?
(I'll be handling this and the corresponding crypto/tls change while Nick
is away on vacation)

[Adam Langley (Gerrit)]

Adam Langley has posted comments on this change.

crypto/rsa: implement crypto.Decrypter

Patch Set 6: Code-Review+2

[Adam Langley (Gerrit)]

Adam Langley has submitted this change and it was merged.

crypto/rsa: implement crypto.Decrypter

Decrypter is an interface to support opaque private keys that perform

decryption operations. This interface is analogous to the crypto.Signer
interface.

This change introduces the crypto.Decrypter interface and implements
the crypto.Decrypter interface for rsa.PrivateKey with both OAEP and
PKCS#1 v1.5 padding modes.

Change-Id: I433f649f84ed3c2148337d735cafd75f1d94a904
Reviewed-on: https://go-review.googlesource.com/3900
Reviewed-by: Adam Langley <a...@golang.org>

---
M src/crypto/crypto.go
M src/crypto/rsa/pkcs1v15.go

M src/crypto/rsa/pkcs1v15_test.go
M src/crypto/rsa/rsa.go
4 files changed, 102 insertions(+), 8 deletions(-)

Approvals:
Adam Langley: Looks good to me, approved

[Adam Langley (Gerrit)]

Adam Langley has posted comments on this change.

crypto/rsa: implement crypto.Decrypter

Patch Set 7:

Please pull and send another CL with the crypto/tls changes.

--
https://go-review.googlesource.com/3900
Gerrit-Reviewer: Adam Langley <a...@golang.org>
Gerrit-Reviewer: Ben Burkert <b...@benburkert.com>
Gerrit-Reviewer: Gobot Gobot <go...@golang.org>
Gerrit-Reviewer: Jacob Haven <ja...@cloudflare.com>
Gerrit-Reviewer: Nick Sullivan <nicholas...@gmail.com>

Gerrit-HasComments: No