Skip to content

Conversation

@barracuda156
Copy link

@barracuda156 barracuda156 commented Jun 2, 2025

SecRandom is the same issue as in utelle/wxpdfdoc#94
Pointer size for ppc64 was fixed upstream, but still incorrect here: __POWERPC__ includes ppc64, we do not want that.
<endian.h> neither exists nor is needed on macOS.
I don’t know why the code uses __BYTE_ORDER == __LITTLE_ENDIAN, which may not be defined at all. SQLITE_BYTEORDER is already defined above, so just use that.

@utelle If endian swap macros are needed, macros from `<libkern/OSByteOrder.h> can be used. Should we use those instead of hand-written code?

@utelle
Copy link
Owner

utelle commented Jun 2, 2025

SecRandom is the same issue as in utelle/wxpdfdoc#94

Thanks for reporting the issue. You are right that it is the same issue, and it should be fixed, of course. However, the situation in wxSQLite3 is a bit different than in wxPdfDocument. While _ wxPdfDocument_ doesn't have a dependency on another project for the crypto code, wxSQLite3 uses the source amalgamation of SQLite3MC. Therefore any issues within sqlite3mc_amalgamation.c should be fixed upstream in SQLite3MC, not here.

Before taking further action let's look at your individual commits:

  1. Do not include endian.h on macOS 02039aa
    The affected upstream file is fastpbkdf2.c. Maybe including <endian.h> can be avoided completely. I have to check that in more detail.

  2. Add POWERPC to Big-endian macros 02166d9
    This affects the file sqliteInt.h of SQLite (which is used by SQLite3MC via the SQLite amalgamation). If you feel the byte order logic is wrong, please report it on the SQLite forum.

  3. Fix SQLITE_PTRSIZE for ppc64 62e45c2
    This affects the file tclsqlite.c, which is an unmodified copy of that file from upstream SQLite. The code will only be used, if you activate the TCL extension. Nevertheless, in this context here (SQLite amalgamation) the preprocessor instructions will never be activated. However, if you feel that it should be corrected in SQLite itself, please report it on the SQLite forum.

  4. Minor fix to endian macro 922052f
    I'd like to avoid a dependancy on SQLite here. The affected upstream file is fastpbkdf2.c. AFAICT the macros __BYTE_ORDER and __LITTLE_ENDIAN are defined by GCC (maybe included via <endian.h>). We may change this to using the macro __BYTE_ORDER__ (which seems to be defined by GCC independent of any header file - see GCC common predefined macros.

  5. Use SecRandom only on 10.7+ aeacb68
    Should be applied in chacha20poly1305.c (similarly as in wxPdfDocument).

Pointer size for ppc64 was fixed upstream, but still incorrect here: __POWERPC__ includes ppc64, we do not want that.

I think this affects SQLite itself and should be reported on the SQLite forum.

<endian.h> neither exists nor is needed on macOS. I don’t know why the code uses __BYTE_ORDER == __LITTLE_ENDIAN, which may not be defined at all. SQLITE_BYTEORDER is already defined above, so just use that.

As said above I will check whether the code can be adjusted easily to avoid including that header file.

If endian swap macros are needed, macros from `<libkern/OSByteOrder.h> can be used. Should we use those instead of hand-written code?

fastpbkdf2.c is a third party source file, to which I applied only minor modifications. Byte swapping code is used elsewhere in the project, too. So, I will check whether we can use a different approach here.

@barracuda156
Copy link
Author

barracuda156 commented Jun 2, 2025

Pointer size fix has already been merged to sqlite upstream. See: https://sqlite.org/forum/forumpost/fc4bfa307db74d04

Usage of __BYTE_ORDER__ will work for modern compilers (it won’t for Xcode gcc, for example, but that does not matter, I guess, since the build with Xcode gcc fails for different reasons anyway). So yes, that will do.

@barracuda156
Copy link
Author

@utelle So should I close this here? If all changes should be done in https://github.com/utelle/SQLite3MultipleCiphers instead?

P. S. Endianness macros can be handled in a way you prefer, but something should be done, since currently it just fails on non-existing endian.h.

@utelle
Copy link
Owner

utelle commented Jun 2, 2025

Pointer size fix has already been merged to sqlite upstream. See: https://sqlite.org/forum/forumpost/fc4bfa307db74d04

Good. However, file tclsqlite.c was not adjusted in that course. For completeness that should probably also be done in SQLite.

Usage of __BYTE_ORDER__ will work for modern compilers (it won’t for Xcode gcc, for example, but that does not matter, I guess, since the build with Xcode gcc fails for different reasons anyway). So yes, that will do.

The macro __BYTE_ORDER__ is used in other third-party code in SQLite3MC, too. However, I'm open to any practical solution.

So should I close this here? If all changes should be done in https://github.com/utelle/SQLite3MultipleCiphers instead?

Right. I will not apply fixes to sqlite3mc_amalgamation.c here. It has to be fixed in SQLite3 Multiple Ciphers - or SQLite. So, you may close the PR. I didn't do it myself yet, because of the ongoing discussion.

P. S. Endianness macros can be handled in a way you prefer, but something should be done, since currently it just fails on non-existing endian.h.

I fully agree, and I will address this issue.

@barracuda156
Copy link
Author

Thank you. Then I close this, and will open a PR to the right repo with select fixes from here. I leave endianness part to you then; as long as portable gcc macros are used (__BYTE_ORDER__, __ORDER_*_ENDIAN__), we are good. No point to bother with gcc-4.2 compatibility, recent wx itself requires a modern compiler.

@utelle
Copy link
Owner

utelle commented Jun 2, 2025

Then I close this, and will open a PR to the right repo with select fixes from here. I leave endianness part to you then; as long as portable gcc macros are used (__BYTE_ORDER__, __ORDER_*_ENDIAN__), we are good. No point to bother with gcc-4.2 compatibility, recent wx itself requires a modern compiler.

I applied 2 commits to SQLite3 Multiple Ciphers addressing the use of the header SecRandom.h and the use of the header endian.h and the byte order macros:

  1. SecRandom.h: utelle/SQLite3MultipleCiphers@d4074d5
  2. endian.h: utelle/SQLite3MultipleCiphers@a69902c
    As you can see I used the byte order macros defined by SQLite.

Maybe you could apply these modifications in your environment to check whether it now works for you.

@barracuda156
Copy link
Author

@utelle Thank you, I will try that.

@utelle
Copy link
Owner

utelle commented Jun 5, 2025

Thank you, I will try that.

Good. Please report back whether it works.

Most likely there will be a SQLite 3.50.1 release within relatively short time. Thereafter I will make new releases for SQLite3MC and wxSQLite3.

@utelle
Copy link
Owner

utelle commented Jun 6, 2025

Most likely there will be a SQLite 3.50.1 release within relatively short time.

SQLite 3.50.1 was released today.

Thereafter I will make new releases for SQLite3MC and wxSQLite3.

I intend to prepare the new releases over the weekend.

AFAICT the only missing piece is the detection of big endian byte order for POWERPC. However, that is something that needs to be adjusted in SQLite itself. However, the code will work even without this adjustment, only maybe slightly slower.

@barracuda156
Copy link
Author

AFAICT the only missing piece is the detection of big endian byte order for POWERPC.

That was more a matter of consistency rather than necessity. Compiler macros should handle endianness before arch-specific macros are reached. Well, I hope so :)

@barracuda156
Copy link
Author

@utelle Hi, all good with 4.10.8, at least it has built with no patches from my side. Thank you!

@barracuda156 barracuda156 deleted the apple branch June 8, 2025 01:36
@utelle
Copy link
Owner

utelle commented Jun 8, 2025

all good with 4.10.8, at least it has built with no patches from my side.

Good to hear.

Unfortunately, the latest changes caused problems for SQLite3MC to compile under Apple's tvOS. However, this does not affect wxSQLite3. In the meantime I already applied modifications to resolve the problem.

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 this pull request may close these issues.

2 participants