-
Notifications
You must be signed in to change notification settings - Fork 82
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
fix(protocol): use strict base64 encoding #129
base: master
Are you sure you want to change the base?
Conversation
This ensures the strict version of base64 is used for all encoding/decoding.
2bBvMMTz0HWhb_mva-EfeoojgjhlmJ02hY8OEPi2yUU should be instead: |
I asked about this a few months ago and got an official response from FIDO devs group |
Sorry but this is not related. This only enforces the use of zero'd padding bits during decoding as per RFC4648 Section 3.5: Canonical Encoding, not the padding or URL encoding. The encoding issues were already addressed months ago in 0.6.0, 0.7.0, and 0.7.1, and are as far as I am aware are entirely in-line with the relevant standards sections (to be linked). Should also point out From 3. Dependencies see Base64url encoding:
Specific Excerpts:
From 5.1.3. Create a New Credential - PublicKeyCredential’s Create Method:
Please feel free to open a discussion if you want to discuss this further. |
If you can provide evidence via the webauthn and referenced specifications that I'm wrong feel free to open an issue, otherwise you can open a discussion where I can more clearly explain it. Just because worked previously with your code does not mean it was actually correct. The current code works with popular well maintained libraries such as SimpleWebauthn, and passes conformance testing.
I mean.. I could be wrong. But the specifications I linked seem crystal clear that I'm not from my perspective, when you add in the fact that the I would not have expected the go developers to have made the same mistake for so long either: import (
"encoding/base64"
"testing"
"github.com/stretchr/testify/assert"
)
func TestBase64URL(t *testing.T) {
testCases := []struct {
name string
have string
encoding *base64.Encoding
expected string
}{
{
"ShouldDecodeWithUnderscoresAndHyphens",
"2bBvMMTz0HWhb_mva-EfeoojgjhlmJ02hY8OEPi2yUU",
base64.RawURLEncoding,
"",
},
{
"ShouldErrorWithPlusAndSlash",
"2bBvMMTz0HWhb/mva+EfeoojgjhlmJ02hY8OEPi2yUU=",
base64.RawURLEncoding,
"illegal base64 data at input byte 13",
},
{
"ShouldErrorWithPlusAndSlashWithoutPadding",
"2bBvMMTz0HWhb/mva+EfeoojgjhlmJ02hY8OEPi2yUU",
base64.RawURLEncoding,
"illegal base64 data at input byte 13",
},
{
"ShouldErrorWithPlusAndSlashURLEncoding",
"2bBvMMTz0HWhb/mva+EfeoojgjhlmJ02hY8OEPi2yUU=",
base64.URLEncoding,
"illegal base64 data at input byte 13",
},
{
"ShouldErrorWithPlusAndSlashWithoutPaddingURLEncoding",
"2bBvMMTz0HWhb/mva+EfeoojgjhlmJ02hY8OEPi2yUU",
base64.URLEncoding,
"illegal base64 data at input byte 13",
},
}
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
_, err := tc.encoding.DecodeString(tc.have)
if len(tc.expected) == 0 {
assert.NoError(t, err)
} else {
assert.EqualError(t, err, tc.expected)
}
})
}
} |
This ensures the strict version of base64 is used for all encoding/decoding.