CPPKI: Do not double base64 encode #3549
Conversation
There was a problem hiding this comment.
Reviewed 40 of 40 files at r1.
Reviewable status: all files reviewed, 7 unresolved discussions (waiting on @oncilla)
go/lib/infra/modules/trust/v2/trustdbsqlite/db.go, line 222 at r1 (raw file):
defer e.Unlock() h := hash([]byte(d.Signed.EncodedTRC))
I think it would be cleaner if the conversion to the byte slice happens in the hash function itself.
go/lib/scrypto/cert/v2/as_signed_test.go, line 78 at r1 (raw file):
valid[:len(valid)/2]
just that should be enough
go/lib/scrypto/cert/v2/as_signed_test.go, line 143 at r1 (raw file):
valid[:len(valid)/2]
just that should be enough
go/lib/scrypto/cert/v2/issuer_signed_test.go, line 78 at r1 (raw file):
valid[:len(valid)/2]
just that should be enough
go/lib/scrypto/cert/v2/issuer_signed_test.go, line 144 at r1 (raw file):
valid[:len(valid)/2]
just that should be enough
go/lib/scrypto/trc/v2/signed_test.go, line 79 at r1 (raw file):
valid[:len(valid)/2]
just that should be enough
go/lib/scrypto/trc/v2/signed_test.go, line 144 at r1 (raw file):
valid[:len(valid)/2]
just that should be enough
We use the JWS standard that prescribes that the payload and protected
metadata must be encoded as base 64 raw url safe encoding.
In the go code, we have typed byte slices with respective encode and
decode functions because the json library only allows using the standard
base64 encoding.
Helpfully, when calling json.{Unm,M}arshal, byte slices are encoded in
base64 automatically. Which results in double encoding in our case.
This PR changes the encoded types to string types to prevent the
json library from encoding the data.
oncilla
force-pushed
the
pub-cppki-double-base64
branch
from
e27ed64 to
442b142
Compare
There was a problem hiding this comment.
Reviewable status: 14 of 48 files reviewed, 7 unresolved discussions (waiting on @lukedirtwalker)
go/lib/infra/modules/trust/v2/trustdbsqlite/db.go, line 222 at r1 (raw file):
Previously, lukedirtwalker (Lukas Vogel) wrote…
I think it would be cleaner if the conversion to the byte slice happens in the hash function itself.
In that case I need to cast everything to string first, as typed strings cannot be passed to string argument.
go/lib/scrypto/cert/v2/as_signed_test.go, line 78 at r1 (raw file):
Previously, lukedirtwalker (Lukas Vogel) wrote…
valid[:len(valid)/2]just that should be enough
valid[:len(valid)/2] is not guaranteed to be valid base64, so we might not even get to the branch we try to cover.
if you look at the coverage report, it will shrink with the proposed change.
Made it a bit prettier
go/lib/scrypto/cert/v2/as_signed_test.go, line 143 at r1 (raw file):
Previously, lukedirtwalker (Lukas Vogel) wrote…
valid[:len(valid)/2]just that should be enough
Done.
go/lib/scrypto/cert/v2/issuer_signed_test.go, line 78 at r1 (raw file):
Previously, lukedirtwalker (Lukas Vogel) wrote…
valid[:len(valid)/2]just that should be enough
Done.
go/lib/scrypto/cert/v2/issuer_signed_test.go, line 144 at r1 (raw file):
Previously, lukedirtwalker (Lukas Vogel) wrote…
valid[:len(valid)/2]just that should be enough
Done.
go/lib/scrypto/trc/v2/signed_test.go, line 79 at r1 (raw file):
Previously, lukedirtwalker (Lukas Vogel) wrote…
valid[:len(valid)/2]just that should be enough
Done.
go/lib/scrypto/trc/v2/signed_test.go, line 144 at r1 (raw file):
Previously, lukedirtwalker (Lukas Vogel) wrote…
valid[:len(valid)/2]just that should be enough
Done.
There was a problem hiding this comment.
Reviewed 34 of 34 files at r2.
Reviewable status:complete! all files reviewed, all discussions resolved
go/lib/infra/modules/trust/v2/trustdbsqlite/db.go, line 222 at r1 (raw file):
Previously, Oncilla wrote…
In that case I need to cast everything to string first, as typed strings cannot be passed to string argument.
:sad_panda:
We use the JWS standard that prescribes that the payload and protected
metadata must be encoded as base 64 raw url safe encoding.
In the go code, we have typed byte slices with respective encode and
decode functions because the json library only allows using the standard
base64 encoding.
Helpfully, when calling json.{Unm,M}arshal, byte slices are encoded in
base64 automatically. Which results in double encoding in our case.
This PR changes the encoded types to string types to prevent the
json library from encoding the data.
This change is