Closed datosh closed 1 year ago
Sounds good to me. Happy to review.
On Thu, Aug 3, 2023 at 09:48 Fabian Kammel @.***> wrote:
Hey 👋
I was trying to use 0.7.0 in order to verify an SNP attestation report on AWS. I know now that VLEK is used instead of VCEK and found #67 https://github.com/google/go-sev-guest/pull/67. I will comment there as well once I played around with it :)
While using 0.7.0 I got a somewhat confusing error. The error I got was timeout while downloading the VCEK. So I wasted quite a bit of time debugging (non existing network issues). What was actually happening is that the library was trying to fetch https://kdsintf.amd.com/vcek/v1/Milan/00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000?blSPL=3&teeSPL=0&snpSPL=10&ucodeSPL=207 and AMD correctly responded with 404. Currently everything >300 is handled the same https://github.com/google/go-sev-guest/blob/main/verify/trust/trust.go#L105. I think we could narrow down the status code a bit and / or inform users about the underlying error while retrying.
AMD seems to honor the status codes documented in Chapter 4.1 https://www.amd.com/system/files/TechDocs/57230.pdf. So we could test for HTTP 429 and error early otherwise?
Let me know what you think. I am open to put in a PR as well.
— Reply to this email directly, view it on GitHub https://github.com/google/go-sev-guest/issues/69, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAFL4HFG5I2VX47GDJRVJOTXTPI4VANCNFSM6AAAAAA3DBUQ2E . You are receiving this because you are subscribed to this thread.Message ID: @.***>
Hey :wave:
I was trying to use 0.7.0 in order to verify an SNP attestation report on AWS. I know now that VLEK is used instead of VCEK and found https://github.com/google/go-sev-guest/pull/67. I will comment there as well once I played around with it :)
While using 0.7.0 I got a somewhat confusing error. The error I got was
timeout
while downloading the VCEK. So I wasted quite a bit of time debugging (non existing network issues). What was actually happening is that the library was trying to fetchhttps://kdsintf.amd.com/vcek/v1/Milan/00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000?blSPL=3&teeSPL=0&snpSPL=10&ucodeSPL=207
and AMD correctly responded with 404. Currently everything >300 is handled the same. I think we could narrow down the status code a bit and / or inform users about the underlying error while retrying.AMD seems to honor the status codes documented in Chapter 4.1. So we could test for HTTP 429 and error early otherwise?
Let me know what you think. I am open to put in a PR as well.