Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reverting some changes #554

Merged
merged 1 commit into from
Oct 18, 2024
Merged

Conversation

abergs
Copy link
Collaborator

@abergs abergs commented Oct 18, 2024

Until further review, we've decided to revert these changes.

I'm keeping these around until we've understood if we really can drop them
@codecov-commenter
Copy link

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 74.62%. Comparing base (11121e6) to head (ac11a81).

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@                  Coverage Diff                  @@
##           conformance-1.7.20-4     #554   +/-   ##
=====================================================
  Coverage                 74.62%   74.62%           
=====================================================
  Files                       102      102           
  Lines                      2719     2719           
  Branches                    464      464           
=====================================================
  Hits                       2029     2029           
  Misses                      582      582           
  Partials                    108      108           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@abergs abergs merged commit 8a65ee0 into conformance-1.7.20-4 Oct 18, 2024
7 checks passed
@abergs abergs deleted the small-conformance-1.7.20-4 branch October 18, 2024 10:08
@abergs abergs mentioned this pull request Oct 18, 2024
abergs added a commit that referenced this pull request Oct 18, 2024
This PR does not reach full conformance testing score because of #554, but sets the bed nicely for reaching conformance.

* FIDO Conformance Tools v1.7.15 fixes


TrustAnchor.cs : 32
Server-ServerAuthenticatorAttestationResponse-Resp-5 Test server processing "packed" FULL attestation
F-10 Send ServerAuthenticatorAttestationResponse with FULL "packed" attestation, with attStmt.x5c containing full chain, and check that server returns an error
https://datatracker.ietf.org/doc/html/rfc5280#section-6.1

AuthenticatorAttestationRawResponse.cs : 18
Server-ServerAuthenticatorAttestationResponse-Resp-1 Test server processing ServerAuthenticatorAttestationResponse structure
F-4 Send ServerAuthenticatorAttestationResponse that is missing "type" field and check that server returns an error

CredentialCreateOptions.cs : 96
Server-ServerAuthenticatorAttestationResponse-Resp-4 Test server support of the authentication algorithms
P-8 Send a valid ServerAuthenticatorAttestationResponse with SELF "packed" attestation, for "ALG_SIGN_RSASSA_PKCSV15_SHA1_RAW" aka "RS1" algorithm, and check that server succeeds
Server-ServerAuthenticatorAttestationResponse-Resp-9 Test server processing "tpm" attestation
P-2 Send a valid ServerAuthenticatorAttestationResponse with "tpm" attestation for SHA-1, and check that server succeeds

CredentialCreateOptions.cs  : 210
Server-ServerPublicKeyCredentialCreationOptions-Req-1 Test server generating ServerPublicKeyCredentialCreationOptionsRequest
P-1 Get ServerPublicKeyCredentialCreationOptionsResponse, and check that: (a) response MUST contain ...

AuthenticationExtensionsClientInputs.cs : 23 public string AppID { private get; set; }
Server-ServerPublicKeyCredentialGetOptionsResponse-Req-1 Test server generating ServerPublicKeyCredentialGetOptionsResponse
P-1 Get ServerPublicKeyCredentialGetOptionsResponse, and check that: (a) response MUST contain ...

AuthenticationExtensionsClientInputs.cs :  44 public bool? UserVerificationMethod { private get; set; }
Server-ServerPublicKeyCredentialGetOptionsResponse-Req-1 Test server generating ServerPublicKeyCredentialGetOptionsResponse
P-1 Get ServerPublicKeyCredentialGetOptionsResponse, and check that: (a) response MUST contain ...

AuthenticatorAssertionResponse.cs : 128
Server-ServerAuthenticatorAssertionResponse-Resp-3
P4,P6,P7

CryptoUtils.cs 64 (trustpath length 1 with exact match in attestation root certs)
Server-ServerAuthenticatorAttestationResponse-Resp-5 Test server processing "packed" FULL attestation
P-3 Send a valid ServerAuthenticatorAttestationResponse with FULL "packed" attestation that contains batch certificate, that is simply self referenced in the metadata, and check that server succeeds

CryptoUtils.cs 105 - X509RevocationMode.Online makes conformance sad
Server-ServerAuthenticatorAttestationResponse-Resp-9 Test server processing "tpm" attestation
P-1 Send a valid ServerAuthenticatorAttestationResponse with "tpm" attestation for SHA-256, and check that server succeeds‣
P-2 Send a valid ServerAuthenticatorAttestationResponse with "tpm" attestation for SHA-1, and check that server succeeds‣
P-3 Send a valid ServerAuthenticatorAttestationResponse with "tpm" attestation pubArea.nameAlg is not matching algorithm used for generate attested.name, and check that server succeeds

TestController.cs tojson -> serialize
serialization error

* Json serialization fix

Json serialization fix. (Object type vs ToJson())

* Unit test fix

* tokenbindig, AppId, UVP

Back to 100% conformance.
TokenBinding logic readded.
AppId: prevent serialization in a nicer way.
UV flags are verified differently for conformance testing, otherwise as described in the RFC.

* unit test fix (tokenbinding dto parsing)

* fix azure pipeline

fix azure pipeline's whitespace error + removing unused using

* Improve trustanchor test coverage

Improve trustanchor test coverage based on codecov report

* TestPackedttestationAsyncFailTrustAnchorOnRootCertInTrustPath only works on Windows

* Do not make this private

* Keep Tokenbinding around

* Update AuthenticatorAssertionResponse.cs

* Added XML comments to requestTokenBinding

* Added comment about UVM

* Simplify UVP

* format

* Reverting some changes (#554)

I'm keeping these around until we've understood if we really can drop them

* Ignores Demo/Conformance

* Refactored away from bool to enum.

* File based namespace

* format

---------

Co-authored-by: Gabor Mihaly <[email protected]>
Co-authored-by: googyi <[email protected]>
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.

2 participants