dotnet / dotnet/sdk

Revisit certificate pinning in `dotnetup` signature verification with Crypto Board / Threat Model

Open
#54,534 0 comments 1 reaction 0 assignees View on GitHub
Area-dotnetup untriaged
Dominant language
C#
Stars
3.2k
Forks
1.3k
PR merge metrics
PR metrics pending

Description

### Prerequisites

- [x] I have read the [dotnetup documentation](https://github.com/dotnet/sdk/tree/release/dnup/documentation/general/dotnetup).
- [x] I have searched for [existing dotnetup issues](https://github.com/dotnet/sdk/issues?q=is%3Aissue%20state%3Aopen%20label%3Adotnetup).
- [x] I have searched for [existing dotnetup discussions](https://github.com/dotnet/sdk/discussions/categories/dotnetup).

### Issue type

Feature request

### Description

## Summary

The current `dotnetup` signature verifier pins on the signer (and timestamp) certificate's
issuer/subject identity, in addition to chain + EKU + algorithm checks. That pinning was
required by the approved Crypto Board (CB) and Threat Model (TM) docs that this
implementation was built against.

In review of #54300, signing experts flagged that this is no longer the current
recommendation:

> The current MS Crypto Board recommendations are against certificate pinning
> strategies, and would bias toward checks like "ensure id-kp-codeSign is present,
> ignore all other EKUs, and make sure it's rooted against a certificate in the codesign
> trust list" perhaps with a follow-on of making sure something in the subject/SAN can
> bind the certificate to the expected issuer (... by "issuer" here I mean "code signer",
> not "CA", so that would be the subject field of the certificate). Do note that that
> opinion has changed in the last few years, pinning used to be all the rage.
>
> — @bartonjs, [#54300 (comment)](https://github.com/dotnet/sdk/pull/54300#discussion_r3326487151)

A similar concern was raised separately by @dtivel.

In #54300 we kept the pinning behavior for now because removing it would contradict the
CB/TM docs that are the source of truth for this verifier. This issue tracks the follow-up
work to align everything.

## Work to do

This is purely a tracking issue — no code change is intended here on its own. The work is:

1. Re-engage the MS Crypto Board and the threat-model owners with the feedback from
#54300.
2. Get the CB / TM docs updated so they no longer require certificate pinning (issuer
subject, geographic RDNs such as L/ST/C, etc.) as a hard requirement, and instead
reflect the current "rooted to codesign trust list + EKU + subject/SAN binds to expected
code signer" guidance.
3. Coordinate with the security leader group (including @blowdart) on the policy change
and on what the minimum required checks should be going forward.
4. Once the docs are updated, open a follow-up PR against `SignatureVerifier` to relax the
pinning checks to match.

## Out of scope

- The unrelated crypto-agility feedback from #54300 (single-`SignerInfo` restriction,
multi-version `dotnetup` compatibility, CAdES wording) — those are tracked separately.

## References

- PR: [dotnet/sdk#54300 — `dotnetup` verifies release signatures](https://github.com/dotnet/sdk/pull/54300)
- Specific thread: [#54300 (comment)](https://github.com/dotnet/sdk/pull/54300#discussion_r3326487151)
- Spec doc affected: `documentation/general/dotnetup/signature-verification.md`
- Code affected: `src/Installer/Microsoft.Dotnet.Installation/Internal/Signing/SignatureVerifier.cs`

### dotnetup version

_No response_

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.