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 bugs in team reverse sigs and beef up testing #67
Conversation
src/subkey.iced
Outdated
json?.body?[@get_field()]?[@sibkid_slot()] | ||
_clear_reverse_sig : (outer) -> | ||
# body.sibkey.reverse_sig should be the only field different between the two | ||
outer?.body?[@get_field()].reverse_sig = null |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Get rid of the ?
's?
src/subkey.iced
Outdated
@@ -66,20 +65,28 @@ exports.SubkeyBase = class SubkeyBase extends Base | |||
await @reverse_sig_check { json, new_km: @get_new_km() }, esc defer() | |||
cb null | |||
|
|||
_get_reverse_sig : (json) -> | |||
json?.body?[@get_field()]?.reverse_sig |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we're in here, should we call get_field
something more descriptive?
@per_team_key = m | ||
get_new_km : () -> @kms?.signing # use the signing KM | ||
sibkid_slot : () -> "signing_kid" | ||
need_reverse_sig : (json) -> json?.body?.per_team_key? | ||
need_reverse_sig : (json) -> json?.body?.team?.per_team_key? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like we're not implementing sibkid_slot
anymore. Is it ok that we inherit the parent implementation that returns "kid"
? It looks like we're signing ourselves up to override all parent methods that call sibkid_slot
, but that sounds like a tricky contract?
await obj2.verify_v2 varg, defer err | ||
if corrupt | ||
T.assert err?, "got an error back" | ||
T.assert (err.message.indexOf('Reverse sig json mismatch') >= 0), "found right error message" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we be asserting anything in the !corrupt
case? It does look like we mean to test that case below.
Thanks for the review, all fixed! |
LGTM thanks |
No description provided.