passepartoutvpn / openssl-apple

A script for compiling OpenSSL for Apple Devices.
Apache License 2.0
143 stars 68 forks source link

Accurate version bookkeeping #16

Closed ilammy closed 4 years ago

ilammy commented 4 years ago

This patch set improves the accuracy of version information in produced frameworks.

Now it’s possible to set the minimum SDK version to support with the new command-line flags:

Additionally, minimum SDK information is accurately translated into the resulting binary files and framework bundles. Now it should be all consistent between binaries, Info.plist, etc.

Finally, the OpenSSL version is now written into ‘marketing version’ field of Info.plist files so that vulnerability analysis tools have an easier time figuring out the version of the OpenSSL framework.

ilammy commented 4 years ago

Also, @keeshux, I just want to thanks for making this script.

The situation with OpenSSL on Apple platforms is... well... less than ideal. Your work is the most streamlined and easy to use way to build OpenSSL, based on what I tried. It just works ❤️

I guess the only easier way is to use package managers like CocoaPods – which you do maintain! – but that’s another can of worms. (We needed Carthage support which has its... peculiarities.)

Thank you so very much for making this!

ilammy commented 4 years ago

Oh! I almost forgot. Regarding the last part, about the CFBundleShortVersionString.

It’s not confirmed right now, but it may be possible that version strings like 1.1.1g are not allowed in submissions to App Store (even for embedded framework bundles, not just apps that use them). So I guess you should not be too haste to merge this PR. This question should be cleared up the next week or something.

keeshux commented 4 years ago

Okay, no rush. I'll be waiting for your further input.

Thanks I really appreciate that, remember to also credit the author of the original repository. ;)

vixentael commented 4 years ago

version strings like 1.1.1g are not allowed in submissions to App Store

Yes, this is something we are trying to test. Currently we can confirm, that using “literal versions” is good enough to archive app, build and distribute .ipa in “development” and “ad hoc” modes. Just want to double check with AppStore/TestFlight releases.

Also, we noticed that some dependency testing services (like Whitesource software), look on openssl version inside plist file (and expect to see version with letter). So if version inside plist is incorrect, it might trigger false positive result from dependecy checker that openssl is vulnerable.

vixentael commented 4 years ago

Can confirm, AppStore accepts only numeric versions:

ERROR ITMS-90060: "This bundle is invalid. The value for key CFBundleShortVersionString '1.0.2u' in the Info.plist file at '${bundlePath}' must be a period-separated list of at most three non-negative integers. Please find more information about CFBundleShortVersionString at ...

So before building app to AppStore / TestFlight, developers should change version to numeric-only, for example, 1.0.221.

keeshux commented 4 years ago

I've pushed a fix for the version format. Feel free to optimize my version, I'm bad at bash.

ilammy commented 4 years ago

Nice trick there! I guess this should do fine. As long as the letter version never exceeds z. Since 1.0.0 OpenSSL has never done so, but there were 0.9.8za ... 0.9.8zh. Well, there will be plenty of time to adapt to that if 1.1.1 comes close.

keeshux commented 4 years ago

Nice trick there! I guess this should do fine. As long as the letter version never exceeds z. Since 1.0.0 OpenSSL has never done so, but there were 0.9.8za ... 0.9.8zh. Well, there will be plenty of time to adapt to that if 1.1.1 comes close.

Lol yeah I guess there's time to think about it, fortunately still a trivial change if they go 2-letters.

vixentael commented 4 years ago

I think current PR looks good! 👍