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

Allow to disable reference of AES-256-XTS for Themis iOS #329

Merged
merged 18 commits into from
Oct 25, 2018

Conversation

vixentael
Copy link
Contributor

Currently building Themis iOS with BoringSSL as backend leads to error:

Undefined symbols for architecture x86_64:
  "_EVP_aes_256_xts", referenced from:
      _algid_to_evp in libthemis.a(soter_sym.o)
ld: symbol(s) not found for architecture x86_64

AES-256-XTS implementation is missing in BoringSSL CocoaPod. BoringSSL CocoaPod is rather popular and used in thousands apps, and probably they had a reason to exclude XTS, so I won't suggest them to change their podspec.

However, Themis is not really using XTS mode (see search results), but soter_sym has two references on it (see soter/openssl/soter_sym and soter/boringssl/soter_sym).

I suggest to have a way to exclude references to EVP_aes_256_xts for Themis iOS using #define SOTER_BORINGSSL_DISABLE_XTS.

This change won't affect other platforms, but will be used only in themis.podspec for iOS.

@vixentael vixentael added the core Themis Core written in C, its packages label Oct 25, 2018
Copy link
Contributor

@ignatk ignatk left a comment

Choose a reason for hiding this comment

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

Should we also add logic in the makefile, which will add SOTER_BORINGSSL_DISABLE_XTS if building Themis on iOS with BoringSSL?

@@ -65,8 +65,13 @@ const EVP_CIPHER* algid_to_evp(uint32_t alg){
return EVP_aes_192_ctr();
case SOTER_SYM_AES_CTR|SOTER_SYM_128_KEY_LENGTH:
return EVP_aes_128_ctr();
// Workaround for using BoringSSL on iOS, because XTS is not included in BoringSSL pod implementation.
// see https://github.com/CocoaPods/Specs/blob/master/Specs/0/8/a/BoringSSL/10.0.6/BoringSSL.podspec.json#L44
// see https://github.com/cossacklabs/themis/issues/223#issuecomment-432720576
Copy link
Contributor

Choose a reason for hiding this comment

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

Please, use consistent comments as the rest of the file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

what kind of comment style would you prefer? because i don't see any consistency across comments in all soter files.

Copy link
Contributor

Choose a reason for hiding this comment

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

Well, at least in this file we have /* ... */ at the top. I generally prefer C-style comments in C files.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure, changed.

when i was reading other files, I noticed rather mixed style of comments, but my pleasure is to make it more consistent :)

@vixentael
Copy link
Contributor Author

Should we also add logic in the makefile, which will add SOTER_BORINGSSL_DISABLE_XTS if building Themis on iOS with BoringSSL?

Themis for iOS is not built via Makefile, so no need to change it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Themis Core written in C, its packages
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants