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

PatternAccuracyDescriptor.MinComplexity needs to be uint64 not uint32 #253

Open
omz13 opened this issue Jul 7, 2024 · 4 comments
Open
Labels
status/needs-triage Issues that need to be triaged. type/potential-bug Potential Bugs

Comments

@omz13
Copy link

omz13 commented Jul 7, 2024

Version

0.10.1

Description

When parsing the metadata, PatternAccuracyDescriptor.MinComplexity is too small; needs to be uint64 not uint32.

Reproduction

Parsing the latest version of the metadata blob (no=80):

json: cannot unmarshal number 34359738368 into Go struct field PatternAccuracyDescriptor.metadataStatement.userVerificationDetails.paDesc.minComplexity of type uint32

some pseudo code:

import (
	webauthnMetadata "github.com/go-webauthn/webauthn/metadata"
)

...
func() whatever(j *jwt) {

	raw := j.Payload.RawVal("entries")
	var entries []webauthnMetadata.MetadataBLOBPayloadEntry
	if err := json.Unmarshal(raw, &entries); err != nil {
		return err
	}
}

Expectations

modify to uint64

MinComplexity uint32 `json:"minComplexity"`

Documentation

No response

@omz13 omz13 added status/needs-triage Issues that need to be triaged. type/potential-bug Potential Bugs labels Jul 7, 2024
@james-d-elliott
Copy link
Member

james-d-elliott commented Jul 7, 2024

From what I can tell this is a bug with the blob. See the following references:

From the MDS3 Statement Spec:

dictionary PatternAccuracyDescriptor {
required unsigned long minComplexity;
unsigned short maxRetries;
unsigned short blockSlowdown;
};

From Web IDL:

2.13.9. unsigned long

The unsigned long type is an unsigned integer type that has values in the range [0, 4294967295].

unsigned long constant values in IDL are represented with integer tokens.

@omz13
Copy link
Author

omz13 commented Jul 7, 2024

Just because a spec says something it doesn't necessarily mean it is correct (erratum exist for a reason).

Is there a way to get confirmation that it is strictly 32 bits, and if so have that blob updated?

Parsing implementations in other languages do seem to be looser with their interpretation. As always: Postel's Law.

@james-d-elliott
Copy link
Member

What evidence to the idea that it must be a uint64 and not an unbounded number is there?

Is there a way to get confirmation that it is strictly 32 bits, and if so have that blob updated?

What additional confirmation would satisfy you other than the people who manage the MDS telling you that in the v3 vendor statements the value must be between 0 and 4294967295?

@omz13
Copy link
Author

omz13 commented Jul 8, 2024

The entry that is causing problems is "aaid": "0056#0002" ("PixelPin - Picture Login") and, unless I am mistaken, this is the only entry with a userVerificationDetails.paDesc.minComplexity value; it has also been around since 2018.

Ping @dturnerx - can you help clarify (or suggest who else could resolve) this situation? Is the MDS blob (no = 80) borked wrt this entry, or is the entry correct but the spec slightly off (and it really should say uint64).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status/needs-triage Issues that need to be triaged. type/potential-bug Potential Bugs
Projects
None yet
Development

No branches or pull requests

2 participants