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.

Sign up for GitHub

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

Jump to bottom

Test GoThemis with Go 1.11~1.14 on CircleCI #595

Merged
merged 8 commits into from
Feb 27, 2020
Merged

Test GoThemis with Go 1.11~1.14 on CircleCI #595

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
0650b32
Run GoThemis tests across multiple Go versions
ilammy Feb 26, 2020
f8bf6c1
Regression test for bytes.Buffer bug in go 1.11
ilammy Feb 26, 2020
2914dd7
Sanitize input buffers in go 1.11 and earlier
ilammy Feb 26, 2020
5b2f623
Changelog entry
ilammy Feb 26, 2020
b8f5e21
Simple benchmarks for Secure Message
ilammy Feb 27, 2020
11cf7c7
Revert "Sanitize input buffers in go 1.11 and earlier"
ilammy Feb 27, 2020
89c0246
Revert "Changelog entry"
ilammy Feb 27, 2020
5a7a89b
Update regression test
ilammy Feb 27, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
102 changes: 98 additions & 4 deletions .circleci/config.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,6 @@ jobs:
docker:
- image: cossacklabs/android-build:2019.01
environment:
GOTHEMIS_IMPORT: github.com/cossacklabs/themis/gothemis
# NIST STS tests tend to fail in Docker environment
NO_NIST_STS: 1
WITH_FATAL_WARNINGS: yes
Expand Down Expand Up @@ -208,8 +207,6 @@ jobs:
- run: make ENGINE=boringssl BUILD_PATH=build_with_boringssl prepare_tests_basic
- run: make BUILD_PATH=cover_build COVERAGE=y prepare_tests_basic
- run: make prepare_tests_all
- run: mkdir -p $HOME/go/src/$GOTHEMIS_IMPORT
- run: rsync -auv gothemis/ $HOME/go/src/$GOTHEMIS_IMPORT/
- run: lcov --directory . --zerocounters
# run only if CIRCLE_PR_NUMBER variable is not set (it's not pull request and COVERALLS_TOKEN will be set via circleCI for non-PR build) and COVERALLS_TOKEN is set
# we should calculate coverage for gothemis and send report before sending coverage of main C part
Expand All @@ -227,7 +224,6 @@ jobs:
# - run: make clean_themispp_test && CXX="clang++" CFLAGS="-std=c++17" make themispp_test && make test_cpp
- run: make test_python
- run: make test_ruby
- run: make test_go
- run: make test_rust
- run: make fuzz
- run: $HOME/valgrind/bin/valgrind build/tests/soter_test 2>&1 | grep "ERROR SUMMARY\|definitely lost\|indirectly lost\|possibly lost" | awk '{sum += $4} END {print $0; if ( sum > 0 ) { exit 1 } }'
Expand All @@ -247,6 +243,102 @@ jobs:
- ~/.cargo
- ~/.rustup

gothemis:
docker:
- image: cossacklabs/build:ubuntu-bionic
environment:
GOTHEMIS_IMPORT: github.com/cossacklabs/themis/gothemis
WITH_FATAL_WARNINGS: yes
steps:
- run:
name: Install Go versions
command: |
# These is not much difference between fetching these binaries from
# CircleCI cache and downloading them from scratch: both take around
# 30 seconds. But CircleCI caches are immutable and tricky to update.
#
# You can find latest versions and their hashes here:
# https://golang.org/dl/
# We support latest stable releases (as declared by Go team)
# as well as some historical versions on best-effort basis
go_versions=(
"1.14 1.14 https://dl.google.com/go/go1.14.linux-amd64.tar.gz 08df79b46b0adf498ea9f320a0f23d6ec59e9003660b4c9c1ce8e5e2c6f823ca"
"1.13 1.13.8 https://dl.google.com/go/go1.13.8.linux-amd64.tar.gz 0567734d558aef19112f2b2873caa0c600f1b4a5827930eb5a7f35235219e9d8"
"1.12 1.12.17 https://dl.google.com/go/go1.12.17.linux-amd64.tar.gz a53dd476129d496047487bfd53d021dd17e0c96895865a0e7d0469ce3db8c8d2"
"1.11 1.11.13 https://dl.google.com/go/go1.11.13.linux-amd64.tar.gz 50fe8e13592f8cf22304b9c4adfc11849a2c3d281b1d7e09c924ae24874c6daa"
)
mkdir -p "$HOME/go-install"
for v in "${go_versions[@]}"
do
set -- $v
install_name="$HOME/go-install/go-$1"
tarball_name="$HOME/go-install/go-$2.tar.gz"
tarball_url="$3"
expect_hash="$4"
# Download the tarball only if we do not have it already installed
if [[ ! -e "$install_name" ]]
then
wget -O "$tarball_name" "$tarball_url"
actual_hash=$(sha256sum "$tarball_name" | awk '{print $1}')
if [[ "$actual_hash" != "$expect_hash" ]]
then
echo 1>&2 "Go distribution checksum mismatch!"
echo 1>&2 " actual: $actual_hash"
echo 1>&2 " expect: $expect_hash"
exit 1
fi
# Go tarball already contains a "go" directory, but we want
# it to be named differently to allow parallel installs
echo "Unpacking $tarball_name into $install_name..."
tar -C "$HOME/go-install" -xzf "$tarball_name"
mv "$HOME/go-install/go" "$install_name"
fi
done
- checkout
- run:
name: Init submodules
command: |
git reset --hard HEAD
git submodule sync
git submodule update --init
- run:
name: Install Themis Core
command: |
make
sudo make install
- run:
name: Prepare tests
command: make prepare_tests_all
- run:
name: Install GoThemis
command: |
mkdir -p $HOME/go/src/$GOTHEMIS_IMPORT
cp -ar gothemis/* $HOME/go/src/$GOTHEMIS_IMPORT
- run:
name: GoThemis unit tests (Go 1.14)
command: |
export PATH="$HOME/go-install/go-1.14/bin:$PATH"
go version
make test_go
- run:
name: GoThemis unit tests (Go 1.13)
command: |
export PATH="$HOME/go-install/go-1.13/bin:$PATH"
go version
make test_go
- run:
name: GoThemis unit tests (Go 1.12)
command: |
export PATH="$HOME/go-install/go-1.12/bin:$PATH"
go version
make test_go
- run:
name: GoThemis unit tests (Go 1.11)
command: |
export PATH="$HOME/go-install/go-1.11/bin:$PATH"
go version
make test_go

jsthemis:
docker:
- image: cossacklabs/android-build:2019.01
Expand Down Expand Up @@ -529,6 +621,7 @@ workflows:
- benchmark
- android
- x86_64
- gothemis
- jsthemis
- php5
- php70
Expand All @@ -550,6 +643,7 @@ workflows:
- benchmark
- android
- x86_64
- gothemis
- jsthemis
- php5
- php70
Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ _Code:_
- **Go**

- New function `keys.NewSymmetricKey()` can be used to generate symmetric keys for Secure Cell ([#561](https://github.com/cossacklabs/themis/pull/561)).
- Worked around a bug in CGo 1.11 and earlier causing panics when `bytes.Buffer` is passed to GoThemis methods ([#595](https://github.com/cossacklabs/themis/pull/595)).

- **iOS and macOS**

Expand Down
13 changes: 11 additions & 2 deletions gothemis/cell/cell.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,8 +126,10 @@ static bool decrypt(const void *key, size_t key_len, const void *prot, size_t pr
*/
import "C"
import (
"github.com/cossacklabs/themis/gothemis/errors"
"unsafe"

"github.com/cossacklabs/themis/gothemis/errors"
"github.com/cossacklabs/themis/gothemis/utils"
)

// Secure Cell operation mode.
Expand Down Expand Up @@ -155,7 +157,7 @@ type SecureCell struct {

// New makes a new Secure Cell with master key and specified mode.
func New(key []byte, mode int) *SecureCell {
return &SecureCell{key, mode}
return &SecureCell{utils.SanitizeBuffer(key), mode}
}

func missing(data []byte) bool {
Expand All @@ -182,6 +184,9 @@ func (sc *SecureCell) Protect(data []byte, context []byte) ([]byte, []byte, erro
}
}

data = utils.SanitizeBuffer(data)
context = utils.SanitizeBuffer(context)

var ctx unsafe.Pointer
var ctxLen C.size_t

Expand Down Expand Up @@ -256,6 +261,10 @@ func (sc *SecureCell) Unprotect(protectedData []byte, additionalData []byte, con
}
}

protectedData = utils.SanitizeBuffer(protectedData)
additionalData = utils.SanitizeBuffer(additionalData)
context = utils.SanitizeBuffer(context)

var add, ctx unsafe.Pointer = nil, nil
var addLen, ctxLen C.size_t = 0, 0

Expand Down
23 changes: 23 additions & 0 deletions gothemis/cell/cell_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import (
"crypto/rand"
"math/big"
"testing"

"github.com/cossacklabs/themis/gothemis/keys"
)

func testProtect(mode int, context []byte, t *testing.T) {
Expand Down Expand Up @@ -96,3 +98,24 @@ func TestProtect(t *testing.T) {

testProtect(ModeContextImprint, context, t)
}

// Regression test for cgo false positive, resolved in go 1.12:
// https://github.com/golang/go/issues/14210
func TestBufferGo111(t *testing.T) {
key, err := keys.NewSymmetricKey()
if err != nil {
t.Fatalf("cannot generate master key: %v", err)
}
sc := New(key.Value, ModeSeal)

data := []byte("some data to encrypt")

b := new(bytes.Buffer)
b.WriteString("context in bytes.Buffer")
context := b.Bytes()

_, _, err = sc.Protect(data, context)
if err != nil {
t.Errorf("Protect() failed: %v", err)
}
}
6 changes: 5 additions & 1 deletion gothemis/compare/compare.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,11 @@ static int compare_result(void *ctx)
*/
import "C"
import (
"github.com/cossacklabs/themis/gothemis/errors"
"runtime"
"unsafe"

"github.com/cossacklabs/themis/gothemis/errors"
"github.com/cossacklabs/themis/gothemis/utils"
)

// Secure comparison result.
Expand Down Expand Up @@ -115,6 +117,7 @@ func (sc *SecureCompare) Append(secret []byte) error {
if nil == secret || 0 == len(secret) {
return errors.New("Secret was not provided")
}
secret = utils.SanitizeBuffer(secret)
if !bool(C.compare_append(sc.ctx, unsafe.Pointer(&secret[0]), C.size_t(len(secret)))) {
return errors.New("Failed to append secret")
}
Expand Down Expand Up @@ -147,6 +150,7 @@ func (sc *SecureCompare) Proceed(data []byte) ([]byte, error) {
if nil == data || 0 == len(data) {
return nil, errors.New("Data was not provided")
}
data = utils.SanitizeBuffer(data)

if !bool(C.compare_proceed_size(sc.ctx, unsafe.Pointer(&data[0]), C.size_t(len(data)), &outLen)) {
return nil, errors.New("Failed to get output size")
Expand Down
24 changes: 17 additions & 7 deletions gothemis/message/message.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,11 @@ static bool process(const void *priv, size_t priv_len, const void *public, size_
*/
import "C"
import (
"unsafe"

"github.com/cossacklabs/themis/gothemis/errors"
"github.com/cossacklabs/themis/gothemis/keys"
"unsafe"
"github.com/cossacklabs/themis/gothemis/utils"
)

const (
Expand All @@ -92,22 +94,30 @@ func messageProcess(private *keys.PrivateKey, peerPublic *keys.PublicKey, messag
if nil == message || 0 == len(message) {
return nil, errors.New("No message was provided")
}
message = utils.SanitizeBuffer(message)

var privateValue, peerPublicValue []byte
var priv, pub unsafe.Pointer
var privLen, pubLen C.size_t
priv = nil
pub = nil
privLen = 0
pubLen = 0

if nil != private && 0 < len(private.Value) {
priv = unsafe.Pointer(&private.Value[0])
privLen = C.size_t(len(private.Value))
if private != nil {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

interesting to make the benchmark, how much slower it works with copying data and without.
maybe we can skip copying values of private/public keys because in most cases it generated by our code keys.New where we don't use bytes.Buffer or something like that and we don't accept any buffer which can be used for new generated keys. So I can't imagine a case where public/private.Value will be something else than raw byte slice.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

interesting to make the benchmark, how much slower it works with copying data and without

Well, memcpy() is generally very cheap compared to more advanced processing so I don't think it will have any measurable impact. But sure, I'll add some simple benchmarks and report the findings.

maybe we can skip copying values of private/public keys because in most cases it generated by our code keys.New where we don't use bytes.Buffer or something like that

It is true that keys.New() will produce well-formed []byte slices, but we are not dealing with newly generated keys all the time. For example, we will need to deal with slices of unknown origin when the keys are received from external storage, as typically this goes by something like this:

keyBytes, _ := ioutil.ReadAll("my_key.pub")
key := &keys.PublicKey{Value: keyBytes}
smessage := message.New(nil, key)

smessage.Verify(data)

It's unlikely that any system utility will use bytes.Buffer because Buffer() returns a slice of some internal buffer, so it's not suitable as a general API. However, I can easily imagine some custom-made protocol parser which uses it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here are some numbers.

$ go version
go version go1.11.13 darwin/amd64

Three successive runs of go test -bench.

With workaround 
goos: darwin
goarch: amd64
pkg: github.com/cossacklabs/themis/gothemis/message
BenchmarkSMessageEncryptRSA-4   	   20000	     77981 ns/op
BenchmarkSMessageEncryptEC-4    	    3000	    457969 ns/op
BenchmarkSMessageDecryptRSA-4   	    1000	   1754761 ns/op
BenchmarkSMessageDecryptEC-4    	    3000	    459720 ns/op
BenchmarkSMessageSignRSA-4      	    1000	   1665766 ns/op
BenchmarkSMessageSignEC-4       	   10000	    190236 ns/op
BenchmarkSMessageVerifyRSA-4    	   20000	     67589 ns/op
BenchmarkSMessageVerifyEC-4     	    5000	    258329 ns/op
PASS
ok  	github.com/cossacklabs/themis/gothemis/message	19.787s

goos: darwin
goarch: amd64
pkg: github.com/cossacklabs/themis/gothemis/message
BenchmarkSMessageEncryptRSA-4   	   20000	     86262 ns/op
BenchmarkSMessageEncryptEC-4    	    3000	    478931 ns/op
BenchmarkSMessageDecryptRSA-4   	    1000	   1946130 ns/op
BenchmarkSMessageDecryptEC-4    	    3000	    442622 ns/op
BenchmarkSMessageSignRSA-4      	    1000	   1807819 ns/op
BenchmarkSMessageSignEC-4       	   10000	    191501 ns/op
BenchmarkSMessageVerifyRSA-4    	   20000	     65709 ns/op
BenchmarkSMessageVerifyEC-4     	   10000	    245958 ns/op
PASS
ok  	github.com/cossacklabs/themis/gothemis/message	19.979s

goos: darwin
goarch: amd64
pkg: github.com/cossacklabs/themis/gothemis/message
BenchmarkSMessageEncryptRSA-4   	   20000	     79789 ns/op
BenchmarkSMessageEncryptEC-4    	    3000	    444484 ns/op
BenchmarkSMessageDecryptRSA-4   	    1000	   1611965 ns/op
BenchmarkSMessageDecryptEC-4    	    3000	    428861 ns/op
BenchmarkSMessageSignRSA-4      	    1000	   1672447 ns/op
BenchmarkSMessageSignEC-4       	   10000	    185019 ns/op
BenchmarkSMessageVerifyRSA-4    	   20000	     64634 ns/op
BenchmarkSMessageVerifyEC-4     	   10000	    242627 ns/op
PASS
ok  	github.com/cossacklabs/themis/gothemis/message	18.179s
Without workaround 
goos: darwin
goarch: amd64
pkg: github.com/cossacklabs/themis/gothemis/message
BenchmarkSMessageEncryptRSA-4   	   20000	     78610 ns/op
BenchmarkSMessageEncryptEC-4    	    3000	    433074 ns/op
BenchmarkSMessageDecryptRSA-4   	    1000	   1625414 ns/op
BenchmarkSMessageDecryptEC-4    	    3000	    438958 ns/op
BenchmarkSMessageSignRSA-4      	    1000	   1619214 ns/op
BenchmarkSMessageSignEC-4       	   10000	    180392 ns/op
BenchmarkSMessageVerifyRSA-4    	   20000	     65154 ns/op
BenchmarkSMessageVerifyEC-4     	    5000	    249397 ns/op
PASS
ok  	github.com/cossacklabs/themis/gothemis/message	17.320s

goos: darwin
goarch: amd64
pkg: github.com/cossacklabs/themis/gothemis/message
BenchmarkSMessageEncryptRSA-4   	   20000	     78407 ns/op
BenchmarkSMessageEncryptEC-4    	    3000	    431794 ns/op
BenchmarkSMessageDecryptRSA-4   	    1000	   1621143 ns/op
BenchmarkSMessageDecryptEC-4    	    3000	    427959 ns/op
BenchmarkSMessageSignRSA-4      	    1000	   1606592 ns/op
BenchmarkSMessageSignEC-4       	   10000	    186542 ns/op
BenchmarkSMessageVerifyRSA-4    	   20000	     66023 ns/op
BenchmarkSMessageVerifyEC-4     	    5000	    252454 ns/op
PASS
ok  	github.com/cossacklabs/themis/gothemis/message	18.223s

goos: darwin
goarch: amd64
pkg: github.com/cossacklabs/themis/gothemis/message
BenchmarkSMessageEncryptRSA-4   	   20000	     83979 ns/op
BenchmarkSMessageEncryptEC-4    	    3000	    576246 ns/op
BenchmarkSMessageDecryptRSA-4   	    1000	   1728592 ns/op
BenchmarkSMessageDecryptEC-4    	    3000	    462749 ns/op
BenchmarkSMessageSignRSA-4      	    1000	   1774446 ns/op
BenchmarkSMessageSignEC-4       	   10000	    197490 ns/op
BenchmarkSMessageVerifyRSA-4    	   20000	     73172 ns/op
BenchmarkSMessageVerifyEC-4     	    5000	    245907 ns/op
PASS
ok  	github.com/cossacklabs/themis/gothemis/message	18.621s

Measurements have pretty high variance but I don't see any significant difference in them. Benchmarks don't give us raw data and I don't want to do tons of runs to do proper statistics.

privateValue = utils.SanitizeBuffer(private.Value)
}
if len(privateValue) != 0 {
priv = unsafe.Pointer(&privateValue[0])
privLen = C.size_t(len(privateValue))
}

if nil != peerPublic && 0 < len(peerPublic.Value) {
pub = unsafe.Pointer(&peerPublic.Value[0])
pubLen = C.size_t(len(peerPublic.Value))
if peerPublic != nil {
peerPublicValue = utils.SanitizeBuffer(peerPublic.Value)
}
if len(peerPublicValue) != 0 {
pub = unsafe.Pointer(&peerPublicValue[0])
pubLen = C.size_t(len(peerPublicValue))
}

var outputLength C.size_t
Expand Down
Loading