-
Notifications
You must be signed in to change notification settings - Fork 365
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support LibreSSL #2380
base: master
Are you sure you want to change the base?
Support LibreSSL #2380
Conversation
Upstream-PR: tpm2-software/tpm2-tss#2380 Signed-off-by: orbea <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we need a test, perhaps you could create a docker image to add to the ci matrix? You could submit a docker container here: https://github.com/tpm2-software/tpm2-software-container
A test case would be a good idea, but I know almost nothing about docker and I am unsure what distro would even work? The current situation is the primary LibreSSL user is OpenBSD and most linux distros have removed it by default, but I am still trying to keep it working for the Gentoo LibreSSL overlay. https://github.com/gentoo/libressl Additionally OpenBSD doesn't ship tpm2-tss, but Gentoo does. |
Upstream-PR: tpm2-software/tpm2-tss#2380 Signed-off-by: orbea <[email protected]> Signed-off-by: Quentin Retornaz <[email protected]>
I'll build you a container, standby. |
src/tss2-esys/esys_crypto_ossl.c
Outdated
@@ -392,7 +392,7 @@ iesys_cryptossl_hmac_start(ESYS_CRYPTO_CONTEXT_BLOB ** context, | |||
"Error EVP_MD_CTX_create", cleanup); | |||
} | |||
|
|||
#if OPENSSL_VERSION_NUMBER < 0x10101000L | |||
#if OPENSSL_VERSION_NUMBER < 0x10101000L || defined(LIBRESSL_VERSION_NUMBER) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if it's an old version of LibreSSL, this would fail. I think the root cause is that we are not including #include <openssl/opensslv.h>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have patches to another project that I think are a more robust fix to the same problem: https://github.com/stefanberger/libtpms/pull/340/files
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually probably not the right fix, silly libressl:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tested with 3.5.3 and I get this error.
src/tss2-esys/esys_crypto_ossl.c: In function ‘iesys_cryptossl_hmac_start’:
src/tss2-esys/esys_crypto_ossl.c:400:18: error: implicit declaration of function ‘EVP_PKEY_new_raw_private_key’; did you mean ‘EC_KEY_set_private_key’? [-Werror=implicit-function-declaration]
400 | if (!(hkey = EVP_PKEY_new_raw_private_key(EVP_PKEY_HMAC, NULL, key, size))) {
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
| EC_KEY_set_private_key
src/tss2-esys/esys_crypto_ossl.c:400:16: error: assignment to ‘EVP_PKEY *’ {aka ‘struct evp_pkey_st *’} from ‘int’ makes pointer from integer without a cast [-Werror=int-conversion]
400 | if (!(hkey = EVP_PKEY_new_raw_private_key(EVP_PKEY_HMAC, NULL, key, size))) {
| ^
cc1: all warnings being treated as errors
Unfortunately libressl doesn't have EVP_PKEY_new_raw_private_key
yet.
I'm not sure this would make a difference for older libressl versions? They would presumably need the older openssl API too. Really 3.5 changed and fixed so many issues that its been difficult to worry about older versions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I'm starting to understand the implications of what they did and how there version number maps. What's the lowest known working version? The newest version of 3.5.3 but you're reporting it doesn't work now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With this PR, libressl-3.5.3 and presumably 3.5.2 the build succeeds. The first stable 3.5 release with all of the API changes was 3.5.2 while 3.5.0 was a developmental release and 3.5.1 was a security fix and 3.5.3 is a bug fix release.. However I am not sure when the last time tpm2-tss worked with older versions of libressl.
I started using Gentoo shortly before Gentoo and Alpine dropped libressl from the main repos which left the remaining users suddenly in the deep end. It was only after libressl-3.5.3 where I started going through the libressl overlay in more depth to update several neglected ebuilds when I discovered that tpm2-tss which did not build. I was able to drop the old libressl patches for tpm-tools and tpm2-tools at least. :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok I just merged tpm2-software/tpm2-software-container#71, so in a little bit we should have a working docker container with libressl 3.5.3 in it. Ill send you a diff to add testing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@orbea I think when the image is published, add this to your PR and re-upload, this way we can test:
diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index 08dba6857f9b..2e38162b02fd 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -85,6 +85,27 @@ jobs:
- name: failure
if: ${{ failure() }}
run: cat $(find ../ -name test-suite.log) || true
+ test-libressl:
+ runs-on: ubuntu-latest
+ if: "!contains(github.ref, 'coverity_scan')"
+ strategy:
+ matrix:
+ docker_image: [fedora-34-libressl]
+ steps:
+ - name: Check out repository
+ uses: actions/checkout@v2
+ with:
+ fetch-depth: 0
+ - name: Launch Action
+ uses:
+ tpm2-software/ci/runCI@main
+ with:
+ CC: gcc
+ DOCKER_IMAGE: ${{ matrix.docker_image }}
+ PROJECT_NAME: ${{ github.event.repository.name }}
+ - name: failure
+ if: ${{ failure() }}
+ run: cat $(find ../ -name test-suite.log) || true
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! I added this to the PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unfortunately github container registry that hosts are images is having some issues, but we will keep working through them and I will re-trigger the CI when the image is hosted.
So you now have a docker container you can test in, but it looks like some type of memory issue as it's segfaulting: You can test in that docker container by: docker pull ghcr.io/tpm2-software/fedora-34-libressl
docker run -it fedora-34-libressl
# run commands here You can kick off the whole CI test by: docker run --rm --env-file /path/to/tpm2-tss/.ci/docker.env -v /path/to/tpm2-tss:/workspace/tpm2-tss <IMAGE ID> /bin/bash -c '/workspace/tpm2-tss/.ci/docker.run' |
I can reproduce failing tests locally, but only when using
|
Yeah that config seems to be used in the test harness on the CI |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to find the bug in the docker container run...
I've been meaning to find time to work on trying to debug this, but real life took over. |
I rebased the PR removing the check for I have also found out why the tests are failing, libressl doesn't have For example in
|
Upstream-PR: tpm2-software/tpm2-tss#2380 Signed-off-by: orbea <[email protected]>
you could use the configure option |
Actually I didn't notice earlier that starting with
Looking at the log it shows they also use I confirmed that this started in commit 37bb561. |
There is already a fix that is currently in pr jam: #2585 |
@orbea since this is a draft pr you could temporally cherry pick the fix to see whether all tests in the ci are ok. |
420a9d8
to
e15a7cb
Compare
CI fails on a new test.
Perhaps its a problem exposed by the build config or maybe by using a now older LibreSSL version. I currently have |
The seems to be a problem with curl which uses openssl and libasan (docker image from github):
also without --enable-code-coverage the error occurs:
|
As pointed out in my LibreSSL issue (libressl/portable#840 (comment))
Probably the best solution is to just hide it with LibreSSL or if possible entirely replace or remove it. For the tests I will have to try this with asan and see if I can reproduce any of these failures. |
I am having trouble running the tests with asan where they all fail with.
However this seems restricted to tpm2-tss where asan does work in other projects and example programs I have tried. |
… is available. If the configure option --enable-self-generated-certificate is not used this test can't be executed because no certificate will be stored in NV ram. The test will be skipped if no certificate is available. Fixes: tpm2-software#2558 Signed-off-by: Juergen Repp <[email protected]>
This works with LibreSSL 3.5.x. Missing in LibreSSL: * RAND_OpenSSL (Deprecated in OpenSSL >= 3.0) * NID_sm2 Signed-off-by: orbea <[email protected]>
See if the CI passes.
Codecov Report
@@ Coverage Diff @@
## master #2380 +/- ##
==========================================
- Coverage 83.61% 82.58% -1.03%
==========================================
Files 363 363
Lines 41686 41682 -4
==========================================
- Hits 34855 34423 -432
- Misses 6831 7259 +428
... and 5 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
This works with LibreSSL 3.5.x.