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

Documentation doesn't define credentialBytes for parsedCredentialRequest.Verify #291

Open
coolaj86 opened this issue Sep 3, 2024 · 0 comments
Labels
status/needs-triage Issues that need to be triaged. type/potential-bug Potential Bugs

Comments

@coolaj86
Copy link

coolaj86 commented Sep 3, 2024

Version

0.11.2

Bottom Line

parsedCredentialCreation.Response.AttestationObject.AuthData.AttData.CredentialPublicKey is the CBOR (not ASN.1 / DER) publicKey bytes referred to as credentialBytes by parsedCredentialRequest.Verify.

It would be great if the documentation were updated to reflect that, since the term on its own is ambiguous.

Description

parsedCredentialRequest.Verify doesn't define what credentialBytes is or where it comes from, and even looking at the function definition, it's not clear what it is.

There's no other mention of credentialBytes anywhere in the code.

Reproduction

I was originally preparing to file this as a bug that the credentialBytes don't accept the ASN.1 / DER publicKey bytes, but as I was preparing the test case, I eventually found that

parsedCredentialCreation.Response.AttestationObject.AuthData.AttData.CredentialPublicKey is the CBOR-encoded key that the function expects.

package main

import (
	"fmt"
	"log"

	"github.com/go-webauthn/webauthn/protocol"
)

func main() {
	rpID := "local.pocketid.app"
	challenge := "GrHwQknBaKm65edJQhp3lcC8CwYsvV0OrC9jDK4zBAo"

	credentialCreationResponse := []byte(`{
	  "authenticatorAttachment": "cross-platform",
	  "id": "0t-MoyT0A62LEsHNlBnamxOpjyBF4jkP4To82gM_HP2lBHxQXIsVOF2-oT_gEFZD",
	  "rawId": "0t-MoyT0A62LEsHNlBnamxOpjyBF4jkP4To82gM_HP2lBHxQXIsVOF2-oT_gEFZD",
	  "response": {
		"attestationObject": "o2NmbXRkbm9uZWdhdHRTdG10oGhhdXRoRGF0YVjCvyNDyj998pf0HSG0UpSMrRAgHVyp9ZDvZgDE0HEs2jDFAAAABAAAAAAAAAAAAAAAAAAAAAAAMNLfjKMk9AOtixLBzZQZ2psTqY8gReI5D-E6PNoDPxz9pQR8UFyLFThdvqE_4BBWQ6UBAgMmIAEhWCDS34yjJPQDrYsSwc2UwDa13_7I_6xJ4D8SvymXQJlffyJYIMj5vh_A5ClZdQiAy9Imbmlu_bLIMGf1OXBtoRS4lYZioWtjcmVkUHJvdGVjdAI",
		"authenticatorData": "vyNDyj998pf0HSG0UpSMrRAgHVyp9ZDvZgDE0HEs2jDFAAAABAAAAAAAAAAAAAAAAAAAAAAAMNLfjKMk9AOtixLBzZQZ2psTqY8gReI5D-E6PNoDPxz9pQR8UFyLFThdvqE_4BBWQ6UBAgMmIAEhWCDS34yjJPQDrYsSwc2UwDa13_7I_6xJ4D8SvymXQJlffyJYIMj5vh_A5ClZdQiAy9Imbmlu_bLIMGf1OXBtoRS4lYZioWtjcmVkUHJvdGVjdAI",
		"clientDataJSON": "eyJ0eXBlIjoid2ViYXV0aG4uY3JlYXRlIiwiY2hhbGxlbmdlIjoiRlZNRjh4TTd1U1U3U19RamJ4MC1KUjBQV2hfbGprN0NaVWNtQkczS0cxUSIsIm9yaWdpbiI6Imh0dHBzOi8vbG9jYWwucG9ja2V0aWQuYXBwIiwiY3Jvc3NPcmlnaW4iOmZhbHNlfQ",
		"publicKey": "MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAE0t-MoyT0A62LEsHNlMA2td_-yP-sSeA_Er8pl0CZX3_I-b4fwOQpWXUIgMvSJm5pbv2yyDBn9TlwbaEUuJWGYg",
		"publicKeyAlgorithm": -7,
		"publicKeyAlgorithmName": "ES256",
		"transports": [
		  "nfc",
		  "usb"
		]
	  },
	  "type": "public-key"
	}`)

	credentialRequestResponse := []byte(`{
	  "authenticatorAttachment": "cross-platform",
	  "id": "0t-MoyT0A62LEsHNlBnamxOpjyBF4jkP4To82gM_HP2lBHxQXIsVOF2-oT_gEFZD",
	  "rawId": "0t-MoyT0A62LEsHNlBnamxOpjyBF4jkP4To82gM_HP2lBHxQXIsVOF2-oT_gEFZD",
	  "response": {
		"authenticatorData": "vyNDyj998pf0HSG0UpSMrRAgHVyp9ZDvZgDE0HEs2jAFAAAABw",
		"clientDataJSON": "eyJ0eXBlIjoid2ViYXV0aG4uZ2V0IiwiY2hhbGxlbmdlIjoiR3JId1FrbkJhS202NWVkSlFocDNsY0M4Q3dZc3ZWME9yQzlqREs0ekJBbyIsIm9yaWdpbiI6Imh0dHBzOi8vbG9jYWwucG9ja2V0aWQuYXBwIiwiY3Jvc3NPcmlnaW4iOmZhbHNlfQ",
		"signature": "MEYCIQCy_EZw4k0JT51ywQnd1z3x_UgFb3p3nEy1b7G3AnpLWgIhAIoC-9luoBy5FTRfq-66-uZ0fYDCAKh9M1YITV9TfUff",
		"userHandle": "YWpAdGhlcm9vdGNvbXBhbnkuY29t"
	  },
	  "type": "public-key"
	}`)

	parsedCredentialCreation, err := protocol.ParseCredentialCreationResponseBytes(credentialCreationResponse)
	if err != nil {
		log.Fatalf("Failed to parse creation attestation bytes: %v", err)
	}

	parsedCredentialRequest, err := protocol.ParseCredentialRequestResponseBytes(credentialRequestResponse)
	if err != nil {
		log.Fatalf("Failed to parse request assertion bytes: %v", err)
	}

	appId := ""
	allowedOrigins := []string{fmt.Sprintf("https://%s", rpID)}
	var topOrigins []string = nil
	userVerification := true

	err = parsedCredentialRequest.Verify(
		challenge,
		rpID,
		allowedOrigins,
		topOrigins,
		protocol.TopOriginAutoVerificationMode,
		appId,
		userVerification,
		parsedCredentialCreation.Response.AttestationObject.AuthData.AttData.CredentialPublicKey,
	)
	if err != nil {
		log.Fatalf("Assertion verification failed: %v", err)
	}
	fmt.Println("Assertion verified successfully.")
}

Expectations

It should be more clear as to what credentialBytes is, and it should be referred by the same name in the documentation and code where it is used.

Perhaps renaming it to:

  • credentialPublicKey
  • or to be clear that it's not the age-old DER format: credentialPublicKeyCBORBytes

Documentation

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

1 participant