Skip to content

Add SHA according to FIPS 180-4#2

Merged
kleinreact merged 9 commits into
mainfrom
add-sha
Oct 29, 2024
Merged

Add SHA according to FIPS 180-4#2
kleinreact merged 9 commits into
mainfrom
add-sha

Conversation

@kleinreact
Copy link
Copy Markdown
Member

@kleinreact kleinreact commented Oct 23, 2024

This PR adds

  • a specification of the FIPS PUB 180-4: Secure Hash Standard (SHS) that has been formalized using a purely functional description in Haskell,
  • streaming based hardware implementations for all of the hash algorithms of FIPS 180-4 utilizing a request-response interface, and
  • some sanity checks for testing the functional correctness of the specification and the implementations, where cryptohash is used as an independent reference implementation.

Synthesis of the design requires some fixes and workarounds that are not available upstream yet.

where we use this dedicated branch to utilize the available fixes at the moment. Nevertheless, further errors still might to be expected for synthesizing the design.

I propose to already merge this PR, as clash-crypto still is in a preliminary development state anyway and to keep track of the missing compiler support via issues separately.

@kleinreact kleinreact linked an issue Oct 23, 2024 that may be closed by this pull request
@kleinreact kleinreact marked this pull request as ready for review October 23, 2024 10:10
@kleinreact kleinreact requested a review from diegodiv October 23, 2024 10:10
Copy link
Copy Markdown
Contributor

@diegodiv diegodiv left a comment

Choose a reason for hiding this comment

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

Very nice work! I made some suggestions, most of them we already talked about. The last fix seems to make it synthesizable, which is super nice. I think that's very good for a first revision, and we'll improve on the design later anyway.

Comment thread src/Data/Constraint/Nat/Extra.hs Outdated
Comment thread src/Clash/Crypto/Hash/SHA/Streaming/Padding.hs Outdated
Comment thread src/Data/Constraint/Nat/Extra.hs Outdated
Comment thread src/Clash/Crypto/Hash/SHA/Specification/Types.hs
Comment thread src/Clash/Crypto/Hash/SHA/Specification/Types.hs
Comment thread src/Clash/Signal/Delayed/Extra.hs Outdated
Comment thread src/Clash/Crypto/Hash/SHA/Streaming/Stages.hs
Comment thread src/Clash/Crypto/Hash/SHA/Specification/Definitions.hs
Comment thread src/Clash/Crypto/Hash/SHA/Streaming.hs
(KnownDomain dom, HiddenClockResetEnable dom) ⇒
DSignal dom k b →
DSignal dom k a →
DSignal dom (k + r) a
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.

I don't find this signature particularly readable: maybe we could have a type embodying the function type returned here, like

type Distributer a b d =   (dom  Domain) (k  Nat).
  (KnownDomain dom, HiddenClockResetEnable dom) 
  DSignal dom k b 
  DSignal dom k a 
  DSignal dom (k + d) a

and then use it as the return type of the functions (both the inside one and the top-level one).

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I don't like the idea of introducing an additional type alias here, as it does not work well with haddock in terms of documentation and it wouldn't have any particular meaning, except that it reduces redundancy.

The Haskell compiler also will take care that both signatures are kept in sync.

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.

True, but that's mostly about readability - I'd be inclined to think that knowing at first sight what it does by looking at the signature is quite important, and I find it a bit difficult to read. Maybe just having a layout-based solution would be enough.

@kleinreact kleinreact requested a review from diegodiv October 29, 2024 11:15
Copy link
Copy Markdown
Contributor

@diegodiv diegodiv left a comment

Choose a reason for hiding this comment

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

LGTM, very nice job. I didn't opt for very thorough testing as the goal is to test it by other means and find issues, so the current state it's in is fine by me!

For the other remarks, we can discuss them later if needed, but we can move them out of the scope of this PR.

-- | @Just . Right@ encodes a data frame.
pattern Data ∷ BitVector n → PaddedMsgFrame n
pattern Data f = Just (Right f)

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.

Nice!

@kleinreact kleinreact merged commit c348085 into main Oct 29, 2024
@kleinreact kleinreact deleted the add-sha branch October 29, 2024 13:47
kleinreact added a commit that referenced this pull request Nov 8, 2024
Adds
* a specification of the FIPS PUB 180-4: Secure Hash Standard (SHS) that has been formalized using a purely functional description in Haskell,
* streaming based hardware implementations for all of the hash algorithms of FIPS 180-4 utilizing a request-response interface, and
* some sanity checks for testing the functional correctness of the specification and the implementations, where cryptohash is used as an independent reference implementation.
@kleinreact kleinreact mentioned this pull request Jun 11, 2025
@enathang enathang mentioned this pull request Jun 26, 2025
kleinreact added a commit to QBayLogic/clash-crypto-fork that referenced this pull request Jun 1, 2026
kleinreact added a commit to QBayLogic/clash-crypto-fork that referenced this pull request Jun 1, 2026
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.

Implement SHA-256 in hardware

2 participants