oqs-demos
oqs-demos copied to clipboard
Pin libraries to current releases
Sets the default tag for openssl, liboqs, oqs-provider to the current latest relese instead of main/master. Also updates curl to the latest release.
Inlcudes some fixes to support multi-platform builds, now supporting both linux/amd64 and linux/arm64, where only linux/amd64 worked before.
Related Issues: #230 #213 #182
Demos updated (ordered by interest):
- [x] curl
- [x] nginx
- [x] openssl3
- [x] http
- [x] openssh
- [x] ~chromium~ N/A
- [x] epiphany
- [ ] haproxy
- [ ] wireshark
- [ ] envoy
- [x] h2load
- [ ] mosquito
- [x] ngtcp2
- [ ] openlitespeed
- [x] openvpn
- [ ] unbound
- [x] ~quic~ N/A
I was originally going to open an issue about the fact that the curl demo couldn't build/run on macOS without the --platform linux/amd64 flag, but as I worked on this changes I made solved that issue. Also #213 seems to partially cover that issue
@ajbozarth Thanks for the fix to the lib64 installpath. What's missing in my view is testing (CI) for this: I'd suggest two runs (via GH CI): One building the "main/master" image as "canary" for OQS; the second one the "version pegged" image for user consumption. Of course only the latter should be pushed to docker hub during merge. And also, it should be a cross-platform image, thus truly testing x64 and arm64 "suitability". Thanks.
The curl/README.md should be updated with any new arguments, and the change in defaults (Rather than stating explicit version it could just say 'updated to the most recent versions that have been tested' or similar).
Noticed that text in README.md refers to how to build the image, and provides a few examples. But the tag varies.
Once we get to USAGE.md it states to rundocker run -it openquantumsafe/curl perftest.sh- so by default this will run the version from dockerhub.
It could be helpful to at least have the default build example using the correct tag perhaps docker build . -t open-quantum-safe/curl
What's missing in my view is testing (CI) for this
@baentsch After our discussion during this week's sync up I took another look and realized I had only investigated the GitHub Actions and not circleci, leading me to think I would have to write tests from scratch. Which I evaluated would be better done in a follow up PR, but upon finding the circleci test configs I will make updates there. Though as #294 says, we should move our circleci automation to gh actions in a future PR.
But the tag varies
@planetf1 I will take a look through the docs, and make sure the tag it uses (docker hub or local build) matches the context of the section it's in. If it doesn't or is unclear I will made the edit or add clarification to the surrounding text
I just pushed an update addressing @planetf1 inline comments, and adding a couple lines to the circleCI to build and test the curl image using main/master libs as well as the pinned ones.
@planetf1 I looked into the docs for inconsistencies in the build image name and found that is very consistent, using the local image name in the README and using the docker hub image in USAGE. It also includes a line at the top of USAGE clarifying that it uses the public image name throughout that doc.
The curl/README.md should be updated with any new arguments
I updated the README to include the new ARGs and removed direct reference to versions so as any future updates to the pinned version would not require updates to the docs
Lastly, on the CI update, what I pushed was a simple addition since we should add more through tests when we move testing to GH Actions, which currently only does builds, and only for one demo. (covered by #284 IIUC)
As promised in https://github.com/open-quantum-safe/oqs-demos/issues/284#issuecomment-2377475642 I have just pushed my update for the nginx demo and added a task list to the top description for tracking my progress in updating demos. I will also be updating the circleCI for the 3 jobs currently being run (openssl3, nginx, and httpd).
As I will be pushing updated here periodically as I finish them, I will ping again for review when I have finished. Feel free to either wait until then or review as I go at your leisure.
Status Update
Fully updated demos
- curl
- nginx
- openssl3
- httpd
- openssh
- h2load
- openvpn
Updated demo (with issues)
- ngtcp2 - I was only able to update
ngtcp2tov1.5.0(released May 2024) and notv1.8.1due to encountering a segfault in later versions - epiphany - I have pushed my updates for this demo, but due to issues with X Windows I was unable to test it. If someone running an OS with native X Windows could test it. It is intended to run on Gnome, but I only had access to working macOS, Windows and Fedora instances, all of which don't have native X Windows.
Still to be worked on
- haproxy - @davidkel offered to take a stab at updating this demo
- wireshark - This demo has interest, but would take work to update
- envoy - This demo is out of date with no interest, should we drop it?
- mosquito - @davidkel offered to take a stab at updating this demo
- openlitespeed - This update will require recreating the openlitespeed patch files (impl w/o patch fixes)
- unbound - This demo is out of date with no interest, should we drop it?
Personal notes
I believe the work here is getting close to done. I plan to bring up some of my notes above at next Tuesday's meeting to discuss what "done" is here. After the discussion with the team I plan to start looking at #294 as a follow up to this work.
ngtcp2 should be able to build with boringssl
ngtcp2 should be able to build with boringssl
Supporting building with boringssl is a great idea, but the current demo doesn't support it. So I would consider that a new feature that should be handled in a separate PR
Thanks for the summary, @ajbozarth . Regarding haproxy, may I suggest @pi-314159 and @davidkel sync? IMO all that's needed there is a link to the haproxy documentation as per https://github.com/open-quantum-safe/oqs-demos/issues/273#issuecomment-2284599611. Unless of course you want to keep supporting a docker image despite the upstream apparently already supporting oqs (?). Sounds a bit like wasted effort, no?
Unless of course you want to keep supporting a docker image despite the upstream apparently already supporting oqs (?). Sounds a bit like wasted effort, no?
I get what you mean, I'll discuss with @davidkel and see if he still wants to update if it's just "putting the doc steps into a Dockerfile" I'll also raise the topic of just dropping it in Tuesday's meeting (I already planned on discussing dropping at least unbound since it's out of date and there was no interest in it).
Sharing plans based on todays discussion on the OQS call:
Demo specific updates:
- epiphany - @SWilson4 volunteered to test this since he's running Ubuntu, if it work it will stay, otherwise we will most like decide to deprecate this demo due to the X11 dependency
- haproxy - @davidkel has sent me a patch file, so I will be updating this demo and pushing in the next couple days
- wireshark - @davidkel will be looking into this, but has not committed to updating it. The OQS team still agrees this demo should be updated and supported despite it's X11 dependency
- envoy - We will mark this demo deprecated
- mosquito - @davidkel is currently working on updating this
- openlitespeed - I will be taking some time to create the patch files and update this demo
- unbound - We will mark this demo deprecated
Follow up work:
- I will be updating the deprecated demos README to clarify their deprecation
- In a separate followup PR I will move the deprecated demos into a new
deprecateddirectory (as decided on the call) - Withe this there will now be three tiers of demos:
- Supported: demos that have a support person
- Unsupported: demos without a support person (these will still be "supported" by CI though)
- Deprecated: demos that are out of date, these will be moved into a
deprecateddirectory and not included in CI
- In a followup PR I will be adding GitHub Actions CI to replace circleCI, this will handle building, testing, and deploying demos
FYI X11 is still working for me on mac sequoia (I have XQuartz installed) and Fedora core 41 .. will try demo
Withe this there will now be three tiers of demos:
Supported: demos that have a support person
Unsupported: demos without a support person (these will still be "supported" by CI though)
Deprecated: demos that are out of date, these will be moved into a deprecated directory and not included in CI
"Unsupported" but CI-supported -- sounds weird. Can I suggest the terms Maintained - Unmaintained - Deprecated instead? For the latter I'd also presume the existing docker images (and links to them) will be removed when these terms get defined/documented as to what they actually entail? For example, I'd imagine Maintained integrations shall also be tested against the respective master while Unmaintained ones will only run CI against the known-good ("pinned") version(?)
Maintained - Unmaintained - Deprecated instead
This is much better, thank you
For the latter I'd also presume the existing docker images (and links to them) will be removed when these terms get defined/documented as to what they actually entail?
We did not discuss what to do with already published docker images for deprecated demos, In my opinion as long as those published docker images still run as expected there no reason to delete them.
I'd imagine Maintained integrations shall also be tested against the respective master while Unmaintained ones will only run CI against the known-good ("pinned") version(?)
I had intended to run all of the against main/master, but when I get to the CI work we can discuss if we only want to test the maintained demos and not unmaintained against main/master
I tried epiphany on fedora workstation 41 (where I can run X11 apps)
I noticed the launch invocation seems incorrect in that it does not pass $DISPLAY. When I do so I get:
➜ epiphany git:(curl) docker run --net=host --env DISPLAY=${DISPLAY} --volume="$HOME/.Xauthority:/home/oqs/.Xauthority:rw" epiphany
(epiphany:7): dbind-WARNING **: 17:33:17.498: Couldn't connect to accessibility bus: Failed to connect to socket /run/user/1000/at-spi/bus: No such file or directory
Gtk-Message: 17:33:17.521: Failed to load module "canberra-gtk-module"
Gtk-Message: 17:33:17.521: Failed to load module "pk-gtk-module"
Gtk-Message: 17:33:17.522: Failed to load module "canberra-gtk-module"
Gtk-Message: 17:33:17.523: Failed to load module "pk-gtk-module"
bwrap: No permissions to create new namespace, likely because the kernel does not allow non-privileged user namespaces. See <https://deb.li/bubblewrap> or <file:///usr/share/doc/bubblewrap/README.Debian.gz>.
** (epiphany:7): ERROR **: 17:33:17.921: Failed to fully launch dbus-proxy: Child process exited with code 1
/home/oqs/startepiphany.sh: line 9: 7 Trace/breakpoint trap (core dumped) epiphany
which is some issue relating to the accessibility bus. The main app window starts to draw, then the app exits. However I don't think this is necessarily worth pursuing
I would suggest we update the textual description for each image published on dockerhub. In particular I agree we don't need to remove old demos, but we should at least clearly add a note to the description that they are maintained or deprecated, as well as ensure there's a link back to the relevant directory in this source repository.
Per my above comments I have pushed the following:
- @davidkel updates to haproxy and mosquitto demos
- doc updates to deprecate envoy, wireshark and unbound
- Updated README to use new "Maintained - Unmaintained - Deprecated" keywords (note table diffs are messy)
This leaves two open action items:
- [x] Update openlitespeed demo: which I'll be doing next
- [ ] Test epiphany demo: If this come back broken for @SWilson4 we will revert the update and instead deprecate it
Point of discussion - Wireshark
@davidkel brought to my attention that Wireshark already added a bunch of PQ support a while back (see here), given the overhead for updating the wireshark demo and the existing support I decided to mark it deprecated. If there is push back on this I suggest we open a follow up issue to "un-deprecate it" and some other brave soul can pick it up.
Was looking through the issue backlog while waiting on a openlitespeed build and the expanded work in the PR might mean closing a handful of open issue with the PR:
- #230 - The issue this PR set out to fix
- #182 - This Pr either update or deprecate all remaining openssl 1.1.1 dependent demos
- #273 #226 - HAProxy demo has been updated and now builds on mac
- #266 - Depending on Spencer's testing results, this may be closed, either due to it working again or deprecating it
- #255 - Wireshark has been deprecated, but we may want to repurpose this issue to handle un-deprecating it as mentioned above
- #171 - Unbound has been deprecated so there no need to add CI for it
If there's no issues with my above assertions I will add Fixes #230 #182 #273 #226 #266 #171 (note #255 not included) to the top description to be auto closed when this PR is merged.
After some effort trying to update the openlitespeed demo, I'm hit too many build issues and don't have the expertise on openlitespeed to sort them out. As there was no interest in the openlitespeed demo to begin with I am marking it as deprecated instead. If there's interest I can still push the updates I made that aren't building if someone else wants to take a shot. They are currently sitting in a separate branch on my fork, where I'll leave it for future reference https://github.com/ajbozarth/oqs-demos/commit/24f81fbb05989249ad27d459e336cc90dac7a2c9
As mentioned above I've added a Fixes tag to close the related Issues in the description, if anyone feels one of those Issues should not be closed by this PR according to my previous reasons, I can remove it. I also self assigned those Issues under the assumption that this PR addresses them.
I also opened #312 as a follow up issue to handle the creation of a deprecated directory as we discussed.
As mentioned I left a comment on #255 since I did not include it in the Fixes list despite deprecating wireshark.
Lastly @SWilson4 did you have any luck testing epiphany? Because if it does not work for you I plan to do the following:
- [x] revert the epiphany updates in this PR
- [x] push those updates to a separate branch like I did for openlitespeed
- [x] remove #266 from the Fixes list and leave a comment like I did for wireshark on #255
- [x] mark epiphany as deprecated
Worth noting here: I just found out that openssl-3.4.0 was release last week. I'd rather not block the merge of this PR, instead I would like to wait until my CI updates and take the opportunity to use the update to openssl-3.4.0 to test them
I get a similar error to @planetf1 when I try to run epiphany on Ubuntu 24.
I get a similar error to @planetf1 when I try to run epiphany on Ubuntu 24.
In that case I'll take some time today to go through the steps I listed above to deprecate epiphany.
I also noticed CI complaining about my lack of commit sign-offs for my more recent commits, I'll address that at the same time
I've taken the steps to deprecate epiphany, and updated the relevant docs and GitHub issues.
With this, the PR is now in a state of final review. I have no more outstanding work items or review items to address prior to merging.
The only caveat is the DCO CI failing due to my forgetting to sign off on my later commits. Remediating this would be messy due to multiple merges with master. I could do a squash and force push if it is an actual blocker, but afaik every PR is merged with a "Squash and merge" so is it necessary? If so I will need to create a new branch to hold the epiphany commit that's referenced in its deprecation message since a squash and force would delete the referenced url. @baentsch you usually handle merges in this repo.
As an FYI in case it's useful in the future for @ajbozarth or others, you can automate signing off on commits for a repo by following the (very well hidden) instructions in .git/hooks/prepare-commit-msg.sample.
As an FYI in case it's useful in the future for @ajbozarth or others, you can automate signing off on commits for a repo by following the (very well hidden) instructions in .git/hooks/prepare-commit-msg.sample.
Thanks, I knew I could automate it, but this is the first project I've worked on that required sign offs so I hadn't taken the time to figure out how to do so
A few updates per discussion in todays call:
- I fixed a few ARG names I missed so that all demos use the same consistent
ARGs for liboqs, provider, and openssl (for easy find and replace updates) - I moved the referenced git commits in the epiphany and openlitespeed deprecation messages into a separate branch so that this PR's git history can be squashed (I also added a rule to prevent me from accidentally deleting the reference branch)
After this PR has final approval I will run a giant git squash with sign-off to fix the DCO failures as per todays discussion (unless @ryjones you have better solution?)
After this PR has final approval I will run a giant git squash with sign-off to fix the DCO failures as per todays discussion (unless @ryjones you have better solution?)
You could to a rebase, packing down to a minimal number of commits. A squash is also OK. You could do one commit for deprecated demos, one for updated demos, or whatever reads best.