Commit Graph

72 Commits

Author SHA1 Message Date
Roland Shoemaker 61ed6d5c33 crypto/rsa,crypto/internal/boring: fix PSS salt handling
Fix the coversion between our sentinel salt length variables and the
BoringSSL versions in SignRSAPSS. We previously set -1 (hash length
equals salt length) when 0 was passed when we should've been setting
-2. This now matches the conversion that happens in VerifyRSAPSS. Also
adds a note documenting why we do this.

Additionally in non-Boring mode, properly handle passing of salt lengths
with a negative value which aren't one of the magic constants, returning
an error instead of panicking.

See https://commondatastorage.googleapis.com/chromium-boringssl-docs/rsa.h.html#RSA_sign_pss_mgf1
for the BoringSSL docs.

Fixes #54803

Change-Id: Id1bd14dcf0ef4733867367257830ed43e25ef882
Reviewed-on: https://go-review.googlesource.com/c/go/+/426659
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Damien Neil <dneil@google.com>
Run-TryBot: Roland Shoemaker <roland@golang.org>
2022-09-27 23:19:20 +00:00
Russ Cox b30ba3df9f crypto/internal/boring/bcache: make Cache type-safe using generics
Generics lets us write Cache[K, V] instead of using unsafe.Pointer,
which lets us remove all the uses of package unsafe around the
uses of the cache.

I tried to do Cache[*K, *V] instead of Cache[K, V] but that was not possible.

Change-Id: If3b54cf4c8d2a44879a5f343fd91ecff096537e9
Reviewed-on: https://go-review.googlesource.com/c/go/+/423357
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Auto-Submit: Russ Cox <rsc@golang.org>
2022-08-18 00:30:19 +00:00
Russ Cox 8f9bfa9b7b crypto/internal/boring: factor Cache into crypto/internal/boring/bcache
Requested by the maintainers of the OpenSSL-based fork of Go+BoringCrypto,
to make maintaining that fork easier.

Change-Id: I770e70ecc12b589034da31edecf59c73b2c6e1dd
Reviewed-on: https://go-review.googlesource.com/c/go/+/407135
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Russ Cox <rsc@golang.org>
2022-06-27 11:50:31 +00:00
Russ Cox ea5d7cbc26 all: boringcrypto post-merge cleanup
This CL addresses the comments on CL 403154.

For #51940.

Change-Id: I99bb3530916d469077bfbd53095bfcd1d2aa82ef
Reviewed-on: https://go-review.googlesource.com/c/go/+/403976
Reviewed-by: Roland Shoemaker <roland@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
2022-06-06 19:10:07 +00:00
Russ Cox e845f572ec [dev.boringcrypto] crypto/ecdsa, crypto/rsa: use boring.Cache
In the original BoringCrypto port, ecdsa and rsa's public and private
keys added a 'boring unsafe.Pointer' field to cache the BoringCrypto
form of the key. This led to problems with code that “knew” the layout
of those structs and in particular that they had no unexported fields.

In response, as an awful kludge, I changed the compiler to pretend
that field did not exist when laying out reflect data. Because we want
to merge BoringCrypto in the main tree, we need a different solution.
Using boring.Cache is that solution.

For #51940.

Change-Id: Ideb2b40b599a1dc223082eda35a5ea9abcc01e30
Reviewed-on: https://go-review.googlesource.com/c/go/+/395883
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Roland Shoemaker <roland@golang.org>
2022-04-29 14:23:32 +00:00
Russ Cox 9e9c7a0aec [dev.boringcrypto] crypto/..., go/build: align deps test with standard rules
One annoying difference between dev.boringcrypto and master is that
there is not a clear separation between low-level (math/big-free)
crypto and high-level crypto, because crypto/internal/boring imports
both encoding/asn1 and math/big.

This CL removes both those problematic imports and aligns the
dependency rules in the go/build test with the ones in the main
branch.

To remove encoding/asn1, the crypto/internal/boring APIs change to
accepting and returning encoded ASN.1, leaving crypto/ecdsa to do the
marshaling and unmarshaling, which it already contains code to do.

To remove math/big, the crypto/internal/boring package defines
type BigInt []uint, which is the same representation as a big.Int's
internal storage. The new package crypto/internal/boring/bbig provides
conversions between BigInt and *big.Int. The boring package can then
be in the low-level crypto set, and any package needing to use bignum
APIs (necessarily in the high-level crypto set) can import bbig to
convert.

To simplify everything we hide from the test the fact that
crypto/internal/boring imports cgo. Better to pretend it doesn't and
keep the prohibitions that other packages like crypto/aes must not use
cgo (outside of BoringCrypto).

	$ git diff origin/master src/go/build/deps_test.go
	diff --git a/src/go/build/deps_test.go b/src/go/build/deps_test.go
	index 6ce872e297..a63979cc93 100644
	--- a/src/go/build/deps_test.go
	+++ b/src/go/build/deps_test.go
	@@ -402,9 +402,13 @@ var depsRules = `
	 	NET, log
	 	< net/mail;

	+	NONE < crypto/internal/boring/sig;
	+	sync/atomic < crypto/internal/boring/fipstls;
	+	crypto/internal/boring/sig, crypto/internal/boring/fipstls < crypto/tls/fipsonly;
	+
	 	# CRYPTO is core crypto algorithms - no cgo, fmt, net.
	 	# Unfortunately, stuck with reflect via encoding/binary.
	-	encoding/binary, golang.org/x/sys/cpu, hash
	+	crypto/internal/boring/sig, encoding/binary, golang.org/x/sys/cpu, hash
	 	< crypto
	 	< crypto/subtle
	 	< crypto/internal/subtle
	@@ -413,6 +417,8 @@ var depsRules = `
	 	< crypto/ed25519/internal/edwards25519/field, golang.org/x/crypto/curve25519/internal/field
	 	< crypto/ed25519/internal/edwards25519
	 	< crypto/cipher
	+	< crypto/internal/boring
	+	< crypto/boring
	 	< crypto/aes, crypto/des, crypto/hmac, crypto/md5, crypto/rc4,
	 	  crypto/sha1, crypto/sha256, crypto/sha512
	 	< CRYPTO;
	@@ -421,6 +427,7 @@ var depsRules = `

	 	# CRYPTO-MATH is core bignum-based crypto - no cgo, net; fmt now ok.
	 	CRYPTO, FMT, math/big, embed
	+	< crypto/internal/boring/bbig
	 	< crypto/rand
	 	< crypto/internal/randutil
	 	< crypto/ed25519
	@@ -443,7 +450,8 @@ var depsRules = `
	 	< golang.org/x/crypto/hkdf
	 	< crypto/x509/internal/macos
	 	< crypto/x509/pkix
	-	< crypto/x509
	+	< crypto/x509;
	+	crypto/internal/boring/fipstls, crypto/x509
	 	< crypto/tls;

	 	# crypto-aware packages
	@@ -653,6 +661,9 @@ func findImports(pkg string) ([]string, error) {
	 	}
	 	var imports []string
	 	var haveImport = map[string]bool{}
	+	if pkg == "crypto/internal/boring" {
	+		haveImport["C"] = true // kludge: prevent C from appearing in crypto/internal/boring imports
	+	}
	 	fset := token.NewFileSet()
	 	for _, file := range files {
	 		name := file.Name()

For #51940.

Change-Id: I26fc752484310d77d22adb06495120a361568d04
Reviewed-on: https://go-review.googlesource.com/c/go/+/395877
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Roland Shoemaker <roland@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
2022-04-29 14:23:28 +00:00
Russ Cox f4c0f42f99 [dev.boringcrypto] all: add boringcrypto build tags
A plain make.bash in this tree will produce a working,
standard Go toolchain, not a BoringCrypto-enabled one.

The BoringCrypto-enabled one will be created with:

	GOEXPERIMENT=boringcrypto ./make.bash

For #51940.

Change-Id: Ia9102ed993242eb1cb7f9b93eca97e81986a27b3
Reviewed-on: https://go-review.googlesource.com/c/go/+/395881
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2022-04-29 14:23:22 +00:00
Chressie Himpel e07d63964b [dev.boringcrypto] all: merge master into dev.boringcrypto
Change-Id: I52009bf809dda4fbcff03aa82d0ea8aa2a978fa2
2022-04-20 16:57:46 +02:00
Russ Cox 19309779ac all: gofmt main repo
[This CL is part of a sequence implementing the proposal #51082.
The design doc is at https://go.dev/s/godocfmt-design.]

Run the updated gofmt, which reformats doc comments,
on the main repository. Vendored files are excluded.

For #51082.

Change-Id: I7332f099b60f716295fb34719c98c04eb1a85407
Reviewed-on: https://go-review.googlesource.com/c/go/+/384268
Reviewed-by: Jonathan Amsterdam <jba@google.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2022-04-11 16:34:30 +00:00
Russ Cox 81431c7aa7 all: replace `` and '' with “ (U+201C) and ” (U+201D) in doc comments
go/doc in all its forms applies this replacement when rendering
the comments. We are considering formatting doc comments,
including doing this replacement as part of the formatting.
Apply it to our source files ahead of time.

For #51082.

Change-Id: Ifcc1f5861abb57c5d14e7d8c2102dfb31b7a3a19
Reviewed-on: https://go-review.googlesource.com/c/go/+/384262
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2022-04-05 17:52:29 +00:00
Filippo Valsorda ed1f812cef [dev.boringcrypto] all: merge commit 9d0819b27c (CL 314609) into dev.boringcrypto
There used to be two BoringCrypto-specific behaviors related to cipher
suites in crypto/tls:

1. in FIPS-only mode, only a restricted set of AES ciphers is allowed

2. NOT in FIPS-only mode, AES would be prioritized over ChaCha20 even if
   AES hardware was not available

The motivation of (2) is unclear, and BoringSSL doesn't have equivalent
logic. This merge drops (2), and keeps (1). Note that the list of
FIPS-only ciphers does not have priority semantics anymore, but the
default logic still sorts them the same way as they used to be.

Change-Id: I50544011085cfa2b087f323aebf5338c0bd2dd33
2021-05-13 12:59:22 -04:00
Himanshu Kishna Srivastava 8f676144ad crypto/rsa: fix salt length calculation with PSSSaltLengthAuto
When PSSSaltLength is set, the maximum salt length must equal:

    (modulus_key_size - 1 + 7)/8 - hash_length - 2
and for example, with a 4096 bit modulus key, and a SHA-1 hash,
it should be:

     (4096 -1 + 7)/8 - 20 - 2 = 490
Previously we'd encounter this error:

     crypto/rsa: key size too small for PSS signature

Fixes #42741

Change-Id: I18bb82c41c511d564b3f4c443f4b3a38ab010ac5
Reviewed-on: https://go-review.googlesource.com/c/go/+/302230
Reviewed-by: Emmanuel Odeke <emmanuel@orijtech.com>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Trust: Emmanuel Odeke <emmanuel@orijtech.com>
Run-TryBot: Emmanuel Odeke <emmanuel@orijtech.com>
TryBot-Result: Go Bot <gobot@golang.org>
2021-03-29 15:20:11 +00:00
Richard Pickering 9de49ae01a crypto/rsa: correct EncryptOAEP doc comment
Fixes #44777

Corrected the documentation comment on the EncryptOAEP function from
'if a given public key is used to decrypt two types of messages' to
'if a given public key is used to encrypt two types of messages'.

Change-Id: I02aff90d0414960eae72352c0e4d8ba2e8f8eca6
GitHub-Last-Rev: ea28663f87
GitHub-Pull-Request: golang/go#45032
Reviewed-on: https://go-review.googlesource.com/c/go/+/301714
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Robert Findley <rfindley@google.com>
2021-03-18 13:31:52 +00:00
Dmitri Shuralyov d85ef2b979 [dev.boringcrypto] all: merge master into dev.boringcrypto
Change-Id: I61d6a6d4959fdea8339b9d666385bf6b4ed49d03
2020-07-09 21:23:49 -04:00
Katie Hockman 4b09c8ad6f crypto: fix PKCS space in docs
Change-Id: Iee53acb963a889410f8c6daaa9a7841f1b12c6fb
Reviewed-on: https://go-review.googlesource.com/c/go/+/241497
Run-TryBot: Katie Hockman <katie@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
2020-07-08 17:21:49 +00:00
Dmitri Shuralyov dd98c0ca3f [dev.boringcrypto] all: merge master into dev.boringcrypto
Change-Id: I083d1e4e997b30d9fab10940401eaf160e36f6c1
2020-05-07 23:31:52 -04:00
Dmitri Shuralyov a9d2e3abf7 [dev.boringcrypto] all: merge master into dev.boringcrypto
Change-Id: Idd59c37d2fd759b0f73d2ee01b30f72ef4e9aee8
2020-05-07 18:24:58 -04:00
Filippo Valsorda 36c94f8421 [dev.boringcrypto] crypto/internal/boring: reject short signatures in VerifyRSAPKCS1v15
This matches the new crypto/rsa behavior introduced in CL 226203.

Updates #21896

Change-Id: If04eeff933d7310c2baa0f8fd26907892c2397fd
Reviewed-on: https://go-review.googlesource.com/c/go/+/227651
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Katie Hockman <katie@golang.org>
2020-05-07 12:47:08 -04:00
Filippo Valsorda a8e83d51a0 crypto/rsa,crypto/ecdsa,crypto/ed25519: implement PrivateKey.Equal
Fixes #38190

Change-Id: I10766068ee18974e81b3bd78ee0b4d83cc9d1a8c
Reviewed-on: https://go-review.googlesource.com/c/go/+/231417
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Katie Hockman <katie@golang.org>
2020-05-05 18:05:10 +00:00
Filippo Valsorda c9d5f60eaa math/big: add (*Int).FillBytes
Replaced almost every use of Bytes with FillBytes.

Note that the approved proposal was for

    func (*Int) FillBytes(buf []byte)

while this implements

    func (*Int) FillBytes(buf []byte) []byte

because the latter was far nicer to use in all callsites.

Fixes #35833

Change-Id: Ia912df123e5d79b763845312ea3d9a8051343c0a
Reviewed-on: https://go-review.googlesource.com/c/go/+/230397
Reviewed-by: Robert Griesemer <gri@golang.org>
2020-05-05 00:36:44 +00:00
Filippo Valsorda e067ce5225 [dev.boringcrypto] all: merge master into dev.boringcrypto
Change-Id: I2dcec316fd08d91db4183fb9d3b9afde65cc248f
2020-04-08 17:48:41 -04:00
Filippo Valsorda 9baafabac9 crypto/rsa: refactor RSA-PSS signing and verification
Cleaned up for readability and consistency.

There is one tiny behavioral change: when PSSSaltLengthEqualsHash is
used and both hash and opts.Hash were set, hash.Size() was used for the
salt length instead of opts.Hash.Size(). That's clearly wrong because
opts.Hash is documented to override hash.

Change-Id: I3e25dad933961eac827c6d2e3bbfe45fc5a6fb0e
Reviewed-on: https://go-review.googlesource.com/c/go/+/226937
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Katie Hockman <katie@golang.org>
2020-04-02 17:46:57 +00:00
Roland Shoemaker 5db079d2e5 crypto/rsa: reject invalid length PKCS#1v1.5 signatures
Per RFC 8017, reject signatures which are not the same length as the RSA
modulus. This matches the behavior of SignPKCS1v15 which properly left pads
the signatures it generates to the size of the modulus.

Fixes #21896

Change-Id: I2c42a0b24cf7fff158ece604b6f0c521a856d932
GitHub-Last-Rev: 6040f79906
GitHub-Pull-Request: golang/go#38140
Reviewed-on: https://go-review.googlesource.com/c/go/+/226203
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-03-31 17:41:35 +00:00
Filippo Valsorda b5f2c0f502 crypto/rsa,crypto/ecdsa,crypto/ed25519: implement PublicKey.Equal
This makes all modern public keys in the standard library implement a
common interface (below) that can be used by applications for better
type safety and allows for checking that public (and private keys via
Public()) are equivalent.

interface {
    Equal(crypto.PublicKey) bool
}

Equality for ECDSA keys is complicated, we take a strict interpretation
that works for all secure applications (the ones not using the
unfortunate non-constant time CurveParams implementation) and fails
closed otherwise.

Tests in separate files to make them x_tests and avoid an import loop
with crypto/x509.

Re-landing of CL 223754. Dropped the test that was assuming named curves
are not implemented by CurveParams, because it's not true for all
curves, and anyway is not a property we need to test. There is still a
test to check that different curves make keys not Equal.

Fixes #21704
Fixes #38035

Reviewed-on: https://go-review.googlesource.com/c/go/+/223754
Reviewed-by: Katie Hockman <katie@golang.org>
Change-Id: I736759b145bfb4f7f8eecd78c324315d5a05385c
Reviewed-on: https://go-review.googlesource.com/c/go/+/225460
Run-TryBot: Filippo Valsorda <filippo@golang.org>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-03-26 15:00:36 +00:00
Bryan C. Mills fb2a6343de Revert "crypto/rsa,crypto/ecdsa,crypto/ed25519: implement PublicKey.Equal"
This reverts CL 223754.

Reason for revert: new tests are failing on all longtest builders.

Change-Id: I2257d106c132f3a02c0af6b20061d4f9a8093c4f
Reviewed-on: https://go-review.googlesource.com/c/go/+/225077
Run-TryBot: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Katie Hockman <katie@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-03-23 21:15:27 +00:00
Filippo Valsorda 5c9bd499e1 crypto/rsa,crypto/ecdsa,crypto/ed25519: implement PublicKey.Equal
This makes all modern public keys in the standard library implement a
common interface (below) that can be used by applications for better
type safety and allows for checking that public (and private keys via
Public()) are equivalent.

interface {
    Equal(crypto.PublicKey) bool
}

Equality for ECDSA keys is complicated, we take a strict interpretation
that works for all secure applications (the ones not using the
unfortunate non-constant time CurveParams implementation) and fails
closed otherwise.

Tests in separate files to make them x_tests and avoid an import loop
with crypto/x509.

Fixes #21704

Change-Id: Id5379c96384a11c5afde0614955360e7470bb1c4
Reviewed-on: https://go-review.googlesource.com/c/go/+/223754
Reviewed-by: Katie Hockman <katie@golang.org>
2020-03-23 17:56:24 +00:00
Katie Hockman ff197f326f [dev.boringcrypto] all: merge master into dev.boringcrypto
Change-Id: I3cd94be655e5374b52494f756ff087352705da6d
2019-09-03 17:05:23 -04:00
Dominik Honnef c4ca60313e all: change some function documentation to be more idiomatic
Change-Id: I932de9bb061a8ba3332ef03207983e8b98d6f1e5
Reviewed-on: https://go-review.googlesource.com/c/go/+/187918
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2019-07-28 18:09:57 +00:00
Filippo Valsorda 9bf9e7d4b2 [dev.boringcrypto] crypto: move crypto/internal/boring imports to reduce merge conflicts
As suggested by dmitshur@, move them to their own block so they don't
conflict with changes in the upstream imports.

Change-Id: Id46fb7c766066c406023b0355f4c3c860166f0fe
Reviewed-on: https://go-review.googlesource.com/c/go/+/181277
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
2019-06-09 19:24:48 +00:00
Filippo Valsorda 324f8365be [dev.boringcrypto] all: merge master into dev.boringcrypto
Change-Id: I29cf70cab6b4c28891dce2a3ccf18b690ff568a0
2019-06-05 06:23:38 -04:00
Russ Cox 02fe6ba958 all: remove PEM-encoded private keys from tests
Gerrit is complaining about pushes that affect these files
and forcing people to use -o nokeycheck, which defeats
the point of the check. Hide the keys from this kind of scan
by marking them explicitly as testing keys.

This is a little annoying but better than training everyone
who ever edits one of these test files to reflexively override
the Gerrit check.

The only remaining keys explicitly marked as private instead
of testing are in examples, and there's not much to do
about those. Hopefully they are not edited as much.

Change-Id: I4431592b5266cb39fe6a80b40e742d97da803a0b
Reviewed-on: https://go-review.googlesource.com/c/go/+/178178
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2019-05-21 20:03:55 +00:00
Filippo Valsorda 623650b27a [dev.boringcrypto] all: merge master into dev.boringcrypto
Change-Id: I218ba1b89a2df6e4335c6a5846889d9a04affe5d
2018-10-15 17:09:34 -04:00
Brad Fitzpatrick da0d1a44ba all: use strings.ReplaceAll and bytes.ReplaceAll where applicable
I omitted vendor directories and anything necessary for bootstrapping.
(Tested by bootstrapping with Go 1.4)

Updates #27864

Change-Id: I7d9b68d0372d3a34dee22966cca323513ece7e8a
Reviewed-on: https://go-review.googlesource.com/137856
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2018-09-26 22:14:25 +00:00
Filippo Valsorda 77db076129 [dev.boringcrypto] all: merge master into dev.boringcrypto
Add a couple of skips for slow js/wasm tests.

Change-Id: Ic95256b1d3c6e5e2f0cc536fad51e914d31cda9e
2018-07-03 15:17:00 -04:00
Filippo Valsorda b77f5e4c85 [dev.boringcrypto] crypto/rsa: drop random source reading emulation
Now that the standard library behavior in reading from the randomness
source is not reliable thanks to randutil.MaybeReadByte, we don't need
to emulate its behavior.

Also, since boring.RandReader is never deterministic, add an early exit
to randutil.MaybeReadByte.

Change-Id: Ie53e45ee64af635595181f71abd3c4340c600907
Reviewed-on: https://go-review.googlesource.com/117555
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
2018-06-13 21:59:09 +00:00
Filippo Valsorda a4b7722ffa [dev.boringcrypto] all: merge master into dev.boringcrypto
Conflicts due to randutil.MaybeReadByte (kept at the top for patch
maintainability and consistency):
	src/crypto/ecdsa/ecdsa.go
	src/crypto/rsa/pkcs1v15.go
	src/crypto/rsa/rsa.go

Change-Id: I03a2de541e68a1bbdc48590ad7c01fbffbbf4a2b
2018-06-08 18:43:33 -04:00
Adam Langley 6269dcdc24 crypto: randomly read an extra byte of randomness in some places.
Code has ended up depending on things like RSA's key generation being
deterministic given a fixed random Reader. This was never guaranteed and
would prevent us from ever changing anything about it.

This change makes certain calls randomly (based on the internal
fastrand) read an extra byte from the random Reader. This helps to
ensure that code does not depend on internal details.

I've not added this call in the key generation of ECDSA and DSA because,
in those cases, key generation is so obvious that it probably is
acceptable to do the obvious thing and not worry about code that depends
on that.

This does not affect tests that use a Reader of constant bytes (e.g. a
zeroReader) because shifting such a stream is a no-op. The stdlib uses
this internally (which is fine because it can be atomically updated if
the crypto libraries change).

It is possible that external tests could be doing the same and would
thus break if we ever, say, tweaked the way RSA key generation worked.
I feel that addressing that would be more effort than it's worth.

Fixes #21915

Change-Id: I84cff2e249acc921ad6eb5527171e02e6d39c530
Reviewed-on: https://go-review.googlesource.com/64451
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
2018-06-07 15:09:25 +00:00
Tim Cooper 161874da2a all: update comment URLs from HTTP to HTTPS, where possible
Each URL was manually verified to ensure it did not serve up incorrect
content.

Change-Id: I4dc846227af95a73ee9a3074d0c379ff0fa955df
Reviewed-on: https://go-review.googlesource.com/115798
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
2018-06-01 21:52:00 +00:00
Filippo Valsorda 019a994e32 [dev.boringcrypto] crypto/rsa: fix boringFakeRandomBlind to work with (*big.Int).ModInverse
http://golang.org/cl/108996 removed the local modInverse and its call in
decrypt in favor of (*big.Int).ModInverse. boringFakeRandomBlind copies
decrypt, so it needs to be updated as well.

Change-Id: I59a6c17c2fb9cc7f38cbb59dd9ed11846737d220
Reviewed-on: https://go-review.googlesource.com/113676
Run-TryBot: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2018-05-18 15:13:45 +00:00
Filippo Valsorda a3f9ce3313 [dev.boringcrypto] all: merge master into dev.boringcrypto
Conflicts due to simple variable renames (d <-> d0):
    src/crypto/sha1/sha1.go
    src/crypto/sha256/sha256.go
    src/crypto/sha512/sha512.go

Change-Id: I437df180a527fb3ec8b47927ee71960d5d200b76
2018-05-17 21:04:07 -04:00
Brian Kessler 4d44a87243 math/big: return nil for nonexistent ModInverse
Currently, the behavior of z.ModInverse(g, n) is undefined
when g and n are not relatively prime.  In that case, no
ModInverse exists which can be easily checked during the
computation of the ModInverse.  Because the ModInverse does
not indicate whether the inverse exists, there are reimplementations
of a "checked" ModInverse in crypto/rsa.  This change removes the
undefined behavior.  If the ModInverse does not exist, the receiver z
is unchanged and the return value is nil. This matches the behavior of
ModSqrt for the case where the square root does not exist.

name          old time/op    new time/op    delta
ModInverse-4    2.40µs ± 4%    2.22µs ± 0%   -7.74%  (p=0.016 n=5+4)

name          old alloc/op   new alloc/op   delta
ModInverse-4    1.36kB ± 0%    1.17kB ± 0%  -14.12%  (p=0.008 n=5+5)

name          old allocs/op  new allocs/op  delta
ModInverse-4      10.0 ± 0%       9.0 ± 0%  -10.00%  (p=0.008 n=5+5)

Fixes #24922

Change-Id: If7f9d491858450bdb00f1e317152f02493c9c8a8
Reviewed-on: https://go-review.googlesource.com/108996
Run-TryBot: Robert Griesemer <gri@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
2018-04-30 23:45:27 +00:00
Filippo Valsorda c90e0504c0 crypto/rsa: improve the comment of PublicKey.Size
Change-Id: Ic507cb740395e76d1d011a5a2f395b96c3d172a2
Reviewed-on: https://go-review.googlesource.com/105915
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2018-04-09 19:03:04 +00:00
Pascal S. de Kloe daa2d54773 crypto/rsa: add PublicKey.Size accessor
Provide the fixed size from the key pair.

Change-Id: I365c8d0f7d915229ef089e46458d4c83273fc648
Reviewed-on: https://go-review.googlesource.com/103876
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2018-04-03 15:55:48 +00:00
Filippo Valsorda c0094338fb crypto/rsa: improve error message for keys too short for PSS
Fixes #23736

Change-Id: I850d91a512394c4292927d51c475064bfa4e3053
Reviewed-on: https://go-review.googlesource.com/92815
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2018-02-14 15:31:22 +00:00
Russ Cox 185e6094fd [dev.boringcrypto] all: merge master (nearly Go 1.10 beta 1) into dev.boringcrypto
This is a git merge of master into dev.boringcrypto.

The branch was previously based on release-branch.go1.9,
so there are a handful of spurious conflicts that would
also arise if trying to merge master into release-branch.go1.9
(which we never do). Those have all been resolved by taking
the original file from master, discarding any Go 1.9-specific
edits.

all.bash passes on darwin/amd64, which is to say without
actually using BoringCrypto.

Go 1.10-related fixes to BoringCrypto itself will be in a followup CL.
This CL is just the merge.

Change-Id: I4c97711fec0fb86761913dcde28d25c001246c35
2017-12-06 01:03:36 -05:00
Adam Langley 507ca082d1 crypto/{ecdsa,rsa}: rename argument to PrivateKey.Sign.
The crypto.Signer interface takes pre-hased messages for ECDSA and RSA,
but the argument in the implementations was called “msg”, not “digest”,
which is confusing.

This change renames them to help clarify the intended use.

Change-Id: Ie2fb8753ca5280e493810d211c7c66223f94af88
Reviewed-on: https://go-review.googlesource.com/70950
Reviewed-by: Filippo Valsorda <hi@filippo.io>
2017-10-29 19:45:11 +00:00
Russ Cox 2ba76155cd [dev.boringcrypto] crypto/internal/boring: fix finalizer-induced crashes
All the finalizer-enabled C wrappers must be careful to use
runtime.KeepAlive to ensure the C wrapper object (a Go object)
lives through the end of every C call using state that the
wrapper's finalizer would free.

This CL makes the wrappers appropriately careful.

The test proves that this is the bug I was chasing in a
separate real program, and that the KeepAlives fix it.
I did not write a test of every possible operation.

Change-Id: I627007e480f16adf8396e7f796b54e5525d9ea80
Reviewed-on: https://go-review.googlesource.com/64870
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Adam Langley <agl@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2017-09-20 18:01:53 +00:00
Russ Cox a929f3a04d [dev.boringcrypto] crypto/rsa: fix boring GenerateKey to set non-nil Precomputed.CRTValues
This matches the standard GenerateKey and more importantly Precompute,
so that if you generate a key and then store it, read it back, call Precompute
on the new copy, and then do reflect.DeepEqual on the two copies, they
will match. Before this CL, the original key had CRTValues == nil and the
reconstituted key has CRTValues != nil (but len(CRTValues) == 0).

Change-Id: I1ddc64342a50a1b65a48d827e4d564f1faab1945
Reviewed-on: https://go-review.googlesource.com/63914
Reviewed-by: Adam Langley <agl@golang.org>
2017-09-18 00:31:43 +00:00
Russ Cox c9e2d9eb06 [dev.boringcrypto] crypto/rsa: add test for, fix observable reads from custom randomness
In routines like GenerateKey, where bits from the randomness source have a
visible effect on the output, we bypass BoringCrypto if given a non-standard
randomness source (and also assert that this happens only during tests).

In the decryption paths, the randomness source is only for blinding and has
no effect on the output, so we unconditionally invoke BoringCrypto, letting it
use its own randomness source as it sees fit. This in turn lets us verify that
the non-BoringCrypto decryption function is never called, not even in tests.

Unfortunately, while the randomness source has no visible effect on the
decrypt operation, the decrypt operation does have a visible effect on
the randomness source. If decryption doesn't use the randomness source,
and it's a synthetic stream, then a future operation will read a different
position in the stream and may produce different output. This happens
in tests more often than you'd hope.

To keep behavior of those future operations unchanged while still
ensuring that the original decrypt is never called, this CL adds a
simulation of the blinding preparation, to discard the right amount
from the random source before invoking BoringCrypto.

Change-Id: If2f87b856c811b59b536187c93efa99a97721419
Reviewed-on: https://go-review.googlesource.com/63912
Reviewed-by: Adam Langley <agl@golang.org>
2017-09-18 00:26:15 +00:00
Russ Cox 07f6ce9d39 [dev.boringcrypto] crypto/internal/boring: handle RSA verification of short signatures
The standard Go crypto/rsa allows signatures to be shorter
than the RSA modulus and assumes leading zeros.
BoringCrypto does not, so supply the leading zeros explicitly.

This fixes the golang.org/x/crypto/openpgp tests.

Change-Id: Ic8b18d6beb0e02992a0474f5fdb2b73ccf7098cf
Reviewed-on: https://go-review.googlesource.com/62170
Reviewed-by: Adam Langley <agl@golang.org>
2017-09-18 00:07:51 +00:00