Update Wireshark Demo #316
Conversation
Hawazyn
force-pushed
the
main
branch
2 times, most recently
from
668f742 to
8d45f25
Compare
|
Hey @baentsch, I wanted to kindly follow up on this pull request and see if there’s any feedback or additional steps needed from my side to help move it forward. |
Hi @Hayyaaf thanks for the update to use jinja2 -- my original idea was that this code could be added to the jinja2 code setup within oqs-provider (oqs-template folder) but the way you did it is also OK as this arguably only is needed for Two immediate questions: 1) Why is wolfssl support dropped? 2) Did you validate your code with different |
I realize now that I may have misunderstood your original idea . However, if aligning more closely with the oqs-template setup is preferred, I’m happy to explore and adjust the implementation accordingly.
From what I see, the project is moving towards using OQSProvider with OpenSSL 3, so I opted for this approach and removed WolfSSL. Since the old WolfSSL setup required adding algorithms manually, this change reduces the maintenance overhead and simplifies the integration process.
I tested with |
I'm struggling myself with this: It would make sense to (embed it there to) ascertain future availability of "qsc.h" as the code base of
That I can completely understand -- and avoiding manual intervention is what I'd want to avoid wherever possible, too. So, also there, creation of a suitable file better would be integrated to the WolfSSL code base to enable "automatic maintenance". As that's not the goal of OQS, I'm fine with dropping this then.
That's what I was afraid of -- indicative of this problem re-occurring in the future as |
I am committed to ensuring sustainable future support for Wireshark. Anyways, my current approach is copying generate.yml and then utilizing the Jinja2 templates. While I have already tested it, I plan to conduct more comprehensive testing to ensure the generate.yml file continues to work even if it undergoes changes.
Given that OpenSSL with the OQS provider already supports a wide range of PQC algorithms, I wonder why supporting WolfSSL remains important?
Regarding the test results, everything now works correctly on OQS provider version 0.6.0 after addressing an earlier oversight. Let me know if further testing across versions is needed. |
That's a key statement, removing some of my concerns about more automation: Thanks!
It's not important for OQS, but may be for those users we share. Also, I hate giving up functionality without need. But again, we "kind of gave up" on wireshark enablement, so your work is very valuable and should prioritize on helping OQS users. So I'm fine with dropping WolfSSL support.
Nope -- that's sufficient -- particularly in combination with your first statement. |
|
I’m really glad to hear that, and it’s great that we’re aligned on the same page. I have a quick question: would it be possible to include my name in the support list and link to my LinkedIn profile, instead of my GitHub account? |
What would be the reason for that? We always use GH accounts such as for anyone to easily reach out by tagging to the person maintaining. Using LinkedIn would make this pretty complicated. Isn't it sufficient to add the LinkedIn reference to your GH account? |
I totally understand the preference for GitHub links, but as I’m applying to jobs and universities, linking my LinkedIn profile would greatly help showcase my contributions to a broader audience, including recruiters. Would you please consider this adjustment? |
I'm not sure I can follow the logic there: We're talking about a link to a LinkedIn profile, not from a LinkedIn profile, right? Only the latter would showcase GH contributions to a broader, non-GH audience. I do see the benefit for you, so what about a compromise: You add your GH handle but with a link-markup ("[...]()")to LinkedIn? That way the table format doesn't get mangled, people looking up the table can still find your GH handle in case of questions/problems and the LinkedIn connection becomes visible for people unable to check GH profiles: That said, looking at your GH profile isn't very helpful for recruiters: By keeping things private (and not linking to LinkedIn, for example) I'm afraid you're doing yourself a disservice for an audience that does check GH. |
|
I apologize for the miscommunication earlier and appreciate your patience. I agree with your suggestion. I'll update the support list to display my name, with a hyperlink to my GitHub profile. Is it okay to put my name instead of my GitHub username, or is it necessary to use the username? |
After you've had a chance to read this, I'll kindly start deleting the comments that come after the message above, including this one, to keep this PR focused on technical aspects. Thank you for your understanding. |
Hawazyn
force-pushed
the
main
branch
2 times, most recently
from
82135b3 to
0f1e870
Compare
Please don't delete the comments—your questions about linking to your LinkedIn profile (and your reasons for wishing to do so) are valid and help set a precedent in case others want to do the same in the future. |
Sure, I will keep them as requested. |
|
@Hayyaaf Your reference update as you did it is OK: This way people can both tag you and find you via LinkedIn. Before merging, please rebase. |
|
@baentsch Done. |
There was a problem hiding this comment.
Thanks for the re-base @Hayyaaf -- Basically this now LGTM -- I'd still like to suggest some improvements to make deployment on docker hub easier (to maintain): See single comments. But then again, this auto-deployment to docker hub is not active right now (so addressing the comments may be part of a new PR, also taking #318 into account).
README.md
Outdated
|
|
||
| We are explicitly soliciting contributors to maintain those integrations labelled "unsupported". | ||
|
|
||
| Currently available integrations at their respective support level: | ||
|
|
||
| | | **Build instructions** | **Pre-built Docker image or binary files** | Support | | ||
| | | **Build instructions** | **Pre-built Docker image or binary files** | Support? | |
There was a problem hiding this comment.
My apologies, that was a mistake. I’ll correct it.
README.md
Outdated
| | **Unbound** | [Github: oqs-demos/unbound](unbound) | [ Dockerhub: openquantumsafe/unbound](https://hub.docker.com/repository/docker/openquantumsafe/unbound) | unsupported | ||
|
|
||
|
|
||
| It should be possible to use the openssl (s_client), curl and GNOME Web/epiphany clients with all algorithm combinations available at the Open Quantum Safe TLS/X.509 interoperability test server at https://test.openquantumsafe.org (set up using `oqs-provider v0.6.1` and `liboqs v0.10.1`) but no guarantees are given for software not explicitly labelled with the name of a person offering support for it. Since [OQS-BoringSSL](https://github.com/open-quantum-safe/boringssl) no longer maintains the same set of algorithms, software that depends on OQS-BoringSSL (e.g., nginx-quic and curl-quic) may not fully (inter)operate with the test server. |
There was a problem hiding this comment.
Why this massive diff? I'd have expected only changes to Wireshark...
There was a problem hiding this comment.
The large diff is due to reordering; I moved Wireshark to the top of the unsupported demos.
|
@baentsch Could you please review the updates? I intentionally made the top section redundant in both README.md and USAGE.md because users accessing the repository will primarily refer to the README, while Docker Hub users will focus on the USAGE guide. This ensures clarity and accessibility for each audience. Additionally, the commands in the usage instructions need to be updated if the Docker image has already been uploaded to a registry. Should I create an issue to track this? |
Could you please confirm if everything looks good now? |
There was a problem hiding this comment.
The README looks good now, but there's a few things you need to update in the Dockerfile to bring it up to date with the changes made across all demos in #298
Given I'm leaving on a short vacation tomorrow, I most likely won't have time until next Monday to check out the code and try building and running it locally, but if I do find time later today to do so I'll share any results
wireshark/Dockerfile
Outdated
| RUN git clone --depth 1 https://github.com/open-quantum-safe/liboqs.git liboqs && \ | ||
| git clone --depth 1 https://github.com/openssl/openssl.git openssl && \ | ||
| git clone --depth 1 https://github.com/open-quantum-safe/oqs-provider.git oqs-provider && \ |
There was a problem hiding this comment.
You should checkout specific releases for these and set the branch using the standard ENV VARs, you can see this in the curl demo Dockerfile.
The env vars should be set in the same way as that Dockerfile with the same names and default values, this allows for consistent docker images and easier CI and releases across all demos
| --openssldir=${INSTALLDIR}/ssl \ | ||
| shared && \ | ||
| make -j$(nproc) && \ | ||
| make install_sw install_ssldirs |
There was a problem hiding this comment.
Without testing it myself, I'm pretty sure this Dockerfile will fail on arm64 systems (like macOS). This can be addressed by symlinking lib and lib64 for openssl as seen in the openssl3 demo Dockerfile
You should be able to test this by running the Docker build and run steps with --platform linux/amd64,linux/arm64 as detailed in the docker docs, though I believe it requires installing specific docker support to do so (usually included by default in Win/macOS installs)
Don’t worry about it. Please take your time and enjoy your vacation. There’s no rush to address this. |
Besides the changes made across all demos, I’m considering adding a troubleshooting section to address issues like the one you encountered earlier. Do you think it would fit better under ‘USAGE’ or in the ‘README’? |
These are all assumptions based on a non-existent foundation @ajbozarth : This project makes no representations whatsoever regarding the platforms supported, most definitely it doesn't guarantee execution on any: if you want to change this, please contribute a PLATFORMS.md file together with suitable testing to back it up. Manually checking some code works on some platforms is absolutely not scalable/maintainable. Until this has changed, IMO USAGE.md files should state help/limitations as they may be known for specific platforms, ideally soliciting feedback (best PRs) for improving things. |
Hawazyn
force-pushed
the
main
branch
2 times, most recently
from
9381e55 to
bab5800
Compare
|
I have pinned the versions at the top, consistent with the rest of the demos. |
wireshark/USAGE.md
Outdated
|
|
||
| Execute this command to open the wireshark window on your host: | ||
| You can run the Wireshark Docker container on Linux, Windows, or macOS using the following command: |
There was a problem hiding this comment.
As discussed elsewhere, is this statement really correct? Has this been tested OK under these platforms? The reference to ".X11-unix" seems only to apply to X-Windows based systems, not to Windows or MacOS...
There was a problem hiding this comment.
I have personally tested this on both Windows and Linux, and it works as expected on those platforms. The only platform that hasn’t been tested yet is macOS.
There was a problem hiding this comment.
In that case, I'd suggest removing this statement as it may mislead users. Maybe put a statement like "MacOS not tested. Feedback/suggestions for improvement for that platform welcome at https://github.com/open-quantum-safe/oqs-demos/issues"?
There was a problem hiding this comment.
Done, I've applied the change to clarify macOS is not tested.
Hawazyn
force-pushed
the
main
branch
3 times, most recently
from
1ee12f4 to
bc1db60
Compare
wireshark/USAGE.md
Outdated
|
|
||
| To this end, it contains references to algorithms supported by [liboqs](https://github.com/open-quantum-safe/liboqs) and [OQS-OpenSSL](https://github.com/open-quantum-safe/openssl) from the [OpenQuantumSafe](https://openquantumsafe.org) project. | ||
| ```bash |
There was a problem hiding this comment.
This is build documentation that should not be necessary (read: is probably confusing) for normal users (of a readily built dockerimage) and thus should only be present in the README.md.
wireshark/USAGE.md
Outdated
|
|
||
| The image is based on Ubuntu and requires the host to run the Unix X-Window system. | ||
| Replace `<your_host_ip>` with your IP address (e.g., `192.168.x.x`) and `<your_display_port>` with your display port, | ||
| typically `:0`. |
There was a problem hiding this comment.
Does this also apply to Windows? I only know this from Unix/X11...
There was a problem hiding this comment.
Why is this command listed twice (here and below)?
There was a problem hiding this comment.
Does this also apply to Windows? I only know this from Unix/X11...
Yes, this setup can work on Windows as well, but as mentioned in the updated documentation, additional steps are needed to enable X11 display forwarding.
Sorry I see these questions only now, @Hayyaaf : I don't think a separate issue is needed if the USAGE.md file is streamlined a bit as per my latest comments (removing build instructions, basically). Otherwise, the only difference should be the name/location of the image. You may want to take a look at the other demos how we phrased things there to be understandable if the file is deployed on dockerhub (or another registry) as well as within GH. |
|
Hi @baentsch, Thank you for your feedback. To be honest, I did feel a bit overwhelmed while addressing the updates yesterday, but I’ve done my best to streamline the documentation as suggested. Please take a look at the updated version when you have time. The only redundant part left now is the note about macOS support, which mentions that it hasn’t been tested yet and invites feedback. Let me know if any further adjustments are needed. |
Let me apologize in turn: There's been indeed too many "open ends" also on my side -- and failure to write one coherent PR feedback. Will do that later today and try to avoid creating separate issues in general in the future. Thanks for bearing with me @Hayyaaf ! |
Please take your time; there is no need to rush.
Honestly, I would like to thank you so much for your patience and openness throughout this process. I’ve learned a lot from working with you, and I truly appreciate your support and understanding. |
You're welcome! I'm glad I could help bring this demo back to life. Thanks again for your guidance and support throughout the process! |
You're welcome. Thanks in turn for this contribution! One nit, though: LinuxFoundation wants DCO validation on contributions, so asking @ryjones to help you understand what's needed to get that CI item to "green" status |
- Upgrade Ubuntu to version 24.04. - Upgrade Wireshark to version 4.4.1. - Integrate OpenSSL 3 with liboqs and the OQS provider. - Automate the generation of `qsc.h` using `generate_qsc_header.py`. - Organize the build with dedicated directories for sources, builds, and installations. - Migrate from Qt5 to Qt6 for improved compatibility. - Update `README.md` and `USAGE.md`. Signed-off-by: Khalid <[email protected]>
Signed-off-by: Khalid <[email protected]>
|
Good morning @baentsch, Thanks for the reminder, I have added the DCO sign-off now. |
|
Back from vacation and figured I'd leave a final comment here:
@baentsch thanks for this clarification, I had been working on the assumption the docker images should run on any platform in my testing. I agree that requiring and testing it would be unscalable, and small nots in the docs is good enough. @Hayyaaf I tried the demo again and got it running following your comment about using my IP, and realized that I needed to specifically us my 10.0.. IP, not localhost or public IP. So if you want to open a follow up PR updating the docs, we can remove the "not tested on Mac" section. @Hayyaaf I wasn't able to find the original comment you left about the IP address though. Based on prior discussion it seems you went back and deleted some comments you made on the PR, as it seems you're new to open source I wanted to let you know that in general it's best to not delete comments, old comments, even if not relevant can be helpful. On GitHub you can edit comments or hide them instead and readers can still see the comments if they want. |
|
@ajbozarth Thank you for your guidance and for trying the demo again. Yes, I deleted some of my earlier comments because I had the chance to test it on Mac, but unfortunately, it didn’t work due to a misconfiguration in the X11 setup. I will consider submitting a follow-up PR to update the documentation accordingly. Thank you again for your patience, it is much appreciated. |
Summary
This update enhances the Wireshark demo in the
oqs-demosrepository by integrating OpenSSL 3 with the OQS provider for quantum-safe cryptography support. Key improvements include:Updated Dockerfile:
Wireshark 4.4.1(latest version) with OpenSSL 3 with the OQS provider.Automated Header Generation:
generate_qsc_header.py, a Python script that automates the generation ofqsc.h, defining post-quantum cryptographic algorithms for Wireshark.ALGORITHMS.mdfile in the OQS provider repository to ensure support for the latest algorithms.Project Cleanup and Expanded Documentation:
README.mdwith comprehensive setup instructions, configuration options, and cross-platform usage guidelines.USAGE.md,build.sh, andwolfssl-qsc.h) to streamline the project structure.Cross-Platform Notes
Guidance for Linux and macOS display setup was drawn from documentation. Further testing on these platforms is recommended to ensure full functionality and compatibility.
Testing Summary
The updated Wireshark demo has been verified on Windows 11 with Docker 4.35.1 (latest version). Testing confirmed a successful build and functionality of Wireshark with post-quantum cryptography support.
Test Screenshots
The following screenshots show the successful integration of post-quantum cryptography in Wireshark. The tests capture traffic using the Kyber1024 and Frodo640aes algorithms.