Improve tpm

[vanbukin]

No description provided.

[Copilot]

Pull request overview

This PR refines TPM (Trusted Platform Module) “publicArea” decoding in the WebAuthn TPM attestation verifier by expanding parsed TPM selector enums and adding more detailed structure-handling logic, while simplifying some TPM model shapes. It also adjusts demo logging configuration.

Changes:

• Expanded TPM enum coverage (RSA schemes, ECC schemes, KDF schemes) and updated TPM-related documentation/comments.
• Tightened/extended TPM publicArea parsing logic (RSA/ECC scheme parsing, KDF scheme detail consumption) and simplified TPM models (PubArea, RsaParms).
• Updated the FIDO conformance demo logging level configuration.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 9 comments.

Show a summary per file

File	Description
src/WebAuthn.Net/Services/Common/AttestationStatementVerifier/Implementation/Tpm/DefaultTpmPubAreaDecoder.cs	Major updates to TPMT_PUBLIC parsing/validation and additional scheme/KDF consumption logic.
src/WebAuthn.Net/Services/Common/AttestationStatementVerifier/Abstractions/Tpm/Models/Attestation/RsaParms.cs	Simplifies RSA parameters model by dropping KeyBits.
src/WebAuthn.Net/Services/Common/AttestationStatementVerifier/Abstractions/Tpm/Models/Attestation/PubArea.cs	Simplifies public area model by dropping NameAlg and ObjectAttributes.
src/WebAuthn.Net/Services/Common/AttestationStatementVerifier/Abstractions/Tpm/Models/Attestation/Enums/TpmiAlgRsaScheme.cs	Adds RSA scheme constants and updates XML docs link.
src/WebAuthn.Net/Services/Common/AttestationStatementVerifier/Abstractions/Tpm/Models/Attestation/Enums/TpmiAlgKdf.cs	Adds KDF constants and documentation.
src/WebAuthn.Net/Services/Common/AttestationStatementVerifier/Abstractions/Tpm/Models/Attestation/Enums/TpmiAlgEccScheme.cs	Adds ECC scheme constants and expands XML docs.
src/WebAuthn.Net/Services/Common/AttestationStatementVerifier/Abstractions/Tpm/Models/Attestation/Enums/ObjectAttributes.cs	Adds additional object attribute flags + doc tweaks.
src/WebAuthn.Net/Services/Common/AttestationStatementVerifier/Abstractions/Tpm/ITpmPubAreaDecoder.cs	Renames parameter bytes → publicArea.
demo/WebAuthn.Net.Demo.FidoConformance/appsettings.json	Increases logging verbosity for the conformance demo.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (8)

src/WebAuthn.Net/Services/Common/AttestationStatementVerifier/Implementation/Tpm/DefaultTpmPubAreaDecoder.cs:259

• The comment describing the default RSA exponent says "216 + 1"; the TPM default exponent is 2^16 + 1 (65537). This typo is confusing (especially since the correct expression is used later in the method).

        // An exponent of zero indicates that the exponent is the default of 216 + 1.

src/WebAuthn.Net/Services/Common/AttestationStatementVerifier/Implementation/Tpm/DefaultTpmPubAreaDecoder.cs:638

• After decoding TPMT_ECC_SCHEME.scheme, the code only checks Enum.IsDefined and then proceeds to consume scheme details. Per the table comment above, the allowed scheme values depend on the objectAttributes (sign vs decrypt). Without validating that relationship, malformed/invalid pubArea structures can be accepted and (worse) parsed as if they were valid for that key usage.

        var scheme = (TpmiAlgEccScheme) BinaryPrimitives.ReadUInt16BigEndian(rawScheme);
        if (!Enum.IsDefined(scheme))
        {
            return Result<EccParms>.Fail();
        }

src/WebAuthn.Net/Services/Common/AttestationStatementVerifier/Implementation/Tpm/DefaultTpmPubAreaDecoder.cs:804

• kdfScheme is cast from a raw UINT16 but never validated. For an unknown (non-NULL) value this code will still consume exactly 2 bytes of details, which can desynchronize parsing of the rest of pubArea. It’s safer to fail fast on unknown values.

        var kdfScheme = (TpmiAlgKdf) BinaryPrimitives.ReadUInt16BigEndian(rawKdfScheme);
        if (kdfScheme != TpmiAlgKdf.TpmAlgNull)
        {

src/WebAuthn.Net/Services/Common/AttestationStatementVerifier/Implementation/Tpm/DefaultTpmPubAreaDecoder.cs:828

• The decoded KDF scheme hashAlg is read into a local variable that is never used, which will produce an unused-local warning (and may fail builds if warnings are treated as errors). If the value is intentionally ignored, consume it into _.

            // consume hashAlg
            if (!TryConsume(ref buffer, 2, out var rawKdfSchemeHashAlg))
            {
                return Result<EccParms>.Fail();
            }

src/WebAuthn.Net/Services/Common/AttestationStatementVerifier/Abstractions/Tpm/Models/Attestation/Enums/TpmiAlgKdf.cs:35

• Enum member name looks like a typo: TPM_ALG_MGF1 is a mask generation function, but the identifier is spelled "TpmAlgMfg1" (MFG vs MGF). This makes call sites harder to read/search.

    TpmAlgMfg1 = 0x0007,

src/WebAuthn.Net/Services/Common/AttestationStatementVerifier/Abstractions/Tpm/Models/Attestation/Enums/TpmiAlgEccScheme.cs:87

• The summary for TPM_ALG_ECDH is labeled as an EdDSA signature algorithm, but ECDH is a key exchange scheme. This is misleading in generated docs.

    /// <summary>
    /// <para>Edwards-curve Digital Signature Algorithm (HashEdDSA)</para>
    /// <para>
    /// Based on context, this can be one of:
    ///     <list type="bullet">

src/WebAuthn.Net/Services/Common/AttestationStatementVerifier/Abstractions/Tpm/Models/Attestation/Enums/TpmiAlgRsaScheme.cs:46

• There is an empty XML doc tag ("") before the real remarks block. This produces noisy/invalid XML documentation; use a blank doc line instead or remove it.

    /// <remarks></remarks>

src/WebAuthn.Net/Services/Common/AttestationStatementVerifier/Abstractions/Tpm/Models/Attestation/Enums/TpmiAlgEccScheme.cs:11

• The hyperlink target is for the 1.83 structures PDF, but the link text still references Revision 01.59 (2019). Please align the displayed text with the actual referenced document to avoid confusion when cross-checking the spec.

///         <a href="https://trustedcomputinggroup.org/wp-content/uploads/TPM-2.0-1.83-Part-2-Structures.pdf">TPM 2.0 Library - Part 2: Structures, Family "2.0", Level 00 Revision 01.59, November 8, 2019</a>

[Copilot]

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (7)

src/WebAuthn.Net/Services/Common/AttestationStatementVerifier/Implementation/Tpm/DefaultTpmPubAreaDecoder.cs:260

• The comment describing the RSA default exponent has a typo: it says "216 + 1" but should be "2^16 + 1" (65537).

        // A TPM compatible with this specification and supporting RSA shall support two primes and an exponent of zero.
        // An exponent of zero indicates that the exponent is the default of 2^16 + 1.
        // Support for other values is optional.

src/WebAuthn.Net/Services/Common/AttestationStatementVerifier/Implementation/Tpm/DefaultTpmPubAreaDecoder.cs:829

• rawKdfSchemeHashAlg is only declared to satisfy TryConsume and is never used, which can trigger an “assigned but never used” warning. Use out _ and drop the unused local (and the commented-out read).

            // {
            //     return UINT16_Marshal((UINT16*)(source), (buffer), (size));
            // }

            // consume hashAlg

src/WebAuthn.Net/Services/Common/AttestationStatementVerifier/Abstractions/Tpm/Models/Attestation/Enums/TpmiAlgRsaScheme.cs:47

• There are two <remarks> tags on TpmAlgRsapss (one empty and one with content). This can confuse documentation tooling; keep only a single <remarks> block.

    /// <remarks></remarks>
    /// <remarks>

src/WebAuthn.Net/Services/Common/AttestationStatementVerifier/Abstractions/Tpm/Models/Attestation/Enums/TpmiAlgEccScheme.cs:85

• The XML summary for TpmAlgEcdh currently describes EdDSA/HashEdDSA, but this enum value represents ECDH (key exchange). The description should be updated to avoid misleading generated docs.

    /// <summary>
    /// <para>Edwards-curve Digital Signature Algorithm (HashEdDSA)</para>
    /// <para>

src/WebAuthn.Net/Services/Common/AttestationStatementVerifier/Implementation/Tpm/DefaultTpmPubAreaDecoder.cs:487

• This PR adds/changes decoding branches for non-NULL RSA/ECC schemes (including consuming [scheme]details), but the existing TPM ceremony tests appear to cover only the current attestation vectors. Adding unit tests with vectors where scheme != TPM_ALG_NULL (and where kdf.scheme != TPM_ALG_NULL) would help prevent regressions in buffer consumption and validation logic.

        if (scheme == TpmiAlgRsaScheme.TpmAlgRsassa
            || scheme == TpmiAlgRsaScheme.TpmAlgRsapss
            || scheme == TpmiAlgRsaScheme.TpmAlgOaep)
        {
            // consume hashAlg
            if (!TryConsume(ref buffer, 2, out _))
            {
                return Result<RsaParms>.Fail();
            }
        }

src/WebAuthn.Net/Services/Common/AttestationStatementVerifier/Abstractions/Tpm/Models/Attestation/PubArea.cs:37

• PubArea is a public type, and removing the NameAlg / ObjectAttributes properties and constructor parameters is a breaking API change for any external consumers that referenced them. If this library aims to preserve backwards compatibility, consider keeping those members (possibly [Obsolete]) and continuing to populate them in the decoder.

    /// <summary>
    ///     Constructs <see cref="PubArea" />.
    /// </summary>
    /// <param name="type">"algorithm" associated with this object.</param>
    /// <param name="parameters">The algorithm or structure details.</param>
    /// <param name="unique">
    ///     <para>The unique identifier of the structure.</para>
    ///     <para>For an asymmetric key, this would be the public key.</para>
    /// </param>
    public PubArea(
        TpmAlgPublic type,
        AbstractPublicParms parameters,
        AbstractUnique unique)
    {
        Type = type;
        Parameters = parameters;
        Unique = unique;
    }

src/WebAuthn.Net/Services/Common/AttestationStatementVerifier/Abstractions/Tpm/Models/Attestation/RsaParms.cs:27

• RsaParms is public, and dropping the KeyBits property / constructor parameter is a breaking API change for any external consumers. If compatibility is required, consider reintroducing KeyBits (and/or an overload) and still consuming/storing it during decoding.

    /// <summary>
    ///     Constructs <see cref="RsaParms" />.
    /// </summary>
    /// <param name="exponent">The public exponent. A prime number greater than 2.</param>
    public RsaParms(uint exponent)
    {
        Exponent = exponent;
    }

[Copilot]

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (4)

src/WebAuthn.Net/Services/Common/AttestationStatementVerifier/Implementation/Tpm/DefaultTpmPubAreaDecoder.cs:638

• ECC scheme decoding currently only checks that the value is a defined TpmiAlgEccScheme, but it doesn’t validate that the scheme is compatible with the key usage flags in objectAttributes (sign vs decrypt). This can allow structurally invalid TPMT_PUBLIC values (e.g., key-exchange schemes on sign-only keys) to be accepted.

        var scheme = (TpmiAlgEccScheme) BinaryPrimitives.ReadUInt16BigEndian(rawScheme);
        if (!Enum.IsDefined(scheme))
        {
            return Result<EccParms>.Fail();
        }

src/WebAuthn.Net/Services/Common/AttestationStatementVerifier/Abstractions/Tpm/Models/Attestation/Enums/TpmiAlgRsaScheme.cs:47

• There is an empty XML documentation tag (<remarks></remarks>) immediately followed by another <remarks> block. This is redundant and can confuse generated docs.

    /// <summary>
    /// A signature algorithm defined in clause 8.1 (RSASSA-PSS)
    /// </summary>
    /// <remarks></remarks>
    /// <remarks>

src/WebAuthn.Net/Services/Common/AttestationStatementVerifier/Abstractions/Tpm/Models/Attestation/Enums/TpmiAlgEccScheme.cs:85

• The ECDH enum member summary incorrectly refers to EdDSA/HashEdDSA. This appears to be a copy/paste mistake and makes the generated docs misleading.

    /// <summary>
    /// <para>Edwards-curve Digital Signature Algorithm (HashEdDSA)</para>
    /// <para>

src/WebAuthn.Net/Services/Common/AttestationStatementVerifier/Implementation/Tpm/DefaultTpmPubAreaDecoder.cs:788

• The comment states only NIST P-256/P-384/P-521 are supported, but the current check only verifies the value is defined in the enum. This allows other curves to pass decoding and then fail later when converting to ECCurve; consider failing fast here to match the stated support.

        // We currently support only:
        // TPM_ECC_NIST_P256
        // TPM_ECC_NIST_P384
        // TPM_ECC_NIST_P521
        if (!Enum.IsDefined(curveId))