Switch to botan2 from botan

[tim77]

Expected behaviour

Using modern, safe, well maintained libs.

Actual behaviour

Using old, legacy stuff.

[pbek]

Thank you for your concern. Any pull requests coming up?

[tim77]

Unfortunately can't promise this for now. We trying to package it according to Fedora guidelines and push in official repos and there some issues with this old botan happens in Rawhide.

[thmo]

For now, botan has been rebuilt without OpenSSL in Fedora 31 and Rawhide.

Nonetheless, botan v1 has gone EOL 2018-12-31 and is not supported anymore: https://botan.randombit.net/handbook/support.html

[pbek]

Back then I took botan the Botan source (one big CPP file with no dependencies) from QtCreator (see https://github.com/pbek/QOwnNotes/tree/develop/src/libraries/botan), so it should compile just fine in the future.

[pbek]

Qt Creator seems to have switched to Botan 2 with 4.8 and removed it at all in 4.9.

[thmo]

Embedding such an ancient version of a crypto lib doesn't seem to be a good idea to me:
https://botan.randombit.net/security.html

[pbek]

There seems to be no "single file" botan.cpp in https://github.com/qt-creator/qt-creator/tree/4.8/src/libs/3rdparty/botan

[pbek]

Embedding such an ancient version of a crypto lib doesn't seem to be a good idea to me.

We just need it for AES encryption and decryption.
Suggestions and pull requests are welcome.

The new solution needs to work with qmake and cmake, build on all currently supported platforms and distributions and be still able to decrypt encrypted notes from Botan 1.

[tim77]

Also cant build for s390x and ppc64le arches because of bundled botan lib.

[thmo]

I would not expect a compatibility issue regarding notes encrypted with Botan v1.

[pbek]

Also cant build for s390x and ppc64le arches because of bundled botan lib.

I think I also saw the same on the OpenBuildService...

[Waqar144]

I managed to compile the project with Botan2, however, encryption/decryption isn't working (cause it's unable to create the hash).
It's probably a build problem and I will look into it later today.
On a side note, the code required for our particular use case hasn't changed much since botan 1

[pbek]

Great, thank you for taking a look at it!

[Waqar144]

Encryption/Decryption is working now, and is able to decrypt notes from Botan 1.
I will try to decrease the filesize for this library so that we only include modules we need (It's still one big .h and .cpp file + a couple of others)
Will try to send in the PR soon and hopefully @pbek can take a look at it.

[thmo]

Are you going to embed it? If so, why?

[Waqar144]

@thmo Cause it's easier that way and more simple and straightforward. Also, I think it will be more simpler this way when it comes to cross-platform building

[thmo]

@Waqar144 From a distribution perspective, this is discouraged for various reasons (e.g. in case of a security issue, the library can be updated separately).

While I see that embedding can be easier for upstream in certain cases, please foresee an option for distro packagers to use the system-wide installed library instead (and therefore, please also refrain from patching the embedded code).

[Waqar144]

@thmo Not patching the embedded code.
Also, There is an option to use the System wide installed library. Have to test that for botan 2, it's still on botan 1. Maybe you can help with this one?

[pbek]

Thank you for the effort, @Waqar144! The windows build seems to fail currently: https://ci.appveyor.com/project/pbek/qownnotes/builds/28104602/job/was94uyoyuuxppuv

[pbek]

The macOS build also fails: https://travis-ci.org/pbek/QOwnNotes/jobs/597831831

[Waqar144]

😿
Windows build is failing because Botan is configured for x64. There seems to be no way to configure it to in a way that we have a single file which is configured for both(in my understanding). Reading through Botan 1 code, I didnt find any checks for this.
One solution could be to configure Botan separately for x64 and x86 and then include the files accordingly, maybe

For macOS i don't understand much what's going on.

[pbek]

For macOS i don't understand much what's going on.

It looks like that headers are used that doesn't exist on macOS:
libraries/botan/botan.cpp:27049:12: fatal error: 'sys/auxv.h' file not found

[pbek]

Windows build is failing because Botan is configured for x64.

QON is still built for 32bit because the 64bit build process still doesn't work on AppVeyor and we also need 32bit for Windows XP support (Qt 5.7).

[Waqar144]

It looks like that headers are used that doesn't exist on macOS:
libraries/botan/botan.cpp:27049:12: fatal error: 'sys/auxv.h' file not found

Yeah, I know. I was just surprised because that part of the code shouldn't have been running even but i understand now. Diving into the code a bit more, I think this one is easy to fix. I will have to remove the #defines in botan.h and then define them in qmake according to the underlying os(which the qmake file is already doing i think).

For 32 bit support in windows.. I will configure a separate version of botan and then diff it with the 64 bit one to see the differences and maybe it will not be so hard to settle the differences and have a final single file which supports both 64 and 32 bit.

[pbek]

Awesome, I will test the pull request! (btw. no need for force pushing, we can squash merge all commits) ;)

[pbek]

Regarding the cpuid.h problem on non-x86 platforms

These lines are the same with our Botan 1 version as they are now:

QOwnNotes/src/libraries/botan/botan.pri

Lines 27 to 28 in 567b27e

	*g++*:DEFINES += BOTAN_BUILD_COMPILER_IS_GCC
	*clang*:DEFINES += BOTAN_BUILD_COMPILER_IS_CLANG

But these lines differ:

QOwnNotes/src/libraries/botan/botan.cpp

Lines 7400 to 7402 in 567b27e

	#elif defined(BOTAN_BUILD_COMPILER_IS_GCC) || defined(BOTAN_BUILD_COMPILER_IS_CLANG)
	#include <cpuid.h>
	#endif

previously there was:

#elif defined(BOTAN_BUILD_COMPILER_IS_GCC) && (BOTAN_GCC_VERSION >= 430)

  // Only available starting in GCC 4.3
  #include <cpuid.h>

namespace {

  /*
  * Prevent inlining to work around GCC bug 44174
  */
  void __attribute__((__noinline__)) call_gcc_cpuid(Botan::u32bit type,
                                                    Botan::u32bit out[4])
     {
     __get_cpuid(type, out, out+1, out+2, out+3);
     }

  #define CALL_CPUID call_gcc_cpuid

}

[pbek]

I changed above code back to the Botan 1 version and tried a new release-build. It's still the same, it fails to build on non-x86 platforms.

I've no idea why, any clues @randombit?

[pbek]

All arm builds failed on

• https://build.opensuse.org/package/show/home:pbek:QOwnNotes/desktop
• https://launchpad.net/~pbek/+archive/ubuntu/qownnotes/+packages
• https://code.launchpad.net/~pbek/+snap/qownnotes

[pbek]

On https://build.snapcraft.io/user/pbek/QOwnNotes we can tests commits of QON without releasing.
It also tells us that s390x and ppc64el are failing too.

[Waqar144]

@pbek just comment this line:

QOwnNotes/src/libraries/botan/botan.h

Line 92 in 567b27e

#define BOTAN_TARGET_CPU_IS_X86_FAMILY

and add to both these lines separately in botan.pri:

QOwnNotes/src/libraries/botan/botan.pri

Lines 32 to 34 in 567b27e

	contains(QT_ARCH, i386):DEFINES += BOTAN_TARGET_ARCH_IS_X86_32
	else: contains(QT_ARCH, x86_64): DEFINES += BOTAN_TARGET_ARCH_IS_X86_64 \
	BOTAN_TARGET_CPU_HAS_NATIVE_64BIT

BOTAN_TARGET_CPU_IS_X86_FAMILY

It's failing because on line

QOwnNotes/src/libraries/botan/botan.cpp

Line 7394 in 567b27e

#if defined(BOTAN_TARGET_CPU_IS_X86_FAMILY)

it checks whether the cpu is x86 or not and since

QOwnNotes/src/libraries/botan/botan.h

Line 92 in 567b27e

#define BOTAN_TARGET_CPU_IS_X86_FAMILY

says it is, it goes ahead to include cpuid.h

I think this will solve it.

[pbek]

Any PR coming up? 😸

[Waqar144]

hahah ok. I will send in a few minutes.

[pbek]

s390x and ppc64el already looking fine on https://build.snapcraft.io/user/pbek/QOwnNotes, great job @Waqar144!

[pbek]

Arm is working fine too, awesome!

[Waqar144]

Yep! I was just looking at that! Finally made it through for all platforms.
To be honest, I wasn't expecting s390x and ppc to go through

[pbek]

What's still left are all the warning: this header will be made internal in the future...

[Waqar144]

Yes, but I don't think they are going to cause any problems. They are just warnings that all this part of code will be made internal (moved into botan_internal?)
The Botan author could advise us on this.

I can remove these warnings however.

[pbek]

Yes please, best lets remove them in our build, they are just causing confusion.

[pbek]

Meanwhile another release is building. We'll see what happens on https://build.opensuse.org/package/show/home:pbek:QOwnNotes/desktop in the next minutes. 😄

[pbek]

No wonder the last release on OBS didn't work, it was revision 666. 🤣 Now we are at 667!

[Waqar144]

Yes please, best lets remove them in our build, they are just causing confusion.

Alright, let's do that.

Fingers crossed! I am optimistic it will build this time 😉😁

[pbek]

The first arm builds are already succeeding. 🎉

[pbek]

Everything seems to be alright, awesome job, @Waqar144! All jobs ran through!

New glory would await you for example at #125, since you seem to like tricky things. 😆

[pbek]

And a big thank you to @randombit for Botan and your concern helping us!

[Waqar144]

Everything seems to be alright, awesome job, @Waqar144! All jobs ran through!

New glory would await you for example at #125, since you seem to like tricky things. laughing

Haha, That's what i was originally intending to work on lol.
That and the Source Code highlighting issue. That's something I really want, at least in the preview(have you thought about using QWebEngine with QWebchannel + a good markdown js library?).

[pbek]

QWebEngine

yes, I did. But there are a lot of issues with it. Starting with the inability to make it build on ApVeyor (Windows) and Travis (for macOS).

[Waqar144]

yes, I did. But there are a lot of issues with it. Starting with the inability to make it build on ApVeyor (Windows) and Travis (for macOS).

I will try that later. Maybe we can find a way around it.

Anyways, on to the spell checker for now.

[randombit]

Yes the warnings are obnoxious sorry about that! Will be fixed in next release. Tracking this in randombit/botan#2164

[pbek]

Great, thank you for all the work!