Skip to content

Commit 8d9853e

Browse files
authored
Make ssl/crypto report the failures it was swallowing (#92)
The package had two conventions and no rule. RandBytes and Pbkdf2Sha256 checked what OpenSSL returned and raised. Digest and HmacSha256 checked nothing and handed back whatever was in the output buffer. HmacSha256 returned thirty-two zero bytes when HMAC failed. That is not a degenerate hash, it is a code an attacker can send: a verifier comparing a computed code against one it was given accepts when the computation failed. HMAC returns NULL when the key and the message pointers are both null, and an empty Array[U8] has a null pointer, so the code was already reachable with ordinary input. HmacSha256(empty, empty) returned zeros. Empty keys and empty messages are well defined, so both now get a pointer to bytes HMAC will not read, and a NULL return raises. Digest.final returned its @pony_alloc output buffer without looking at what EVP_DigestFinal_ex made of it. @pony_alloc does not zero, so a failed finalise handed back whatever this actor freed last, as a hash. It raises now, and the buffer is dropped. EVP_MD_CTX_new returns NULL when it cannot allocate and EVP_DigestInit_ex dereferences it. The constructors are partial and raise there, at the one point that knows whether OpenSSL gave us a context. Deferring it would let someone build an actor around a Digest that is doomed to raise on every final(). Six lengths were narrowed to a C int with no check. A length in [2^31, 2^32) narrows negative and OpenSSL rejects it. A length past 2^32 narrows to a small positive one: RandBytes(2^32 + 5) allocated four gigabytes, filled five bytes, was told it succeeded, and returned the lot. Each length is checked against what an int holds, before anything is allocated. Digest's constructors, Digest.final and HmacSha256 are partial now. append was already partial, so a streaming digest was always written inside a try. Closes #85 Closes #86 Closes #88
1 parent 11f7bac commit 8d9853e

9 files changed

Lines changed: 395 additions & 169 deletions

File tree

.release-notes/next-release.md

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,3 +134,45 @@ When a server selected an ALPN protocol, OpenSSL was left with a pointer to memo
134134

135135
One thing changes for you: if you set your own resolver with `SSLContext.alpn_set_resolver` and it returns a protocol the client did not offer, the handshake now fails instead of continuing. `ALPNStandardProtocolResolver` is unaffected.
136136

137+
138+
## Make Digest and HmacSha256 raise on failure
139+
140+
`Digest` and `HmacSha256` now raise when OpenSSL cannot do what you asked, where before they returned a wrong result with no error. Three calls gained a `?`: constructing a `Digest`, `Digest.final`, and `HmacSha256`. `Digest.append` was already partial.
141+
142+
```pony
143+
// Before
144+
let digest = Digest.sha256()
145+
digest.append(data)?
146+
let hash = digest.final()
147+
148+
let mac = HmacSha256(key, message)
149+
150+
// After
151+
let digest = Digest.sha256()?
152+
digest.append(data)?
153+
let hash = digest.final()?
154+
155+
let mac = HmacSha256(key, message)?
156+
```
157+
158+
Constructing a `Digest` now raises if OpenSSL cannot allocate its context, rather than returning a digest that fails on every later call. When `HmacSha256` raises, reject the message. Do not fall back to a code of your own — a code you make up is one an attacker can send you.
159+
160+
## Fix HmacSha256 returning an all-zero code when it fails
161+
162+
`HmacSha256` returned thirty-two zero bytes when it could not compute the code, instead of failing. Thirty-two zero bytes is a value an attacker can send, so a program that checks a message by comparing a fresh code against a supplied one would accept a forgery whenever its own computation failed.
163+
164+
It was reachable with ordinary input: computing the code of an empty key and an empty message returned all zeros. `HmacSha256` now returns the real code and raises when the computation actually fails.
165+
166+
## Fix Digest returning a wrong hash or crashing when OpenSSL fails
167+
168+
`Digest` could return a hash of the wrong bytes, or crash while being created, when an OpenSSL call inside it failed.
169+
170+
`final` returned a block of memory as the hash without checking that OpenSSL had written it, so a failed call returned whatever that memory held — a wrong hash, and a leak of whatever was last in it. Creating a digest crashed when OpenSSL could not allocate its working context.
171+
172+
A digest now raises rather than returning a wrong hash, and one that could not be created reports it at the first `append` or `final` rather than crashing.
173+
174+
## Fix crypto functions truncating a length too large for an int
175+
176+
`RandBytes`, `HmacSha256` and `Pbkdf2Sha256` silently truncated a length that did not fit the C `int` OpenSSL takes. `RandBytes`, asked for a number of bytes past four gigabytes, returned a buffer of that size with only a handful of random bytes in it and the rest zero, and reported success.
177+
178+
These functions now raise on a length larger than an `int` can hold, before allocating anything.

CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,9 @@ All notable changes to this project will be documented in this file. This projec
1414
- Allow non-mutating methods to be called on a val receiver ([PR #89](https://github.com/ponylang/ssl/pull/89))
1515
- Fix leaks when loading Windows root certificates fails ([PR #90](https://github.com/ponylang/ssl/pull/90))
1616
- Fix a potential use-after-free in ALPN protocol selection ([PR #91](https://github.com/ponylang/ssl/pull/91))
17+
- Fix HmacSha256 returning an all-zero code when it fails ([PR #92](https://github.com/ponylang/ssl/pull/92))
18+
- Fix Digest returning a wrong hash or crashing when OpenSSL fails ([PR #92](https://github.com/ponylang/ssl/pull/92))
19+
- Fix crypto functions truncating a length too large for an int ([PR #92](https://github.com/ponylang/ssl/pull/92))
1720

1821
### Added
1922

@@ -23,6 +26,7 @@ All notable changes to this project will be documented in this file. This projec
2326
- Add SSLDisposed to SSLState ([PR #68](https://github.com/ponylang/ssl/pull/68))
2427
- Require a val resolver for alpn_set_resolver ([PR #81](https://github.com/ponylang/ssl/pull/81))
2528
- Require a val context for SSLContext.client and server ([PR #81](https://github.com/ponylang/ssl/pull/81))
29+
- Make Digest and HmacSha256 raise on failure ([PR #92](https://github.com/ponylang/ssl/pull/92))
2630

2731
## [2.1.0] - 2026-04-20
2832

examples/digest-example/digest-example.pony

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,23 +4,23 @@ use "../../ssl/crypto"
44

55
actor Main
66
new create(env: Env) =>
7-
let sha256digest: Digest = Digest.sha256()
7+
let sha256digest: Digest = Digest.sha256()?
88
try
99
sha256digest.append("Hello ")?
1010
sha256digest.append("World")?
11-
let hash: Array[U8] val = sha256digest.final()
11+
let hash: Array[U8] val = sha256digest.final()?
1212
env.out.print("SHA256: " + ToHexString(hash))
1313
else
1414
env.out.print("Error computing hash")
1515
end
1616

1717
// SHAKE256 with variable-length output (OpenSSL 3.0.x or 4.0.x)
1818
ifdef "openssl_3.0.x" or "openssl_4.0.x" then
19-
let shake: Digest = Digest.shake256(64)
19+
let shake: Digest = Digest.shake256(64)?
2020
try
2121
shake.append("Hello ")?
2222
shake.append("World")?
23-
let shake_hash: Array[U8] val = shake.final()
23+
let shake_hash: Array[U8] val = shake.final()?
2424
env.out.print("SHAKE256 (64 bytes): " + ToHexString(shake_hash))
2525
else
2626
env.out.print("Error computing SHAKE hash")

0 commit comments

Comments
 (0)