Skip to content

feat: Make SDK FIPS-compliant by using internal SHA1 module#2179

Merged
kodiakhq[bot] merged 3 commits intomainfrom
feat/fix-fips-sha1
May 30, 2025
Merged

feat: Make SDK FIPS-compliant by using internal SHA1 module#2179
kodiakhq[bot] merged 3 commits intomainfrom
feat/fix-fips-sha1

Conversation

@maaarcelino
Copy link
Contributor

The Go standard library has removed support for using SHA1 when in FIPS mode in golang/go@54693a8. However the NIST FIPS spec allows for using SHA1 in certain cases:

SHA-1 for non-digital signature applications:
For all other hash function applications, the use of SHA-1 is acceptable. The other
applications include HMAC, Key Derivation Functions (KDFs), Random Bit Generation, and
hash-only applications (e.g., hashing passwords and using SHA-1 to compute a checksum,
such as the approved integrity technique specified in Section 4.6.1 of [FIPS 140]).

The above removal from the standard lib is too restrictive, as we should be able to use this algo in our SDK for checksums. I've borrowed the implementation of packages sha1 and byteorder and added them as internal modules.

@maaarcelino maaarcelino requested review from a team and blesniewski May 30, 2025 09:08
@github-actions github-actions bot added the feat label May 30, 2025
require.NoError(t, err)
cqIDFields := got.Schema().FieldIndices(CqIDColumn.Name)
require.Len(t, cqIDFields, 1)
assert.Equal(t, "d8f3b1de-8c63-5a0e-a1aa-19e9b5311c24", got.Column(cqIDFields[0]).ValueStr(0))
Copy link
Member

Choose a reason for hiding this comment

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

Was d8f3b1de-8c63-5a0e-a1aa-19e9b5311c24 calculated using the official Sha1 package? Maybe we can have a test that creates a UUID using our internal package, then using the official one, then compares the values are the same?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes d8f3b1de-8c63-5a0e-a1aa-19e9b5311c24 is the value from the official package. This is the same value that is on main currently. This test does what you are describing, but without using the official package to calculate the hash. Instead it is just hardcoded as a string. Does that work, wdyt?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah it works but hard to reason about when reading the test (and also hard to update it), maybe add a comment on how that value was created?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@kodiakhq kodiakhq bot merged commit 5a34e35 into main May 30, 2025
10 checks passed
@kodiakhq kodiakhq bot deleted the feat/fix-fips-sha1 branch May 30, 2025 10:34
kodiakhq bot pushed a commit that referenced this pull request May 30, 2025
🤖 I have created a release *beep* *boop*
---


## [4.84.0](v4.83.0...v4.84.0) (2025-05-30)


### Features

* Make SDK FIPS-compliant by using internal SHA1 module ([#2179](#2179)) ([5a34e35](5a34e35))

---
This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants