Skip to content

fix: deterministic encode cose_sign / cose_sign1 sig_signature#135

Merged
SteveLasker merged 6 commits intoveraison:mainfrom
shizhMSFT:fix_119
Mar 10, 2023
Merged

fix: deterministic encode cose_sign / cose_sign1 sig_signature#135
SteveLasker merged 6 commits intoveraison:mainfrom
shizhMSFT:fix_119

Conversation

@shizhMSFT
Copy link
Copy Markdown
Contributor

fix #119

Signed-off-by: Shiwei Zhang <shizh@microsoft.com>
Signed-off-by: Shiwei Zhang <shizh@microsoft.com>
@shizhMSFT shizhMSFT requested a review from a team February 20, 2023 12:39
@shizhMSFT shizhMSFT added this to the v1.1.0 milestone Feb 20, 2023
@codecov
Copy link
Copy Markdown

codecov bot commented Feb 20, 2023

Codecov Report

Merging #135 (ad6c208) into main (c6971fb) will increase coverage by 0.35%.
The diff coverage is 93.61%.

@@            Coverage Diff             @@
##             main     #135      +/-   ##
==========================================
+ Coverage   92.12%   92.48%   +0.35%     
==========================================
  Files          10       10              
  Lines         978     1024      +46     
==========================================
+ Hits          901      947      +46     
- Misses         51       52       +1     
+ Partials       26       25       -1     
Impacted Files Coverage Δ
cbor.go 87.50% <91.17%> (+3.28%) ⬆️
sign.go 90.47% <100.00%> (+1.54%) ⬆️
sign1.go 89.36% <100.00%> (+2.50%) ⬆️
headers.go 92.14% <0.00%> (-0.91%) ⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Signed-off-by: Shiwei Zhang <shizh@microsoft.com>
@shizhMSFT shizhMSFT marked this pull request as ready for review February 20, 2023 15:19
Signed-off-by: Shiwei Zhang <shizh@microsoft.com>
Signed-off-by: Shiwei Zhang <shizh@microsoft.com>
Signed-off-by: Shiwei Zhang <shizh@microsoft.com>
Comment on lines +118 to +121
case 27:
if data[1] != 0 || data[2] != 0 || data[3] != 0 || data[4] != 0 {
return data, nil
}
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This path cannot be fully tested since it requires more than 4 GiB memory to test with, which is not allowed by GitHub Action.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Captured the issue in #139

Copy link
Copy Markdown
Contributor

@thomas-fossati thomas-fossati left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

Copy link
Copy Markdown
Contributor

@yogeshbdeshpande yogeshbdeshpande left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@SteveLasker SteveLasker merged commit 556edd4 into veraison:main Mar 10, 2023
@shizhMSFT shizhMSFT deleted the fix_119 branch March 14, 2023 08:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Sign1 messages with a non-minimal protected header length cannot be verified

5 participants