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

MacOS build failure with OpenSSL 3.0 #2967

Closed
Jakuje opened this issue Dec 20, 2023 · 6 comments · Fixed by #2993
Closed

MacOS build failure with OpenSSL 3.0 #2967

Jakuje opened this issue Dec 20, 2023 · 6 comments · Fixed by #2993

Comments

@Jakuje
Copy link
Member

Jakuje commented Dec 20, 2023

Problem Description

After merging #2930 changing the macos build to use openssl 3.0, we are getting a new errors:

(1 failure)
/Users/runner/work/OpenSC/OpenSC/OpenSCToken/OpenSC/src/pkcs11/openssl.c:529:9: error: implicit conversion loses integer precision: 'size_t' (aka 'unsigned long') to 'int' [-Werror,-Wshorten-64-to-32]
                                r = EC_POINT_point2oct(group, P, POINT_CONVERSION_COMPRESSED, buf, buf_len, NULL);
                                  ~ ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 error generated

Trying to run the build locally on linux with this CFLAG results in dozens of more issues after fixing this one (I will try to fix them but it will be probably more work than just simple hotfix so it will likely take some time):

CC=clang CFLAGS=-Wshorten-64-to-32 ./configure && make clean && make -j8

Proposed Resolution

  • Fix the build issues
  • Consider building with clang and with these new fancy CFLAGS to catch the issues earlier than in master osx build

Steps to reproduce

See the latest macos builds in master failing

@Jakuje
Copy link
Member Author

Jakuje commented Jan 10, 2024

@metsma did you manage to have a look into this? We can probably ignore the errors or try to fix them (but there is dozens of them).

I am always surprised by the macos ci that works in PR but fails on master. Is there something we can make it failing the same way for both of them?

Or is it unrelated to the OpenSSL version change? By comparing the logs, I see the macos was updated from 12.6.9 to 12.7.2, but it still works in the PRs.

@metsma
Copy link
Contributor

metsma commented Jan 10, 2024

I will try find time for this.

frankmorgner added a commit to frankmorgner/OpenSC that referenced this issue Jan 12, 2024
@metsma
Copy link
Contributor

metsma commented Jan 15, 2024

I did look into, but this requires more in depth knowledge of all OpenSC internals.
Etc. all RSA bit/byte sizes are converted between size_t (unsigned long) until unsigned int and int (OpenSSL BN_bin2bn).
Not sure how it will affect if we change all them to size_t or int?
About usage and algorithm parameters we should probably use more fixed size. Currently I can see there is int, unsigned int, unsigned long. (Maybe u16? or even u32?)

@Jakuje
Copy link
Member Author

Jakuje commented Jan 15, 2024

Thank you for having a look into that! I agree that some syncing on a specific type would be beneficial. I think most of the internal types is already using size_t so I would propose to use size_t where possible (where it is really only size and not a error code or something).

Algorithms or fixed enums should really used that enum number if possible. In other cases, the unsigned long usage mostly stems from the PKCS#11 level specification where this type is used for most of the stuff so my proposal would be to try to stick to this.

I started changing some of the stuff, but there is still a lot to go.

I thought if you have some insight of what changed. If it is because of the new OpenSSL, clang, mac or something completely independent ...

@metsma
Copy link
Contributor

metsma commented Jan 15, 2024

I think this is related to master builds signed packages and then OpenSCTokenApp is pulled to build.
OpenSC package use xcode project and thus different compiler flags.

Jakuje added a commit to Jakuje/OpenSC that referenced this issue Jan 19, 2024
These are exposed by a shorten-64-to-32 warning from clang that is turned
out automatically when building release build for OSX.

They can be also triggered manually with similar configuration like this:

    CC=clang CFLAGS=-Wshorten-64-to-32 ./configure && make -j8

Should fix OpenSC#2967
Jakuje added a commit to Jakuje/OpenSC that referenced this issue Jan 19, 2024
These are exposed by a shorten-64-to-32 warning from clang that is turned
out automatically when building release build for OSX.

They can be also triggered manually with similar configuration like this:

    CC=clang CFLAGS=-Wshorten-64-to-32 ./configure && make -j8

Should fix OpenSC#2967
@Jakuje Jakuje mentioned this issue Jan 19, 2024
3 tasks
Jakuje added a commit to Jakuje/OpenSC that referenced this issue Jan 22, 2024
These are exposed by a shorten-64-to-32 warning from clang that is turned
out automatically when building release build for OSX.

They can be also triggered manually with similar configuration like this:

    CC=clang CFLAGS=-Wshorten-64-to-32 ./configure && make -j8

Should fix OpenSC#2967
Jakuje added a commit to Jakuje/OpenSC that referenced this issue Jan 22, 2024
These are exposed by a shorten-64-to-32 warning from clang that is turned
out automatically when building release build for OSX.

They can be also triggered manually with similar configuration like this:

    CC=clang CFLAGS=-Wshorten-64-to-32 ./configure && make -j8

Should fix OpenSC#2967
Jakuje added a commit to Jakuje/OpenSC that referenced this issue Jan 22, 2024
These are exposed by a shorten-64-to-32 warning from clang that is turned
out automatically when building release build for OSX.

They can be also triggered manually with similar configuration like this:

    CC=clang CFLAGS=-Wshorten-64-to-32 ./configure && make -j8

Should fix OpenSC#2967
Jakuje added a commit to Jakuje/OpenSC that referenced this issue Jan 22, 2024
These are exposed by a shorten-64-to-32 warning from clang that is turned
out automatically when building release build for OSX.

They can be also triggered manually with similar configuration like this:

    CC=clang CFLAGS=-Wshorten-64-to-32 ./configure && make -j8

Should fix OpenSC#2967
Jakuje added a commit to Jakuje/OpenSC that referenced this issue Jan 22, 2024
These are exposed by a shorten-64-to-32 warning from clang that is turned
out automatically when building release build for OSX.

They can be also triggered manually with similar configuration like this:

    CC=clang CFLAGS=-Wshorten-64-to-32 ./configure && make -j8

Should fix OpenSC#2967
Jakuje added a commit to Jakuje/OpenSC that referenced this issue Jan 22, 2024
These are exposed by a shorten-64-to-32 warning from clang that is turned
out automatically when building release build for OSX.

They can be also triggered manually with similar configuration like this:

    CC=clang CFLAGS=-Wshorten-64-to-32 ./configure && make -j8

Should fix OpenSC#2967
@Jakuje
Copy link
Member Author

Jakuje commented Jan 22, 2024

I think the #2993 will fix the master build, but I do not know how to check this in the PR (I tested with the flags locally and in additional specially crafted linux run with clang and these specific flags). I know, its huge, but inputs/reviews are welcomed.

Jakuje added a commit to Jakuje/OpenSC that referenced this issue Jan 25, 2024
These are exposed by a shorten-64-to-32 warning from clang that is turned
out automatically when building release build for OSX.

They can be also triggered manually with similar configuration like this:

    CC=clang CFLAGS=-Wshorten-64-to-32 ./configure && make -j8

Should fix OpenSC#2967
Jakuje added a commit to Jakuje/OpenSC that referenced this issue Jan 26, 2024
These are exposed by a shorten-64-to-32 warning from clang that is turned
out automatically when building release build for OSX.

They can be also triggered manually with similar configuration like this:

    CC=clang CFLAGS=-Wshorten-64-to-32 ./configure && make -j8

Should fix OpenSC#2967
Jakuje added a commit to Jakuje/OpenSC that referenced this issue Jan 29, 2024
These are exposed by a shorten-64-to-32 warning from clang that is turned
out automatically when building release build for OSX.

They can be also triggered manually with similar configuration like this:

    CC=clang CFLAGS=-Wshorten-64-to-32 ./configure && make -j8

Should fix OpenSC#2967
frankmorgner pushed a commit that referenced this issue Jan 30, 2024
These are exposed by a shorten-64-to-32 warning from clang that is turned
out automatically when building release build for OSX.

They can be also triggered manually with similar configuration like this:

    CC=clang CFLAGS=-Wshorten-64-to-32 ./configure && make -j8

Should fix #2967
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 a pull request may close this issue.

2 participants