Commit e777fc0
committed
Peer review feedback
Addresses the Skoll multi-scan findings on the caller-supplied firmware
upgrade policy authorization work.
wolfTPM2_PolicyCommandCodeMake: digestSz is now in/out. On input it is the
output buffer capacity and BUFFER_E is returned when it is smaller than the
hash size, checked before the buffer is zeroed or hashed into. Previously the
function wrote hashSz bytes with no capacity known to it and overwrote
*digestSz before hashing, so a short caller buffer was silently overrun.
Updates the doxygen and the callers in the firmware and nvram examples.
wolfTPM2_PolicyOR: require at least two branch digests. TPM 2.0 Part 3
Sec.23.6 mandates a minimum of two and the fwTPM reference implementation
enforces it as TPM_RC_VALUE, so a one-branch list is now rejected with
BAD_FUNC_ARG rather than sent to the TPM.
Firmware upgrade _ex session contract: the vendor FieldUpgradeStart commands
are hand-marshalled with an authorization area carrying only the session
handle - empty nonceCaller, zero attributes and an empty HMAC. That is correct
only for an unsalted, unbound policy session with no auth value, so
wolfTPM2_FirmwareUpgradeHash_ex now validates the caller session up front and
rejects any session needing a computed session HMAC or parameter encryption
(PolicyAuthValue/PolicyPassword, attached auth value, bind, salt,
encrypt/decrypt/audit attributes, or a non-policy session handle). Validation
runs before any TPM traffic. The doxygen and README state the supported
contract and that a successful start consumes the session.
Firmware policy example cleanup: firmware_policy_clear now returns a status
and reports failure instead of silently ignoring it, and is gated on a new
FirmwarePolicyCtx that records what was actually provisioned - previously any
early failure (a missing firmware file, for example) cleared the platform
policy even when the example had never installed one, wiping a policy the
deployment owned. The PolicyOR filler branch is replaced with
PolicyCommandCode(TPM_CC_SetPrimaryPolicy) so the provisioned policy can
authorize its own removal when platformAuth is not the default. A failed
rollback now propagates to the example exit status.
Note the review's premise that cleanup cannot clear the policy it installed is
incorrect: per TPM 2.0 Part 1 Sec.19.7 a hierarchy is authorized by either its
authValue or its authPolicy, so installing an authPolicy does not disable the
password path that firmware_policy_clear uses. Confirmed on an ST33KTPM over
SPI and pinned by a new regression test.
Firmware examples build fix: ifx_fw_update and st33_fw_update drive
wolfTPM2_FirmwareUpgrade_ex, which only exists with wolfCrypt (it hashes the
manifest with SHA-384), but their feature guards did not require it. Building
--enable-firmware with --disable-wolfcrypt failed to compile. The guards now
include !WOLFTPM2_NO_WOLFCRYPT, matching the pattern used by the boot
examples, and the not-compiled-in message says so. This break predates the
policy work.
Tests: minimum PolicyOR branch count, short-buffer and canary coverage for
wolfTPM2_PolicyCommandCodeMake, caller-session rejection for every unsupported
session shape plus an accepted session reaching the manufacturer dispatch, and
a simulator-only regression test that a platform authPolicy is still clearable
with password authorization while a non-matching policy session is refused.
Documentation: README gains the session contract, a rewritten rollback section
explaining which mode can roll back and how, and the policy options that were
missing from the two usage blocks.1 parent 04a9269 commit e777fc0
9 files changed
Lines changed: 589 additions & 97 deletions
File tree
- examples
- firmware
- nvram
- src
- tests
- wolftpm
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
54 | 54 | | |
55 | 55 | | |
56 | 56 | | |
57 | | - | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
58 | 64 | | |
59 | 65 | | |
60 | 66 | | |
| |||
131 | 137 | | |
132 | 138 | | |
133 | 139 | | |
134 | | - | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
135 | 147 | | |
136 | 148 | | |
137 | 149 | | |
| |||
206 | 218 | | |
207 | 219 | | |
208 | 220 | | |
209 | | - | |
| 221 | + | |
210 | 222 | | |
211 | 223 | | |
| 224 | + | |
| 225 | + | |
212 | 226 | | |
213 | 227 | | |
214 | 228 | | |
| |||
228 | 242 | | |
229 | 243 | | |
230 | 244 | | |
231 | | - | |
232 | | - | |
233 | | - | |
| 245 | + | |
| 246 | + | |
| 247 | + | |
| 248 | + | |
234 | 249 | | |
235 | 250 | | |
236 | 251 | | |
| |||
254 | 269 | | |
255 | 270 | | |
256 | 271 | | |
257 | | - | |
| 272 | + | |
| 273 | + | |
| 274 | + | |
| 275 | + | |
| 276 | + | |
| 277 | + | |
| 278 | + | |
| 279 | + | |
| 280 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
81 | 81 | | |
82 | 82 | | |
83 | 83 | | |
84 | | - | |
85 | | - | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
86 | 87 | | |
87 | 88 | | |
88 | | - | |
| 89 | + | |
89 | 90 | | |
90 | 91 | | |
91 | 92 | | |
| |||
171 | 172 | | |
172 | 173 | | |
173 | 174 | | |
174 | | - | |
| 175 | + | |
| 176 | + | |
| 177 | + | |
| 178 | + | |
| 179 | + | |
175 | 180 | | |
176 | | - | |
177 | | - | |
| 181 | + | |
| 182 | + | |
| 183 | + | |
| 184 | + | |
| 185 | + | |
| 186 | + | |
| 187 | + | |
| 188 | + | |
| 189 | + | |
| 190 | + | |
| 191 | + | |
| 192 | + | |
| 193 | + | |
| 194 | + | |
| 195 | + | |
| 196 | + | |
| 197 | + | |
| 198 | + | |
| 199 | + | |
| 200 | + | |
| 201 | + | |
| 202 | + | |
| 203 | + | |
| 204 | + | |
| 205 | + | |
| 206 | + | |
| 207 | + | |
| 208 | + | |
| 209 | + | |
| 210 | + | |
| 211 | + | |
| 212 | + | |
| 213 | + | |
| 214 | + | |
| 215 | + | |
| 216 | + | |
| 217 | + | |
| 218 | + | |
| 219 | + | |
| 220 | + | |
| 221 | + | |
| 222 | + | |
| 223 | + | |
| 224 | + | |
| 225 | + | |
| 226 | + | |
| 227 | + | |
| 228 | + | |
| 229 | + | |
| 230 | + | |
| 231 | + | |
| 232 | + | |
| 233 | + | |
| 234 | + | |
| 235 | + | |
| 236 | + | |
| 237 | + | |
| 238 | + | |
| 239 | + | |
| 240 | + | |
| 241 | + | |
| 242 | + | |
| 243 | + | |
| 244 | + | |
| 245 | + | |
| 246 | + | |
| 247 | + | |
| 248 | + | |
| 249 | + | |
| 250 | + | |
| 251 | + | |
| 252 | + | |
| 253 | + | |
| 254 | + | |
| 255 | + | |
| 256 | + | |
| 257 | + | |
| 258 | + | |
| 259 | + | |
| 260 | + | |
| 261 | + | |
178 | 262 | | |
179 | 263 | | |
| 264 | + | |
| 265 | + | |
| 266 | + | |
| 267 | + | |
| 268 | + | |
| 269 | + | |
| 270 | + | |
| 271 | + | |
| 272 | + | |
180 | 273 | | |
181 | 274 | | |
182 | | - | |
| 275 | + | |
183 | 276 | | |
184 | 277 | | |
185 | 278 | | |
186 | 279 | | |
187 | | - | |
188 | 280 | | |
189 | 281 | | |
190 | | - | |
191 | | - | |
192 | 282 | | |
193 | 283 | | |
194 | 284 | | |
195 | 285 | | |
| 286 | + | |
| 287 | + | |
| 288 | + | |
196 | 289 | | |
197 | 290 | | |
198 | 291 | | |
| 292 | + | |
199 | 293 | | |
200 | 294 | | |
| 295 | + | |
| 296 | + | |
201 | 297 | | |
202 | 298 | | |
203 | 299 | | |
| |||
218 | 314 | | |
219 | 315 | | |
220 | 316 | | |
221 | | - | |
222 | | - | |
| 317 | + | |
| 318 | + | |
| 319 | + | |
223 | 320 | | |
224 | 321 | | |
225 | 322 | | |
| 323 | + | |
226 | 324 | | |
227 | | - | |
228 | | - | |
229 | | - | |
230 | | - | |
| 325 | + | |
| 326 | + | |
| 327 | + | |
| 328 | + | |
| 329 | + | |
| 330 | + | |
| 331 | + | |
231 | 332 | | |
232 | | - | |
233 | | - | |
| 333 | + | |
| 334 | + | |
234 | 335 | | |
235 | 336 | | |
236 | 337 | | |
237 | 338 | | |
238 | 339 | | |
239 | 340 | | |
240 | 341 | | |
241 | | - | |
| 342 | + | |
242 | 343 | | |
243 | 344 | | |
244 | 345 | | |
| |||
252 | 353 | | |
253 | 354 | | |
254 | 355 | | |
255 | | - | |
| 356 | + | |
256 | 357 | | |
257 | 358 | | |
258 | 359 | | |
| |||
269 | 370 | | |
270 | 371 | | |
271 | 372 | | |
272 | | - | |
273 | | - | |
274 | | - | |
275 | | - | |
276 | | - | |
| 373 | + | |
277 | 374 | | |
278 | 375 | | |
279 | 376 | | |
| |||
283 | 380 | | |
284 | 381 | | |
285 | 382 | | |
286 | | - | |
287 | | - | |
288 | | - | |
| 383 | + | |
289 | 384 | | |
290 | 385 | | |
291 | 386 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
36 | 36 | | |
37 | 37 | | |
38 | 38 | | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
39 | 54 | | |
40 | 55 | | |
41 | 56 | | |
| |||
45 | 60 | | |
46 | 61 | | |
47 | 62 | | |
48 | | - | |
49 | | - | |
50 | | - | |
51 | | - | |
52 | | - | |
53 | | - | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
54 | 72 | | |
55 | 73 | | |
56 | 74 | | |
57 | | - | |
58 | | - | |
59 | | - | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
60 | 89 | | |
61 | 90 | | |
62 | 91 | | |
| |||
0 commit comments