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

Add v2 module #13

Closed

Conversation

derekparker
Copy link
Contributor

There are more changes necessary to support Go 1.19 and
many of them are backwards incompatible as there are
function signature changes. This patch introduces a
v2 module which, when tagged, can be used by our Go
fork and allow us to maintain both versions for
pre-Go1.19 and the current Go1.19+ releases.

There are more changes necessary to support Go 1.19 and
many of them are backwards incompatible as there are
function signature changes. This patch introduces a
v2 module which, when tagged, can be used by our Go
fork and allow us to maintain both versions for
pre-Go1.19 and the current Go1.19+ releases.
@derekparker
Copy link
Contributor Author

Here's a diff of the differences between the v1 module and v2 to make review easier:

diff openssl/aes.go v2/openssl/aes.go
365a366,371
> // NewGCMTLS returns a GCM cipher specific to TLS
> // and should not be used for non-TLS purposes.
> func NewGCMTLS(c cipher.Block) (cipher.AEAD, error) {
> 	return c.(*aesCipher).NewGCMTLS()
> }
> 
Only in v2/openssl: bbig
diff openssl/ecdh_test.go v2/openssl/ecdh_test.go
1c1
< package openssl
---
> package openssl_test
4a5,6
> 	"github.com/golang-fips/openssl-fips/v2/openssl"
> 	"github.com/golang-fips/openssl-fips/v2/openssl/bbig"
10c12
< 	if !Enabled() {
---
> 	if !openssl.Enabled() {
56c58,60
< 	priv, err := NewPrivateKeyECDH("P-256", x, y, k)
---
> 	bx, by, bk := bbig.Enc(x), bbig.Enc(y), bbig.Enc(k)
> 
> 	priv, err := openssl.NewPrivateKeyECDH("P-256", bx, by, bk)
61c65
< 	derived, err := SharedKeyECDH(priv, peerPublicKey)
---
> 	derived, err := openssl.SharedKeyECDH(priv, peerPublicKey)
diff openssl/ecdsa.go v2/openssl/ecdsa.go
13,14d12
< 	"crypto"
< 	"encoding/asn1"
16d13
< 	"math/big"
22c19
< 	R, S *big.Int
---
> 	R, S BigInt
61c58
< func NewPublicKeyECDSA(curve string, X, Y *big.Int) (*PublicKeyECDSA, error) {
---
> func NewPublicKeyECDSA(curve string, X, Y BigInt) (*PublicKeyECDSA, error) {
75c72
< func newECKey(curve string, X, Y *big.Int) (*C.GO_EC_KEY, error) {
---
> func newECKey(curve string, X, Y BigInt) (*C.GO_EC_KEY, error) {
108c105
< func NewPrivateKeyECDSA(curve string, X, Y *big.Int, D *big.Int) (*PrivateKeyECDSA, error) {
---
> func NewPrivateKeyECDSA(curve string, X, Y BigInt, D BigInt) (*PrivateKeyECDSA, error) {
131,147c128
< func SignECDSA(priv *PrivateKeyECDSA, hash []byte, h crypto.Hash) (r, s *big.Int, err error) {
< 	// We could use ECDSA_do_sign instead but would need to convert
< 	// the resulting BIGNUMs to *big.Int form. If we're going to do a
< 	// conversion, converting the ASN.1 form is more convenient and
< 	// likely not much more expensive.
< 	sig, err := SignMarshalECDSA(priv, hash, h)
< 	if err != nil {
< 		return nil, nil, err
< 	}
< 	var esig ecdsaSignature
< 	if _, err := asn1.Unmarshal(sig, &esig); err != nil {
< 		return nil, nil, err
< 	}
< 	return esig.R, esig.S, nil
< }
< 
< func SignMarshalECDSA(priv *PrivateKeyECDSA, hash []byte, h crypto.Hash) ([]byte, error) {
---
> func SignMarshalECDSA(priv *PrivateKeyECDSA, hash []byte) ([]byte, error) {
151,163c132,134
< 	if h == crypto.Hash(0) {
< 		ok := C._goboringcrypto_internal_ECDSA_sign(0, base(hash), C.size_t(len(hash)), (*C.uint8_t)(unsafe.Pointer(&sig[0])), &sigLen, priv.key) > 0
< 		if !ok {
< 			return nil, NewOpenSSLError(("ECDSA_sign failed"))
< 		}
< 	} else {
< 		md := cryptoHashToMD(h)
< 		if md == nil {
< 			panic("boring: invalid hash")
< 		}
< 		if C._goboringcrypto_ECDSA_sign(md, base(hash), C.size_t(len(hash)), (*C.uint8_t)(unsafe.Pointer(&sig[0])), &sigLen, priv.key) == 0 {
< 			return nil, NewOpenSSLError("ECDSA_sign failed")
< 		}
---
> 	ok := C._goboringcrypto_internal_ECDSA_sign(0, base(hash), C.size_t(len(hash)), (*C.uint8_t)(unsafe.Pointer(&sig[0])), &sigLen, priv.key) > 0
> 	if !ok {
> 		return nil, NewOpenSSLError(("ECDSA_sign failed"))
164a136
> 
168,186c140,141
< 
< func VerifyECDSA(pub *PublicKeyECDSA, msg []byte, r, s *big.Int, h crypto.Hash) bool {
< 	// We could use ECDSA_do_verify instead but would need to convert
< 	// r and s to BIGNUM form. If we're going to do a conversion, marshaling
< 	// to ASN.1 is more convenient and likely not much more expensive.
< 	sig, err := asn1.Marshal(ecdsaSignature{r, s})
< 	if err != nil {
< 		return false
< 	}
< 	if h == crypto.Hash(0) {
< 		ok := C._goboringcrypto_internal_ECDSA_verify(0, base(msg), C.size_t(len(msg)), (*C.uint8_t)(unsafe.Pointer(&sig[0])), C.uint(len(sig)), pub.key) > 0
< 		runtime.KeepAlive(pub)
< 		return ok
< 	}
< 	md := cryptoHashToMD(h)
< 	if md == nil {
< 		panic("boring: invalid hash")
< 	}
< 	ok := C._goboringcrypto_ECDSA_verify(md, base(msg), C.size_t(len(msg)), (*C.uint8_t)(unsafe.Pointer(&sig[0])), C.uint(len(sig)), pub.key) > 0
---
> func VerifyECDSA(pub *PublicKeyECDSA, hash []byte, sig []byte) bool {
> 	ok := C._goboringcrypto_internal_ECDSA_verify(0, base(hash), C.size_t(len(hash)), (*C.uint8_t)(unsafe.Pointer(&sig[0])), C.uint(len(sig)), pub.key) > 0
191c146
< func GenerateKeyECDSA(curve string) (X, Y, D *big.Int, err error) {
---
> func GenerateKeyECDSA(curve string) (X, Y, D BigInt, err error) {
diff openssl/goopenssl.h v2/openssl/goopenssl.h
371a372,373
> DEFINEFUNC(int, BN_bn2le_padded, (uint8_t *out, size_t len, const BIGNUM *in), (out, len, in))
> DEFINEFUNC(GO_BIGNUM *, BN_le2bn, (const uint8_t *in, size_t len, BIGNUM *ret), (in, len, ret))
diff openssl/openssl.go v2/openssl/openssl.go
19c19
< 	"math/big"
---
> 	"math/bits"
39a40,44
> // A BigInt is the raw words from a BigInt.
> // This definition allows us to avoid importing math/big.
> // Conversion between BigInt and *big.Int is in crypto/internal/boring/bbig.
> type BigInt []uint
> 
186,188c191,201
< func bigToBN(x *big.Int) *C.GO_BIGNUM {
< 	raw := x.Bytes()
< 	return C._goboringcrypto_BN_bin2bn(base(raw), C.size_t(len(raw)), nil)
---
> func wbase(b BigInt) *C.uint8_t {
> 	if len(b) == 0 {
> 		return nil
> 	}
> 	return (*C.uint8_t)(unsafe.Pointer(&b[0]))
> }
> 
> const wordBytes = bits.UintSize / 8
> 
> func bigToBN(x BigInt) *C.GO_BIGNUM {
> 	return C._goboringcrypto_BN_le2bn(wbase(x), C.size_t(len(x)*wordBytes), nil)
191,194c204,209
< func bnToBig(bn *C.GO_BIGNUM) *big.Int {
< 	raw := make([]byte, C._goboringcrypto_BN_num_bytes(bn))
< 	n := C._goboringcrypto_BN_bn2bin(bn, base(raw))
< 	return new(big.Int).SetBytes(raw[:n])
---
> func bnToBig(bn *C.GO_BIGNUM) BigInt {
> 	x := make(BigInt, (C._goboringcrypto_BN_num_bytes(bn)+wordBytes-1)/wordBytes)
> 	if C._goboringcrypto_BN_bn2le_padded(wbase(x), C.size_t(len(x)*wordBytes), bn) == 0 {
> 		panic("boringcrypto: bignum conversion failed")
> 	}
> 	return x
197c212
< func bigToBn(bnp **C.GO_BIGNUM, b *big.Int) bool {
---
> func bigToBn(bnp **C.GO_BIGNUM, b BigInt) bool {
205,206c220
< 	raw := b.Bytes()
< 	bn := C._goboringcrypto_BN_bin2bn(base(raw), C.size_t(len(raw)), nil)
---
> 	bn := bigToBN(b)
diff openssl/rsa.go v2/openssl/rsa.go
16d15
< 	"math/big"
22,23c21,22
< func GenerateKeyRSA(bits int) (N, E, D, P, Q, Dp, Dq, Qinv *big.Int, err error) {
< 	bad := func(e error) (N, E, D, P, Q, Dp, Dq, Qinv *big.Int, err error) {
---
> func GenerateKeyRSA(bits int) (N, E, D, P, Q, Dp, Dq, Qinv BigInt, err error) {
> 	bad := func(e error) (N, E, D, P, Q, Dp, Dq, Qinv BigInt, err error) {
49c48
< func NewPublicKeyRSA(N, E *big.Int) (*PublicKeyRSA, error) {
---
> func NewPublicKeyRSA(N, E BigInt) (*PublicKeyRSA, error) {
80c79
< func NewPrivateKeyRSA(N, E, D, P, Q, Dp, Dq, Qinv *big.Int) (*PrivateKeyRSA, error) {
---
> func NewPrivateKeyRSA(N, E, D, P, Q, Dp, Dq, Qinv BigInt) (*PrivateKeyRSA, error) {

This patch adds a script to generate the diff between v1 and v2
and another script which can verify whether anything has been
changed and unaccounted for. This can be useful to run in CI to
ensure the versions stay in sync.
@ueno
Copy link
Collaborator

ueno commented Sep 7, 2022

Can we use branches/tags for maintaining multiple versions of the interface, instead of duplicating the code? While I understand the benefit of the current approach, i.e., both v1 and v2 versions are pulled in by create-secondary-patch.sh, where a single revision is hard-coded, I wonder if could be simpler.

Suppose we have v1 interface (for go 1.18 or earlier) in the 1.0 branch and the v2 interface (for go 1.19) is on the master branch in this repository. Stable versions are tagged as v1.0.0 or v2.0.0.

In golang-fips/go repository, create-secondary-patch.sh could be extended to take a tag as a command-line argument. setup-initial-patch.sh could also be adjusted to take a tag in addition to the Go upstream branch name.

With this setup, to populate patches for different branches:

$ git checkout -b go1.18-openssl-fips
$ scripts/setup-initial-patch.sh dev.boringcrypto.go1.18 v1.0.0
...
$ git checkout -b go1.19-openssl-fips
$ scripts/setup-initial-patch.sh release-branch.go1.19 v2.0.0

To resolve the require line in go.mod from a tag name, I guess go get could be used: go get -d github.com/golang-fips/[email protected].

@ueno
Copy link
Collaborator

ueno commented Sep 7, 2022

I've filed golang-fips/go#40 for the proposed script change.

@derekparker
Copy link
Contributor Author

Can we use branches/tags for maintaining multiple versions of the interface, instead of duplicating the code? While I understand the benefit of the current approach, i.e., both v1 and v2 versions are pulled in by create-secondary-patch.sh, where a single revision is hard-coded, I wonder if could be simpler.

Suppose we have v1 interface (for go 1.18 or earlier) in the 1.0 branch and the v2 interface (for go 1.19) is on the master branch in this repository. Stable versions are tagged as v1.0.0 or v2.0.0.

In golang-fips/go repository, create-secondary-patch.sh could be extended to take a tag as a command-line argument. setup-initial-patch.sh could also be adjusted to take a tag in addition to the Go upstream branch name.

With this setup, to populate patches for different branches:

$ git checkout -b go1.18-openssl-fips
$ scripts/setup-initial-patch.sh dev.boringcrypto.go1.18 v1.0.0
...
$ git checkout -b go1.19-openssl-fips
$ scripts/setup-initial-patch.sh release-branch.go1.19 v2.0.0

To resolve the require line in go.mod from a tag name, I guess go get could be used: go get -d github.com/golang-fips/[email protected].

That could work very well @ueno, however in talking with @dbenoit17 we discussed actually moving this whole repo to the v2 version. This would remove backwards compatibility, but we really haven't used this specific backend for anything other than testing the integration, and we don't really plan to ship this version of the backend until Go 1.19.

What do you think?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants