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 and deprecate Themis 0.9.6 compatibility path #614

Merged
merged 15 commits into from
Apr 1, 2020
Merged

Test and deprecate Themis 0.9.6 compatibility path #614

Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
7b500aa
Test for Themis 0.9.6 compatibility
ilammy Dec 8, 2019
459b9e7
Cleanup Context Imprint compatibility code path
ilammy Mar 26, 2020
5adcf33
Cleanup Context Imprint IV computation
ilammy Mar 26, 2020
b31cf33
Drop unused typedef themis_sym_message_hdr_t
ilammy Mar 26, 2020
8a1781f
Remove SCELL_COMPAT define and ignore NO_SCELL_COMPAT variable
ilammy Mar 26, 2020
dfa33a5
Use compatibility path on 32-bit machines too
ilammy Mar 26, 2020
545e782
Revert "Remove SCELL_COMPAT define and ignore NO_SCELL_COMPAT variable"
ilammy Mar 27, 2020
a6de319
Disable SCELL_COMPAT by default
ilammy Mar 27, 2020
9676fda
Describe migration strategy in changelog
ilammy Mar 27, 2020
39a4a1a
Test build WITH_SCELL_COMPAT on CircleCI
ilammy Mar 27, 2020
0846314
Test build WITH_SCELL_COMPAT on GitHub Actions
ilammy Mar 27, 2020
ab34fa2
Move "error" label inside SCELL_COMPAT ifdef
ilammy Mar 28, 2020
5631124
Merge branch 'master' into always-compat
ilammy Mar 28, 2020
78ffd13
Merge branch 'master' into always-compat
ilammy Mar 30, 2020
7932990
Reword changelog for clarity
ilammy Mar 30, 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
3 changes: 3 additions & 0 deletions .circleci/config.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -285,20 +285,23 @@ jobs:
- checkout
- run: git reset HEAD && git submodule sync && git submodule update --init
- run: make
- run: make BUILD_PATH=build_compat WITH_SCELL_COMPAT=yes
- run: make JAVA_HOME=/usr/lib/jvm/default-java themis_jni
- run: sudo make install
- run: sudo make themispp_install
- run: sudo make pythemis_install
- run: sudo make rbthemis_install
- 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_basic BUILD_PATH=build_compat WITH_SCELL_COMPAT=yes
- run: make prepare_tests_all
- 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
- run: '[ -z "$CIRCLE_PR_NUMBER" ] && ! [ -z "$COVERALLS_TOKEN" ] && cd $HOME/go/src/$GOTHEMIS_IMPORT && $HOME/go/bin/goveralls -v -service=circle-ci -repotoken=$COVERALLS_TOKEN || true'
- run: sudo /sbin/ldconfig
- run: make test
- run: make test BUILD_PATH=build_compat
- run: make clean_themispp_test && CXX="g++" CFLAGS="-std=c++03" make themispp_test && make test_cpp
- run: make clean_themispp_test && CXX="g++" CFLAGS="-std=c++11" make themispp_test && make test_cpp
- run: make clean_themispp_test && CXX="g++" CFLAGS="-std=c++14" make themispp_test && make test_cpp
Expand Down
6 changes: 6 additions & 0 deletions .github/workflows/test-core.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,12 +58,18 @@ jobs:
- name: Build Themis Core (BoringSSL)
if: always()
run: make prepare_tests_basic ENGINE=boringssl BUILD_PATH=build-boringssl
- name: Build Themis Core (WITH_SCELL_COMPAT)
if: always()
run: make prepare_tests_basic WITH_SCELL_COMPAT=yes BUILD_PATH=build-compat
- name: Run test suite (OpenSSL)
if: always()
run: make test BUILD_PATH=build-openssl
- name: Run test suite (BoringSSL)
if: always()
run: make test BUILD_PATH=build-boringssl
- name: Run test suite (WITH_SCELL_COMPAT)
if: always()
run: make test BUILD_PATH=build-compat

examples:
name: Code examples
Expand Down
13 changes: 13 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ Changes that are currently in development and have not been released yet.

- Many languages received Secure Cell API overhaul with parts of the old API becoming deprecated. Refer to individual language sections for details.
- ObjCThemis installed via Carthage is now called `objcthemis` instead of just `themis` ([read more](#0.13.0-objcthemis-rename)).
- Themis 0.9.6 compatibility is now disabled by default ([#614](https://github.com/cossacklabs/themis/pull/614)).

_Code:_

Expand All @@ -34,6 +35,18 @@ _Code:_

provide Seal mode API that is safe to use with passphrases ([#577](https://github.com/cossacklabs/themis/pull/577)).

- **Breaking changes**

- Secure Cell compatibility with Themis 0.9.6 is now disabled by default ([#614](https://github.com/cossacklabs/themis/pull/614)).

Old versions of Themis on 64-bit platforms have been using incorrect encryption algorithm with resulted in Secure Cells that cannot be decrypted on 32-bit machines (see [#279](https://github.com/cossacklabs/themis/pull/279) for details).
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Old versions of Themis on 64-bit platforms have been using incorrect encryption algorithm with resulted in Secure Cells that cannot be decrypted on 32-bit machines (see [#279](https://github.com/cossacklabs/themis/pull/279) for details).
Old versions of Themis platforms have been using incorrect calculation of encrypted data length, which resulted in Secure Cells encrypted on 64-machine cannot be decrypted on 32-bit machines (see [#279](https://github.com/cossacklabs/themis/pull/279) for details).

I disagree with "incorrect encryption algorithm" sentence. The issue was not about encryption algorithm, or params, it was about using wrong data type of storing length. We shouldn't make users think that encryption was bad/broken.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Technically if the algorithm says “use u32” and you're using u64, that's incorrect, but I agree with you that it can make an impression as if we'd been using DES by accident.

Copy link
Contributor

Choose a reason for hiding this comment

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

the algorithm says “use u32”

the issue was not about AES algorithm, but in determining hmac length. That's not an algorithm issue, that's implementation issue :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I meant Secure Cell algorithm, as in whatever happens inside themis_secure_cell_mode_encrypt(). Well, given that the implementation is the spec and there is no paper that describes the entire algorithm in some language-agonstic way...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Anyways, I've reworded that paragraph a bit to talk about “encrypted length computation” rather than “incorrect algorithm” and fixed some typos.


Themis 0.10 and later versions include a fix for that issue and compatiblity workaround that allows to decrypt data encrypted by Themis 0.9.6 on 64-bit platforms. This workaround was enabled by default and could be disabled by setting the `NO_SCELL_COMPAT` varible.

Since Themis 0.13 the workaround for Themis 0.9.6 compatibility is *disabled* by default (since it has performance implications). It can be enabled if needed by compling with `WITH_SCELL_COMPAT`.

We are planning to **remove** the workaround completely after Themis 0.9.6 reaches end-of-life in December 2020. Please use this time to migrate existing data if you have been using Themis 0.9.6. To migrate the data, decrypt it and encrypt it back with the latest Themis version.

- **C++**

- Secure Cell API updates ([#588](https://github.com/cossacklabs/themis/pull/588))
Expand Down
3 changes: 2 additions & 1 deletion Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -348,7 +348,8 @@ LDFLAGS += $(SANITIZERS)

# Binary format compatibility with Themis 0.9.6 on x86_64 architecture.
# https://github.com/cossacklabs/themis/pull/279
ifeq ($(NO_SCELL_COMPAT),)
# Themis 0.9.6 is going EOL on 2020-12-13 so it can be removed after that.
ifneq ($(WITH_SCELL_COMPAT),)
CFLAGS += -DSCELL_COMPAT
endif

Expand Down
Loading