-
Notifications
You must be signed in to change notification settings - Fork 41
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
Fix for truncated bits after casting logical shifts to the incorrect width for lint fixes #524
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ong width and bits are truncated fixed by removing the shifts and explicitly taking the bits required
…' with updated timestamp and hash after successful run
…' with updated timestamp and hash after successful run
calebofearth
approved these changes
May 28, 2024
Fixes issue 522 |
upadhyayulakiran
approved these changes
May 29, 2024
calebofearth
added a commit
that referenced
this pull request
May 30, 2024
…width for lint fixes (#524) * fixing bugs caused during lint fixes where shifts were cast as the wrong width and bits are truncated fixed by removing the shifts and explicitly taking the bits required * MICROSOFT AUTOMATED PIPELINE: Stamp 'user/dev/michnorris/lint_bug_fix' with updated timestamp and hash after successful run * updating smoke test to sha the entire mailbox at the start * MICROSOFT AUTOMATED PIPELINE: Stamp 'user/dev/michnorris/lint_bug_fix' with updated timestamp and hash after successful run --------- Co-authored-by: Michael Norris <michnorris@fe74.svceng.com>
Nitsirks
pushed a commit
that referenced
this pull request
Jun 6, 2024
…width for lint fixes (#524) * fixing bugs caused during lint fixes where shifts were cast as the wrong width and bits are truncated fixed by removing the shifts and explicitly taking the bits required * MICROSOFT AUTOMATED PIPELINE: Stamp 'user/dev/michnorris/lint_bug_fix' with updated timestamp and hash after successful run * updating smoke test to sha the entire mailbox at the start * MICROSOFT AUTOMATED PIPELINE: Stamp 'user/dev/michnorris/lint_bug_fix' with updated timestamp and hash after successful run --------- Co-authored-by: Michael Norris <michnorris@fe74.svceng.com>
calebofearth
added a commit
that referenced
this pull request
Jun 14, 2024
* patch for kv exfiltration locking api registers from being modified by uc when data is loaded from the keyvault updating smoke tests to attempt to corrupt the kv data to test the lock * updating kv smoke test to use keyvault for block register * adding multi block hmac keyvault test content * updating keyvault section of the hardware spec to explicitly call out the key locking/clearing inside the crypto function Also detailing the requirement that each iteration of a multi block operation must program the keyvault read/write operation * corrected the expected tag to match the expected output of the hmac block * adding multiblock test to l0 and nightly directed regressions * preventing commands from being issued while key is being copied to the crypto engine * changing the masking to just cover the idle case, no need to check for data present * added busy signal to crypto engines with key access multiple busy signals trigger a fatal error zeroize keyvault reads when read has an error updated ras test to include testing crypto error case * adding new port for busy signals and crypto errors to all the unit level testbenches * fixing jtag aperture to allow access to veer jtag registers only when debug is unlocked jtag path to soc ifc registers is unchanged * MICROSOFT AUTOMATED PIPELINE: Stamp 'user/dev/michnorris/kv_vuln_fix' with updated timestamp and hash after successful run * Fix for truncated bits after casting logical shifts to the incorrect width for lint fixes (#524) * fixing bugs caused during lint fixes where shifts were cast as the wrong width and bits are truncated fixed by removing the shifts and explicitly taking the bits required * MICROSOFT AUTOMATED PIPELINE: Stamp 'user/dev/michnorris/lint_bug_fix' with updated timestamp and hash after successful run * updating smoke test to sha the entire mailbox at the start * MICROSOFT AUTOMATED PIPELINE: Stamp 'user/dev/michnorris/lint_bug_fix' with updated timestamp and hash after successful run --------- Co-authored-by: Michael Norris <michnorris@fe74.svceng.com> * [ENV] Disable wget HSTS in ROM test (#527) * Disable HSTS (https is hardcoded in makefile, no MIM attack here) * MICROSOFT AUTOMATED PIPELINE: Stamp 'cwhitehead-msft-rom-wget-hsts-disable' with updated timestamp and hash after successful run * MICROSOFT AUTOMATED PIPELINE: Stamp 'user/dev/michnorris/kv_vuln_fix' with updated timestamp and hash after successful run * updating hardware spec for crypto error fatal error fixing some typos and doc nits updating covergroups to include new crypto error fatal error bit * MICROSOFT AUTOMATED PIPELINE: Stamp 'user/dev/michnorris/kv_vuln_fix' with updated timestamp and hash after successful run * Apply suggested feedback * MICROSOFT AUTOMATED PIPELINE: Stamp 'user/dev/michnorris/kv_vuln_fix' with updated timestamp and hash after successful run * Removed multiple write scenarios in kv * MICROSOFT AUTOMATED PIPELINE: Stamp 'user/dev/michnorris/kv_vuln_fix' with updated timestamp and hash after successful run * updating register description for internal fw update reset wait cycle count to indicate that 5 is the minimum value allowed updating kv definition description to clarify that SHA is no longer a valid destination * MICROSOFT AUTOMATED PIPELINE: Stamp 'user/dev/michnorris/kv_vuln_fix' with updated timestamp and hash after successful run * MICROSOFT AUTOMATED PIPELINE: Stamp 'user/dev/michnorris/kv_vuln_fix' with updated timestamp and hash after successful run --------- Co-authored-by: Michael Norris <michnorris@fe74.svceng.com> Co-authored-by: Michael Norris <michnorris@gem-cl04-fe-228.svceng.com> Co-authored-by: Michael Norris <michnorris@gem-cl02-fe-373.svceng.com> Co-authored-by: Caleb <11879229+calebofearth@users.noreply.github.com> Co-authored-by: Caleb Whitehead <cwhitehead@microsoft.com> Co-authored-by: Kiran Upadhyayula <kupadhyayula@fe716.svceng.com>
Nitsirks
added a commit
that referenced
this pull request
Jun 14, 2024
* patch for kv exfiltration locking api registers from being modified by uc when data is loaded from the keyvault updating smoke tests to attempt to corrupt the kv data to test the lock * updating kv smoke test to use keyvault for block register * adding multi block hmac keyvault test content * updating keyvault section of the hardware spec to explicitly call out the key locking/clearing inside the crypto function Also detailing the requirement that each iteration of a multi block operation must program the keyvault read/write operation * corrected the expected tag to match the expected output of the hmac block * adding multiblock test to l0 and nightly directed regressions * preventing commands from being issued while key is being copied to the crypto engine * changing the masking to just cover the idle case, no need to check for data present * added busy signal to crypto engines with key access multiple busy signals trigger a fatal error zeroize keyvault reads when read has an error updated ras test to include testing crypto error case * adding new port for busy signals and crypto errors to all the unit level testbenches * fixing jtag aperture to allow access to veer jtag registers only when debug is unlocked jtag path to soc ifc registers is unchanged * MICROSOFT AUTOMATED PIPELINE: Stamp 'user/dev/michnorris/kv_vuln_fix' with updated timestamp and hash after successful run * Fix for truncated bits after casting logical shifts to the incorrect width for lint fixes (#524) * fixing bugs caused during lint fixes where shifts were cast as the wrong width and bits are truncated fixed by removing the shifts and explicitly taking the bits required * MICROSOFT AUTOMATED PIPELINE: Stamp 'user/dev/michnorris/lint_bug_fix' with updated timestamp and hash after successful run * updating smoke test to sha the entire mailbox at the start * MICROSOFT AUTOMATED PIPELINE: Stamp 'user/dev/michnorris/lint_bug_fix' with updated timestamp and hash after successful run --------- Co-authored-by: Michael Norris <michnorris@fe74.svceng.com> * [ENV] Disable wget HSTS in ROM test (#527) * Disable HSTS (https is hardcoded in makefile, no MIM attack here) * MICROSOFT AUTOMATED PIPELINE: Stamp 'cwhitehead-msft-rom-wget-hsts-disable' with updated timestamp and hash after successful run * MICROSOFT AUTOMATED PIPELINE: Stamp 'user/dev/michnorris/kv_vuln_fix' with updated timestamp and hash after successful run * updating hardware spec for crypto error fatal error fixing some typos and doc nits updating covergroups to include new crypto error fatal error bit * MICROSOFT AUTOMATED PIPELINE: Stamp 'user/dev/michnorris/kv_vuln_fix' with updated timestamp and hash after successful run * Apply suggested feedback * MICROSOFT AUTOMATED PIPELINE: Stamp 'user/dev/michnorris/kv_vuln_fix' with updated timestamp and hash after successful run * Removed multiple write scenarios in kv * MICROSOFT AUTOMATED PIPELINE: Stamp 'user/dev/michnorris/kv_vuln_fix' with updated timestamp and hash after successful run * updating register description for internal fw update reset wait cycle count to indicate that 5 is the minimum value allowed updating kv definition description to clarify that SHA is no longer a valid destination * MICROSOFT AUTOMATED PIPELINE: Stamp 'user/dev/michnorris/kv_vuln_fix' with updated timestamp and hash after successful run * MICROSOFT AUTOMATED PIPELINE: Stamp 'user/dev/michnorris/kv_vuln_fix' with updated timestamp and hash after successful run --------- Co-authored-by: Michael Norris <michnorris@fe74.svceng.com> Co-authored-by: Michael Norris <michnorris@gem-cl04-fe-228.svceng.com> Co-authored-by: Michael Norris <michnorris@gem-cl02-fe-373.svceng.com> Co-authored-by: Caleb <11879229+calebofearth@users.noreply.github.com> Co-authored-by: Caleb Whitehead <cwhitehead@microsoft.com> Co-authored-by: Kiran Upadhyayula <kupadhyayula@fe716.svceng.com>
39 tasks
calebofearth
added a commit
that referenced
this pull request
Jun 28, 2024
* patch for kv exfiltration locking api registers from being modified by uc when data is loaded from the keyvault updating smoke tests to attempt to corrupt the kv data to test the lock * updating kv smoke test to use keyvault for block register * adding multi block hmac keyvault test content * updating keyvault section of the hardware spec to explicitly call out the key locking/clearing inside the crypto function Also detailing the requirement that each iteration of a multi block operation must program the keyvault read/write operation * corrected the expected tag to match the expected output of the hmac block * adding multiblock test to l0 and nightly directed regressions * preventing commands from being issued while key is being copied to the crypto engine * changing the masking to just cover the idle case, no need to check for data present * added busy signal to crypto engines with key access multiple busy signals trigger a fatal error zeroize keyvault reads when read has an error updated ras test to include testing crypto error case * adding new port for busy signals and crypto errors to all the unit level testbenches * fixing jtag aperture to allow access to veer jtag registers only when debug is unlocked jtag path to soc ifc registers is unchanged * MICROSOFT AUTOMATED PIPELINE: Stamp 'user/dev/michnorris/kv_vuln_fix' with updated timestamp and hash after successful run * Fix for truncated bits after casting logical shifts to the incorrect width for lint fixes (#524) * fixing bugs caused during lint fixes where shifts were cast as the wrong width and bits are truncated fixed by removing the shifts and explicitly taking the bits required * MICROSOFT AUTOMATED PIPELINE: Stamp 'user/dev/michnorris/lint_bug_fix' with updated timestamp and hash after successful run * updating smoke test to sha the entire mailbox at the start * MICROSOFT AUTOMATED PIPELINE: Stamp 'user/dev/michnorris/lint_bug_fix' with updated timestamp and hash after successful run --------- Co-authored-by: Michael Norris <michnorris@fe74.svceng.com> * [ENV] Disable wget HSTS in ROM test (#527) * Disable HSTS (https is hardcoded in makefile, no MIM attack here) * MICROSOFT AUTOMATED PIPELINE: Stamp 'cwhitehead-msft-rom-wget-hsts-disable' with updated timestamp and hash after successful run * MICROSOFT AUTOMATED PIPELINE: Stamp 'user/dev/michnorris/kv_vuln_fix' with updated timestamp and hash after successful run * updating hardware spec for crypto error fatal error fixing some typos and doc nits updating covergroups to include new crypto error fatal error bit * MICROSOFT AUTOMATED PIPELINE: Stamp 'user/dev/michnorris/kv_vuln_fix' with updated timestamp and hash after successful run * Apply suggested feedback * MICROSOFT AUTOMATED PIPELINE: Stamp 'user/dev/michnorris/kv_vuln_fix' with updated timestamp and hash after successful run * Removed multiple write scenarios in kv * MICROSOFT AUTOMATED PIPELINE: Stamp 'user/dev/michnorris/kv_vuln_fix' with updated timestamp and hash after successful run * updating register description for internal fw update reset wait cycle count to indicate that 5 is the minimum value allowed updating kv definition description to clarify that SHA is no longer a valid destination * MICROSOFT AUTOMATED PIPELINE: Stamp 'user/dev/michnorris/kv_vuln_fix' with updated timestamp and hash after successful run * MICROSOFT AUTOMATED PIPELINE: Stamp 'user/dev/michnorris/kv_vuln_fix' with updated timestamp and hash after successful run --------- Co-authored-by: Michael Norris <michnorris@fe74.svceng.com> Co-authored-by: Michael Norris <michnorris@gem-cl04-fe-228.svceng.com> Co-authored-by: Michael Norris <michnorris@gem-cl02-fe-373.svceng.com> Co-authored-by: Caleb <11879229+calebofearth@users.noreply.github.com> Co-authored-by: Caleb Whitehead <cwhitehead@microsoft.com> Co-authored-by: Kiran Upadhyayula <kupadhyayula@fe716.svceng.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Two logical shifts to align the mailbox data length in bytes to a dword address were cast to address widths to satisfy a lint violation. The cast truncates the MSB that is used for maximum mailbox dlen.
Fixed the mailbox case even though the overflow detection prevents the truncated bit from being functional.
The SHA accelerator cast prevent us from performing a SHA of the entire mailbox starting at address 0. Since we only lose the MSB with the DLEN is set to the maximum value, this is the only case that was impacted.
Added a test case to the sha accel smoke test to perform this 0->end sha.