Skip to content

Commit

Permalink
fix(challenge): urlsafe base64 encoding (#82)
Browse files Browse the repository at this point in the history
This fixes issue with wrong encoding being used for the challenge.
According to the specs[^1] the challenge should be base64 url-safe
encoded. Until now, the package used std encoding which uses slightly
different set of characters. This was reported in the linked issue
and we also encountered the same issue when testing our implementation
of webauthn e2e using github.com/descope/virtualwebauthn.

The commit removes the custom type for challenge and instead passes
around the challenge url-safe base64 encoded right from the point of
creation. Because the string is fully valid it can be safely used
in json as well as stored in redis or other possible storage
implementations that might be used by the clients of the library.

I tried to keep the tests as close to the original as possible.
In some cases that required transforming the challenge back to bytes
considering the previously used padding to be part of the bytes
and then re-encoding into url-safe base64 string.

[^1]: https://www.w3.org/TR/webauthn-2/#dom-collectedclientdata-challenge

Fix: duo-labs/webauthn#128
  • Loading branch information
matoous committed Dec 17, 2022
1 parent 694d289 commit 6abd351
Show file tree
Hide file tree
Showing 11 changed files with 31 additions and 41 deletions.
2 changes: 1 addition & 1 deletion protocol/assertion_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ func TestParsedCredentialAssertionData_Verify(t *testing.T) {
Raw CredentialAssertionResponse
}
type args struct {
storedChallenge Challenge
storedChallenge URLEncodedBase64
relyingPartyID string
relyingPartyOrigin string
verifyUser bool
Expand Down
6 changes: 3 additions & 3 deletions protocol/attestation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ func TestPackedAttestationVerification(t *testing.T) {
var testAttestationOptions = []string{
// Direct Self Attestation with EC256 - MacOS
`{"publicKey": {
"challenge": "rWiex8xDOPfiCgyFu4BLW6vVOmXKgPwHrlMCgEs9SBA=",
"challenge": "rWiex8xDOPfiCgyFu4BLW6vVOmXKgPwHrlMCgEs9SBA",
"rp": {
"name": "http:https://localhost:9005",
"id": "localhost"
Expand All @@ -108,7 +108,7 @@ var testAttestationOptions = []string{
}}`,
// Direct Attestation with EC256
`{"publicKey": {
"challenge": "+Ri5NZTzJ8b6mvW3TVScLotEoALfgBa2Bn4YSaIObHc=",
"challenge": "-Ri5NZTzJ8b6mvW3TVScLotEoALfgBa2Bn4YSaIObHc",
"rp": {
"name": "https://webauthn.io",
"id": "webauthn.io"
Expand All @@ -134,7 +134,7 @@ var testAttestationOptions = []string{
// None Attestation with EC256
`{
"publicKey": {
"challenge": "sVt4ScceMzqFSnfAq8hgLzblvo3fa4/aFVEcIESHIJ0=",
"challenge": "sVt4ScceMzqFSnfAq8hgLzblvo3fa4_aFVEcIESHIJ0",
"rp": {
"name": "https://webauthn.io",
"id": "webauthn.io"
Expand Down
4 changes: 4 additions & 0 deletions protocol/base64.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@ import (
// decoded into a byte slice.
type URLEncodedBase64 []byte

func (e URLEncodedBase64) String() string {
return base64.RawURLEncoding.EncodeToString(e)
}

// UnmarshalJSON base64 decodes a URL-encoded value, storing the result in the
// provided byte slice.
func (e *URLEncodedBase64) UnmarshalJSON(data []byte) error {
Expand Down
16 changes: 4 additions & 12 deletions protocol/challenge.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,26 +2,18 @@ package protocol

import (
"crypto/rand"
"encoding/base64"
)

// ChallengeLength - Length of bytes to generate for a challenge
const ChallengeLength = 32

// Challenge that should be signed and returned by the authenticator
type Challenge URLEncodedBase64

// Create a new challenge to be sent to the authenticator. The spec recommends using
// at least 16 bytes with 100 bits of entropy. We use 32 bytes.
func CreateChallenge() (Challenge, error) {
// Create a new challenge that should be signed and returned by the authenticator.
// The spec recommends using at least 16 bytes with 100 bits of entropy. We use 32 bytes.
func CreateChallenge() (URLEncodedBase64, error) {
challenge := make([]byte, ChallengeLength)
_, err := rand.Read(challenge)
if err != nil {
return nil, err
}
return challenge, nil
}

func (c Challenge) String() string {
return base64.RawURLEncoding.EncodeToString(c)
return URLEncodedBase64(challenge), nil
}
6 changes: 3 additions & 3 deletions protocol/challenge_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,12 @@ import (
func TestCreateChallenge(t *testing.T) {
tests := []struct {
name string
want Challenge
want URLEncodedBase64
wantErr bool
}{
{
"Successfull Challenge Create",
Challenge{},
URLEncodedBase64{},
false,
},
}
Expand Down Expand Up @@ -42,7 +42,7 @@ func TestChallenge_String(t *testing.T) {
wantChallenge := base64.RawURLEncoding.EncodeToString(newChallenge)
tests := []struct {
name string
c Challenge
c URLEncodedBase64
want string
}{
{
Expand Down
20 changes: 8 additions & 12 deletions protocol/client_test.go
Original file line number Diff line number Diff line change
@@ -1,19 +1,16 @@
package protocol

import (
"encoding/base64"
"net/url"
"testing"
)

func setupCollectedClientData(challenge []byte) *CollectedClientData {
ccd := &CollectedClientData{
Type: CreateCeremony,
Origin: "example.com",
func setupCollectedClientData(challenge URLEncodedBase64) *CollectedClientData {
return &CollectedClientData{
Type: CreateCeremony,
Origin: "example.com",
Challenge: challenge.String(),
}

ccd.Challenge = base64.RawURLEncoding.EncodeToString(challenge)
return ccd
}

func TestVerifyCollectedClientData(t *testing.T) {
Expand All @@ -28,7 +25,7 @@ func TestVerifyCollectedClientData(t *testing.T) {
originURL, _ := url.Parse(ccd.Origin)
err = ccd.Verify(storedChallenge.String(), ccd.Type, FullyQualifiedOrigin(originURL))
if err != nil {
t.Fatalf("error verifying challenge: expected %#v got %#v", Challenge(ccd.Challenge), storedChallenge)
t.Fatalf("error verifying challenge: expected %#v got %#v", ccd.Challenge, storedChallenge)
}
}

Expand All @@ -42,9 +39,8 @@ func TestVerifyCollectedClientDataIncorrectChallenge(t *testing.T) {
if err != nil {
t.Fatalf("error creating challenge: %s", err)
}
storedChallenge := Challenge(bogusChallenge)
err = ccd.Verify(storedChallenge.String(), ccd.Type, ccd.Origin)
err = ccd.Verify(bogusChallenge.String(), ccd.Type, ccd.Origin)
if err == nil {
t.Fatalf("error expected but not received. expected %#v got %#v", Challenge(ccd.Challenge), storedChallenge)
t.Fatalf("error expected but not received. expected %#v got %#v", ccd.Challenge, bogusChallenge)
}
}
4 changes: 2 additions & 2 deletions protocol/credential_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ func TestParsedCredentialCreationData_Verify(t *testing.T) {
Raw CredentialCreationResponse
}
type args struct {
storedChallenge Challenge
storedChallenge URLEncodedBase64
verifyUser bool
relyingPartyID string
relyingPartyOrigin string
Expand Down Expand Up @@ -221,7 +221,7 @@ func TestParsedCredentialCreationData_Verify(t *testing.T) {
},
},
args: args{
storedChallenge: byteChallenge,
storedChallenge: URLEncodedBase64(byteChallenge),
verifyUser: false,
relyingPartyID: `webauthn.io`,
relyingPartyOrigin: `https://webauthn.io`,
Expand Down
4 changes: 2 additions & 2 deletions protocol/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ type CredentialAssertion struct {
// In order to create a Credential via create(), the caller specifies a few parameters in a CredentialCreationOptions object.
// See §5.4. Options for Credential Creation https://www.w3.org/TR/webauthn/#dictionary-makecredentialoptions
type PublicKeyCredentialCreationOptions struct {
Challenge Challenge `json:"challenge"`
Challenge URLEncodedBase64 `json:"challenge"`
RelyingParty RelyingPartyEntity `json:"rp"`
User UserEntity `json:"user"`
Parameters []CredentialParameter `json:"pubKeyCredParams,omitempty"`
Expand All @@ -30,7 +30,7 @@ type PublicKeyCredentialCreationOptions struct {
// Its challenge member MUST be present, while its other members are OPTIONAL.
// See §5.5. Options for Assertion Generation https://www.w3.org/TR/webauthn/#assertion-options
type PublicKeyCredentialRequestOptions struct {
Challenge Challenge `json:"challenge"`
Challenge URLEncodedBase64 `json:"challenge"`
Timeout int `json:"timeout,omitempty"`
RelyingPartyID string `json:"rpId,omitempty"`
AllowedCredentials []CredentialDescriptor `json:"allowCredentials,omitempty"`
Expand Down
4 changes: 2 additions & 2 deletions protocol/options_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import (

func TestPublicKeyCredentialRequestOptions_GetAllowedCredentialIDs(t *testing.T) {
type fields struct {
Challenge Challenge
Challenge URLEncodedBase64
Timeout int
RelyingPartyID string
AllowedCredentials []CredentialDescriptor
Expand All @@ -22,7 +22,7 @@ func TestPublicKeyCredentialRequestOptions_GetAllowedCredentialIDs(t *testing.T)
{
"Correct Credential IDs",
fields{
Challenge: Challenge([]byte{0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}),
Challenge: URLEncodedBase64([]byte{0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}),
Timeout: 60,
AllowedCredentials: []CredentialDescriptor{
{
Expand Down
3 changes: 1 addition & 2 deletions webauthn/login.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package webauthn

import (
"bytes"
"encoding/base64"
"net/http"

"github.com/go-webauthn/webauthn/protocol"
Expand Down Expand Up @@ -64,7 +63,7 @@ func (webauthn *WebAuthn) beginLogin(userID []byte, allowedCredentials []protoco
}

sessionData := SessionData{
Challenge: base64.RawURLEncoding.EncodeToString(challenge),
Challenge: challenge.String(),
UserID: userID,
AllowedCredentialIDs: requestOptions.GetAllowedCredentialIDs(),
UserVerification: requestOptions.UserVerification,
Expand Down
3 changes: 1 addition & 2 deletions webauthn/registration.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package webauthn

import (
"bytes"
"encoding/base64"
"net/http"

"github.com/go-webauthn/webauthn/protocol"
Expand Down Expand Up @@ -57,7 +56,7 @@ func (webauthn *WebAuthn) BeginRegistration(user User, opts ...RegistrationOptio

response := protocol.CredentialCreation{Response: creationOptions}
newSessionData := SessionData{
Challenge: base64.RawURLEncoding.EncodeToString(challenge),
Challenge: challenge.String(),
UserID: user.WebAuthnID(),
UserVerification: creationOptions.AuthenticatorSelection.UserVerification,
}
Expand Down

0 comments on commit 6abd351

Please sign in to comment.