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

ignore sig_id sent down from server #14335

Merged
merged 4 commits into from Oct 19, 2018

Conversation

Projects
None yet
2 participants
@maxtaco
Copy link
Contributor

commented Oct 19, 2018

  • this is a follow on to an earlier bug with signature chain replay, in which a badLinks table was indexed by a sketchy sigID
  • we left sigIDs in for the way-less-risky badWhitespace links, but it still can leak out into other parts of the program.
  • this PR removes all untrusted sig_ids sent down from the server, and will only read into unpacked.sigID on retrieval of objects from disk
  • otherwise, we derive the sigID from the verifiable data (the signature)
ignore sig_id sent down from server
- this is a follow on to an earlier bug with signature chain replay, in which a badLinks table was indexed by a sketchy sigID
- we left sigIDs in for the way-less-risky badWhitespace links, but it still can leak out into other parts of the program.
- this PR removes all untrusted sig_ids sent down from the server, and will only read into unpacked.sigID on retrieved of objects from disk
- otherwise, we derive the sigID from the verifiable data (the signature)

@maxtaco maxtaco requested review from patrickxb and heronhaye Oct 19, 2018

@@ -909,6 +899,19 @@ func (c *ChainLink) Unpack(m MetaContext, trusted bool, selfUID keybase1.UID, pa
if i, err := jsonparserw.GetInt(packed, "disk_version"); err == nil {
c.diskVersion = int(i)
}

// It is not acceptable to digest sig_id from the server, but we do derived it

This comment has been minimized.

Copy link
@patrickxb

patrickxb Oct 19, 2018

Contributor

derived -> derive

maxtaco added some commits Oct 19, 2018

@@ -522,7 +522,7 @@ const resetReason = "hardcoded reset"

var hardcodedResets = map[keybase1.LinkID]SpecialChainLink{
"f6dae096194690cfee8974b0e10a99ecac2cc8e4f9383516a1f626f614e566e0": SpecialChainLink{UID: keybase1.UID("2d5c41137d7d9108dbdaa2160ba7e200"), Seqno: keybase1.Seqno(11), Reason: resetReason},
"8d7c1a0c99186f972afc5d3624aca2f88ddc3a5dbf84e826ef0b520c31a78aa3": SpecialChainLink{UID: keybase1.UID("f1c263462dd526695c458af924977719"), Seqno: keybase1.Seqno(18), Reason: resetReason},
"8d7c1a0c99186f972afc5d3624aca2f88ddc3a5dbf84e826ef0b520c31a78aa3": SpecialChainLink{UID: keybase1.UID("f1c263462dd526695c458af924977719"), Seqno: keybase1.Seqno(8), Reason: resetReason},

This comment has been minimized.

Copy link
@maxtaco

maxtaco Oct 19, 2018

Author Contributor

I couldn't ID this user due to this typo.

@maxtaco maxtaco merged commit e35ffcb into master Oct 19, 2018

2 checks passed

ci/circleci Your tests passed on CircleCI!
Details
continuous-integration/jenkins/pr-head This commit looks good
Details

@maxtaco maxtaco deleted the maxtaco/CORE-9188 branch Oct 19, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.