search

veraison / go-cose

go library for CBOR Object Signing and Encryption (COSE)
Mozilla Public License 2.0
49 stars 26 forks source link

test failures in Debian: TestSignature_Sign_Internal and TestSignature_toBeSigned #181

Closed jas4711 closed 3 weeks ago

jas4711 commented 7 months ago

What is the areas you experience the issue in?

Go-COSE Library

What is not working as expected?

Hi! I am packaging go-cose for Debian. I got build failures in the self tests when built inside Debian. See build log here:

https://salsa.debian.org/jas/golang-github-veraison-go-cose/-/jobs/5160101

Output below. Do you have any ideas? The self test seems extensive, great work! We are disabling it until we can figure out how to fix this.

=== RUN   TestSignature_Sign_Internal/valid_message
    sign_test.go:769: Signature.Sign() error = cbor: 3 bytes of extraneous data starting at index 1
=== RUN   TestSignature_Sign_Internal/valid_message_with_empty_parent_protected_header
=== RUN   TestSignature_Sign_Internal/valid_message_with_external
=== RUN   TestSignature_Sign_Internal/nil_external
=== RUN   TestSignature_Sign_Internal/nil_protected_header
--- FAIL: TestSignature_Sign_Internal (0.00s)
    --- FAIL: TestSignature_Sign_Internal/valid_message (0.00s)
...
=== RUN   TestSignature_toBeSigned/valid_signature
    sign_test.go:2289: Signature.toBeSigned() error = cbor: 3 bytes of extraneous data starting at index 1, wantErr false
=== RUN   TestSignature_toBeSigned/invalid_body_protected_header
=== RUN   TestSignature_toBeSigned/invalid_sign_protected_header
=== RUN   TestSignature_toBeSigned/invalid_raw_sign_protected_header
--- FAIL: TestSignature_toBeSigned (0.00s)
    --- FAIL: TestSignature_toBeSigned/valid_signature (0.00s)
    --- PASS: TestSignature_toBeSigned/invalid_body_protected_header (0.00s)

What did you expect to happen?

Build work

How can we reproduce it?

Checkout and build your package via Debian:

https://salsa.debian.org/go-team/packages/golang-github-veraison-go-cose/

Describe your environment

Debian

What is the version of your Go-COSE Library?

1.2.1

thomas-fossati commented 7 months ago

Hi @jas4711, thanks for the report!

Unfortunately, I am unable to reproduce the bug in my setup.

I cloned the repo and ran:

go test -vet=off -v -p 2 github.com/veraison/go-cose

and everything seems to work ok, at least on my laptop.

Could you help me set up the repro environment?

The error you are seeing looks either like a truncation in the CBOR stream, or spurious data that got appended for TBD reasons.

jas4711 commented 7 months ago

Thanks for reply! Reproducing it involves building our Debian packaging which is a somewhat complicated process if you haven't done it before. Maybe we can resolve it with a bit of detective work instead.

Are these self-tests related to the golang-github-fxamacker-cbor-dev aka github.com/fxamacker/cbor/v2 package maybe? Debian has version 2.5.0 which seems to be the same that you use too. The other dependency is golang-github-x448-float16-dev aka github.com/x448/float16 and Debian has 0.8.4 which seems to be the same version you have. The error mention cbor though... looking at Debian's cbor package, it ships with this patch:

https://salsa.debian.org/go-team/packages/golang-github-fxamacker-cbor/-/blob/debian/sid/debian/patches/fix-tests-on-32bit.patch?ref_type=heads

Could that patch result in this self-test error? I'm building on amd64 platform, so the patch shouldn't be necessary and may be causing the problem. I'll try to rebuild the cbor dependency without the patch to see if it makes a difference for your self-tests.

jas4711 commented 7 months ago

ping @gibmat - see also https://github.com/fxamacker/cbor/issues/302 for some discussion

gibmat commented 7 months ago

Debian packaging typically tries to stick to released versions, and version 1.2.1 expects fxamacker/cbor v2.4.0. Since that release, https://github.com/veraison/go-cose/pull/166 was merged into main, with changes for the fxamacker/cbor v2.5.0 release.

When I cherry-pick that PR into the debian packaging all the tests pass.

(As a side note, I'd really like to see fxamacker/cbor merge my PR, but non-64bit systems don't seem to concern the maintainer.)

thomas-fossati commented 7 months ago

Thanks @gibmat for looking at it.

Debian packaging typically tries to stick to released versions, and version 1.2.1 expects fxamacker/cbor v2.4.0. Since that release, #166 was merged into main, with changes for the fxamacker/cbor v2.5.0 release.

BTW, go-cose@latest is v1.1.0

When I cherry-pick that PR into the debian packaging all the tests pass.

Interesting. OK, so to try and recap go-cose's situation:

Is that correct?

Have you tried removing @gibmat's patch to see whether v2.4.0 of fxamacker/cbor is still broken?

thomas-fossati commented 7 months ago

Could that patch result in this self-test error? I'm building on amd64 platform, so the patch shouldn't be necessary and may be causing the problem.

That's what I suspect.

I'll try to rebuild the cbor dependency without the patch to see if it makes a difference for your self-tests.

Awesome. Looking forward to your results.

gibmat commented 7 months ago

BTW, go-cose@latest is v1.1.0

The latest tag of go-cose is v1.2.1 (https://github.com/veraison/go-cose/tags), which is what the Debian packaging tools are picking up on.

Interesting. OK, so to try and recap go-cose's situation:

* fxamacker/cbor v2.4.0 + @gibmat's patch = fail

* fxamacker/cbor v2.5.0 + @gibmat's patch = pass

Is that correct?

No, I think it's just API changes in fxamacker/cbor v2.5.0, which go-cose addressed in #166 but haven't yet been in a tagged version of go-cose.

thomas-fossati commented 7 months ago

BTW, go-cose@latest is v1.1.0

The latest tag of go-cose is v1.2.1 (https://github.com/veraison/go-cose/tags), which is what the Debian packaging tools are picking up on.

You should use v1.1.0 which is advertised as the current release.

SteveLasker commented 6 months ago

Hi @gibmat, thanks for the issue. It looks like this is resolved. Can you please confirm? We'll close in a week if there's no response.

gibmat commented 6 months ago

It's resolved in Debian... by cherry-picking #166. I don't see any new tag/release that includes that PR.

I don't directly use this library, so I'm not particularly invested in if bug is closed or not; I'd defer to @jas4711 on that.

SteveLasker commented 5 months ago

This issue will be resolved after the next minor version of the release.

SteveLasker commented 3 weeks ago

Closing with v1.4.0 released. Please reactivate if you're still seeing issues.