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

Fix for truncated bits after casting logical shifts to the incorrect width for lint fixes #524

Merged
merged 4 commits into from
May 30, 2024

Conversation

Nitsirks
Copy link
Contributor

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.

Michael Norris and others added 3 commits May 28, 2024 08:10
…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
@Nitsirks Nitsirks self-assigned this May 28, 2024
…' with updated timestamp and hash after successful run
@Nitsirks
Copy link
Contributor Author

Nitsirks commented May 29, 2024

Fixes issue 522

@calebofearth calebofearth merged commit 46fdcd8 into main May 30, 2024
56 checks passed
@calebofearth calebofearth deleted the user/dev/michnorris/lint_bug_fix branch May 30, 2024 17:07
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>
@calebofearth calebofearth mentioned this pull request Jun 26, 2024
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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants