require Botan 2.14.0 #1371

rrrooommmaaa · 2020-12-27T12:25:26Z

rrrooommmaaa · 2020-12-27T20:07:20Z

@ronaldtse Looks like we need to put correct version of botan to https://github.com/riboseinc/yum
CentOS 7 is missing it.
CentOS 8 has 2.12.1
Ubuntu 20.04 also has 2.12.1
https://pkgs.org/search/?q=botan2

ronaldtse · 2020-12-28T01:28:43Z

@ronaldtse Looks like we need to put correct version of botan to https://github.com/riboseinc/yum
CentOS 7 is missing it.
CentOS 8 has 2.12.1
Ubuntu 20.04 also has 2.12.1
https://pkgs.org/search/?q=botan2

@dewyatt could you help with it since you are familiar? Thanks!

dewyatt · 2020-12-28T10:59:00Z

@ronaldtse Looks like we need to put correct version of botan to https://github.com/riboseinc/yum
CentOS 7 is missing it.
CentOS 8 has 2.12.1
Ubuntu 20.04 also has 2.12.1
https://pkgs.org/search/?q=botan2

@dewyatt could you help with it since you are familiar? Thanks!

@ronaldtse It's been a while and it looks like there are some issues here. I think I would need maintainer access on GitLab ribose/rpm-spec-botan2 to troubleshoot this.
Alternatively if someone with permissions can maybe look at the mirroring settings and try to force sync that might be all I need.

ronaldtse · 2020-12-28T14:14:53Z

@ribose-jeffreylau could you help here? It would probably be better to move this yum repository to GitHub...

ribose-jeffreylau · 2020-12-29T04:18:56Z

@dewyatt I gave you maintainer access to https://gitlab.com/ribose/rpm-spec-botan2.

Since we don't have a subscription at GitLab, we can no longer mirror GitHub repos in GitLab, that's why the updates aren't automatically built. I manually pushed your changes to GitLab to see if it built --- there're some errors that may require your help.

@ronaldtse Should we investigate moving all the rpm-spec-* CI from GitLab to GitHub?

ronaldtse · 2020-12-29T04:26:42Z

@ribose-jeffreylau we should definitely move all the repos (and workflows) from GitLab back to GitHub, especially since GitHub now supports this type of workflow.

dewyatt · 2020-12-30T06:51:50Z

Well that's unfortunate they took mirroring out of the free tier.
I fixed up the 2.14.0 release so that's published now.

dewyatt · 2020-12-30T07:00:06Z

@ribose-jeffreylau we should definitely move all the repos (and workflows) from GitLab back to GitHub, especially since GitHub now supports this type of workflow.

I think I remember when I migrated these to GitLab that GHA didn't (at the time) have a feature like include:remote (used here), so that may be one of the challenges in migrating those back. But GHA has improved a lot since then, so...

dewyatt · 2020-12-30T08:10:26Z

I think this PR would need an update here as well:

rnp/ci/install.sh

Line 19 in 57c8afb

    
           git clone --depth 1 --branch 2.13.0 https://github.com/randombit/botan "${botan_build}"

(We build most direct dependencies manually in centos CI to get useful stacktraces with ASaN, the RPM is more for end-users)

rrrooommmaaa · 2020-12-30T08:25:46Z

@dewyatt Since there is version 2.16 of Botan in MSYS2 and VCPKG repositories for WIndows, maybe we should use 2.16 everywhere?

dewyatt · 2020-12-30T08:52:59Z

@dewyatt Since there is version 2.16 of Botan in MSYS2 and VCPKG repositories for WIndows, maybe we should use 2.16 everywhere?

Sounds reasonable to me!

rrrooommmaaa · 2020-12-31T10:09:54Z

@ni4 Can you please advise on how to add additional setup steps for CIFuzz? To be exact, how to install Botan manually as we do in other steps, because there is Botan 2.12.1 out of the Ubuntu box.

dewyatt · 2020-12-31T10:15:47Z

@ni4 Can you please advise on how to add additional setup steps for CIFuzz? To be exact, how to install Botan manually as we do in other steps, because we there is Botan 2.12.1 out of the Ubuntu box.

We'll probably have to do some updates to oss-fuzz for this

ni4 · 2020-12-31T10:59:32Z

We'll probably have to do some updates to oss-fuzz for this

Yeah, that's right - Botan for oss-fuzz is installed here: https://github.com/google/oss-fuzz/blob/51098cb7917a822bd8f3adc047c20126c5283113/projects/rnp/build.sh#L20

I'll do a PR and write back once it is merged.

ni4 · 2020-12-31T11:24:31Z

Tracked via google/oss-fuzz#4900

rrrooommmaaa · 2020-12-31T12:05:08Z

@ni4 @dewyatt Should I set up minimum required version 2.16 or just leave 2.14 regardless that we're going to have 2.16 in the repos for the actual build?

dewyatt · 2020-12-31T13:18:31Z

@ni4 @dewyatt Should I set up minimum required version 2.16 or just leave 2.14 regardless that we're going to have 2.16 in the repos for the actual build?

For CMake you mean? I would say we should keep that at the minimum that builds and passes tests for most platforms, which is probably 2.8.0 (some FFI support for x25519 was added there).

2.9.0 might be better since that fixes CVE-2018-20187 (seems relevant to us).

ni4 · 2020-12-31T15:54:57Z

@dewyatt Didn't we need to update to 2.14.0 because of 2020-03-24: Side channel during CBC padding ? It seems relevant to us.

As an alternative we may introduce some variable like CMAKE_FORCE_BOTAN_VERSION, allowing to build with older then 2.14

dewyatt · 2020-12-31T18:06:15Z

@dewyatt Didn't we need to update to 2.14.0 because of 2020-03-24: Side channel during CBC padding ? It seems relevant to us.

As an alternative we may introduce some variable like CMAKE_FORCE_BOTAN_VERSION, allowing to build with older then 2.14

I haven't been keeping up with RNP like I used to so you guys are probably in a better place to make that call, but I don't believe this one affects us.

We use unpadded CBC for G10 so that is not affected.
The fix targets PKCS7, X923, OneAndZeros, and ESP padding. I don't believe we use any of them (via botan). Although it's worth checking whether we have similar issues.
It sounds mostly minor -- the commit message says probably not too serious in most circumstances but is not desirable behavior.

I can also say that adding exit calls to the code paths affected by the fix has no effect on whether ctest passes, so that's a good indicator that we don't need to worry about that CVE.

ni4 · 2021-01-01T11:14:48Z

Happy New Year, guys!

@dewyatt Actually, I think it's more 'political' - to not allow others, who don't dig that deep as you, say 'rnp uses Botan with vulnerabilities'. So I'd vote for 2.14 at least.
@rrrooommmaaa oss-fuzz PR is merged now, so tests should run fine.

ni4 · 2021-01-04T10:42:57Z

@rrrooommmaaa Looks like Windows/MSYS2 workflow still hangs due to Botan's update.

rrrooommmaaa · 2021-01-04T13:41:48Z

@rrrooommmaaa Looks like Windows/MSYS2 workflow still hangs due to Botan's update.

Yes. And setting CTEST_PARALLEL=1 didn't help either.
Strange, but all builds and tests pass ok on my fresh MSYS2 20201109, installed with choco.
Perhaps, need to review msys2 installation by gha.
Do you wish to take over or should I?

ni4 · 2021-01-04T18:50:32Z

Yes. And setting CTEST_PARALLEL=1 didn't help either.
Strange, but all builds and tests pass ok on my fresh MSYS2 20201109, installed with choco.
Perhaps, need to review msys2 installation by gha.
Do you wish to take over or should I?

Feel free to continue on this. I spent a lot of time on this year ago, and wasn't able to find a solution, unfortunately.
There are some details discussed here: #1104 (comment)

rrrooommmaaa · 2021-01-05T10:05:59Z

Feel free to continue on this. I spent a lot of time on this year ago, and wasn't able to find a solution, unfortunately.
There are some details discussed here: #1104 (comment)

So is it because of -DBUILD_SHARED_LIBS=yes?
Should we try -DBUILD_SHARED_LIBS=no or shared lib support is mandatory?

ni4 · 2021-01-05T10:13:45Z

So is it because of -DBUILD_SHARED_LIBS=yes?
Should we try -DBUILD_SHARED_LIBS=no or shared lib support is mandatory?

I don't know, If I know I'd resolve it :) Shared libraries are required for the ruby-rnp, but those tests are not run for msys2/mingw. It would be good to investigate and find the real reason for this problem, to be aware of it in the future.

rrrooommmaaa · 2021-01-05T10:18:02Z

So is it because of -DBUILD_SHARED_LIBS=yes?
Should we try -DBUILD_SHARED_LIBS=no or shared lib support is mandatory?

I don't know, If I know I'd resolve it :) Shared libraries are required for the ruby-rnp, but those tests are not run for msys2/mingw. It would be good to investigate and find the real reason for this problem, to be aware of it in the future.

I understand. But how much time is reasonable to spend on this, @ronaldtse ?
I may end up debugging botan library.

dewyatt · 2021-01-06T16:32:41Z

I can take a look today, it's not that hard to work around it.

EDIT: I did test a couple of workarounds successfully, I'll try to get something out today but I'm slightly out of commission with back pain at the moment.

dewyatt · 2021-01-10T23:43:23Z

I filed randombit/botan#2582 and and created msys2/MINGW-packages#7640. I'll try to get an immediate workaround in here now.

rrrooommmaaa · 2021-01-11T00:59:45Z

I filed randombit/botan#2582 and and created msys2/MINGW-packages#7640. I'll try to get an immediate workaround in here now.

Good! Thanks for your effort in this complicated matter.

rrrooommmaaa · 2021-01-11T13:16:50Z

When the test succeeds, we'll need to restore CTEST_PARALLEL and probably timeout settings too.

ronaldtse · 2021-01-12T00:22:29Z

Seems that the Windows test ran out of time at 3 hours at test 205:

        Start 204: cli_tests-EncryptElgamal
1535
204/213 Test #204: cli_tests-EncryptElgamal ......................................................   Passed   17.75 sec
1536
        Start 205: cli_tests-Misc
1537
Terminate batch job (Y/N)? 
1538

Stuck?

ni4 · 2021-01-12T09:36:19Z

Stuck?

Yeah, looks like updated Botan package didn't get into distribution yet.

ni4 · 2021-01-13T16:12:37Z

ci/before_install.sh

@@ -91,8 +91,8 @@ msys_install() {
    mingw64/mingw-w64-x86_64-python3
  "
  pacman --noconfirm -S --needed ${packages}
-  botan_pkg="mingw-w64-x86_64-libbotan-2.13.0-1-any.pkg.tar.xz"
-  pacman --noconfirm -U http://repo.msys2.org/mingw/x86_64/${botan_pkg} || \
+  botan_pkg="mingw-w64-x86_64-libbotan-2.16.0-1-any.pkg.tar.zst"


@rrrooommmaaa Could you please update it here to mingw-w64-x86_64-libbotan-2.17.3-2-any.pkg.tar.zst as only since this version MinGW applies Daniel's patch from https://github.com/msys2/MINGW-packages/pull/7640/files? I can do it myself, but then we'll end up with rebase/merge hell due to force-push.

P.S. And add some comment why that specific version is used.

Yeah sorry I dropped the ball on this but I have to step away for a few days. That is hopefully a quick solution, I was going to just do a makepkg w/thread_utils disabled in CI so we could have 2.16 across the board, but really I don't think it's that important and your solution should be quicker and easier.

@rrrooommmaaa Could you please update it here to mingw-w64-x86_64-libbotan-2.17.3-2-any.pkg.tar.zst as only since this version MinGW applies Daniel's patch from https://github.com/msys2/MINGW-packages/pull/7640/files? I can do it myself, but then we'll end up with rebase/merge hell due to force-push.

Sure.

ni4 · 2021-01-14T12:44:07Z

@rrrooommmaaa Finally all green! Let's squash it to the single commit and approve.

* require Botan 2.14.0

ronaldtse · 2021-01-14T15:39:16Z

Thank you guys!!

* require Botan 2.14.0

rrrooommmaaa force-pushed the rrrooommmaaa-1279-botan-2-14 branch 3 times, most recently from 7ab1171 to f1eb059 Compare December 27, 2020 14:43

rrrooommmaaa force-pushed the rrrooommmaaa-1279-botan-2-14 branch from f1eb059 to dbb1be6 Compare December 27, 2020 20:13

rrrooommmaaa force-pushed the rrrooommmaaa-1279-botan-2-14 branch from dbb1be6 to fbdfb07 Compare December 28, 2020 10:13

rrrooommmaaa force-pushed the rrrooommmaaa-1279-botan-2-14 branch from fbdfb07 to 20a7382 Compare December 30, 2020 07:27

rrrooommmaaa force-pushed the rrrooommmaaa-1279-botan-2-14 branch from 20a7382 to 5f959f5 Compare December 30, 2020 08:19

rrrooommmaaa force-pushed the rrrooommmaaa-1279-botan-2-14 branch from 5f959f5 to 1a935a8 Compare December 31, 2020 10:02

rrrooommmaaa force-pushed the rrrooommmaaa-1279-botan-2-14 branch 2 times, most recently from ca2248f to 24472d9 Compare January 3, 2021 14:42

rrrooommmaaa force-pushed the rrrooommmaaa-1279-botan-2-14 branch 2 times, most recently from 9d3591f to 5f1338c Compare January 3, 2021 19:50

require Botan 2.14.0

25b881a

rrrooommmaaa force-pushed the rrrooommmaaa-1279-botan-2-14 branch from 5f1338c to 25b881a Compare January 3, 2021 22:05

ni4 mentioned this pull request Jan 4, 2021

Release 0.14.0 #1376

Merged

ni4 added this to the v0.14.0 milestone Jan 4, 2021

ni4 reviewed Jan 13, 2021

View reviewed changes

test

427d74e

rrrooommmaaa merged commit bf76dc2 into master Jan 14, 2021

rrrooommmaaa deleted the rrrooommmaaa-1279-botan-2-14 branch January 14, 2021 15:28

ni4 pushed a commit that referenced this pull request Jan 14, 2021

require Botan 2.14.0 (#1371)

a583210

* require Botan 2.14.0

ni4 pushed a commit that referenced this pull request Jan 14, 2021

require Botan 2.14.0 (#1371)

946d297

* require Botan 2.14.0

ni4 mentioned this pull request Jan 19, 2021

Failure in Windows build in master #1104

Closed

ni4 pushed a commit that referenced this pull request Jan 21, 2021

require Botan 2.14.0 (#1371)

24d0308

* require Botan 2.14.0

require Botan 2.14.0 #1371

require Botan 2.14.0 #1371

Conversation

rrrooommmaaa commented Dec 27, 2020 • edited Loading

rrrooommmaaa commented Dec 27, 2020 • edited Loading

ronaldtse commented Dec 28, 2020

dewyatt commented Dec 28, 2020

ronaldtse commented Dec 28, 2020

ribose-jeffreylau commented Dec 29, 2020

ronaldtse commented Dec 29, 2020

dewyatt commented Dec 30, 2020

dewyatt commented Dec 30, 2020

dewyatt commented Dec 30, 2020

rrrooommmaaa commented Dec 30, 2020

dewyatt commented Dec 30, 2020

rrrooommmaaa commented Dec 31, 2020 • edited Loading

dewyatt commented Dec 31, 2020

ni4 commented Dec 31, 2020

ni4 commented Dec 31, 2020

rrrooommmaaa commented Dec 31, 2020

dewyatt commented Dec 31, 2020

ni4 commented Dec 31, 2020

dewyatt commented Dec 31, 2020

ni4 commented Jan 1, 2021

ni4 commented Jan 4, 2021

rrrooommmaaa commented Jan 4, 2021

ni4 commented Jan 4, 2021

rrrooommmaaa commented Jan 5, 2021 • edited Loading

ni4 commented Jan 5, 2021

rrrooommmaaa commented Jan 5, 2021

dewyatt commented Jan 6, 2021 • edited Loading

dewyatt commented Jan 10, 2021

rrrooommmaaa commented Jan 11, 2021

rrrooommmaaa commented Jan 11, 2021

ronaldtse commented Jan 12, 2021

ni4 commented Jan 12, 2021

ni4 Jan 13, 2021

Choose a reason for hiding this comment

ni4 Jan 13, 2021

Choose a reason for hiding this comment

dewyatt Jan 13, 2021

Choose a reason for hiding this comment

rrrooommmaaa Jan 13, 2021

Choose a reason for hiding this comment

ni4 commented Jan 14, 2021

ronaldtse commented Jan 14, 2021

rrrooommmaaa commented Dec 27, 2020 •

edited

Loading

rrrooommmaaa commented Dec 27, 2020 •

edited

Loading

rrrooommmaaa commented Dec 31, 2020 •

edited

Loading

rrrooommmaaa commented Jan 5, 2021 •

edited

Loading

dewyatt commented Jan 6, 2021 •

edited

Loading