Skip to content
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

test failures in Debian: TestSignature_Sign_Internal and TestSignature_toBeSigned #181

Closed
jas4711 opened this issue Jan 15, 2024 · 12 comments
Labels
question Further information is requested
Milestone

Comments

@jas4711
Copy link

jas4711 commented Jan 15, 2024

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

@jas4711 jas4711 added the bug Something isn't working label Jan 15, 2024
@thomas-fossati
Copy link
Contributor

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
Copy link
Author

jas4711 commented Jan 15, 2024

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
Copy link
Author

jas4711 commented Jan 15, 2024

ping @gibmat - see also fxamacker/cbor#302 for some discussion

@gibmat
Copy link

gibmat commented Jan 16, 2024

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.

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
Copy link
Contributor

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:

  • fxamacker/cbor v2.4.0 + @gibmat's patch = fail
  • fxamacker/cbor v2.5.0 + @gibmat's patch = pass

Is that correct?

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

@thomas-fossati
Copy link
Contributor

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
Copy link

gibmat commented Jan 24, 2024

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
Copy link
Contributor

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
Copy link
Contributor

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.

@SteveLasker SteveLasker added question Further information is requested pending-close Pending closure with no recent activity and removed bug Something isn't working labels Feb 23, 2024
@gibmat
Copy link

gibmat commented Mar 1, 2024

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
Copy link
Contributor

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

@SteveLasker SteveLasker removed the pending-close Pending closure with no recent activity label Mar 8, 2024
@SteveLasker SteveLasker added this to the v1.2.0 milestone Mar 8, 2024
@SteveLasker SteveLasker modified the milestones: v1.2.0, v1.4.0 Jul 25, 2024
@SteveLasker
Copy link
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

4 participants