Opened 5 years ago

Last modified 2 weeks ago

#12399 merge_ready defect

"Hash of session info was not as expected" should be log_protocol_warn

Reported by: ln5 Owned by:
Priority: Medium Milestone: Tor: 0.4.0.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: tor-hs easy logging 035-backport 040-backport 041-backport consider-backport-after-0421
Cc: asn Actual Points:
Parent ID: Points: .1
Reviewer: mikeperry, dgoulet Sponsor:

Description

Seeing

[warn] Hash of session info was not as expected.

on fast relays, both exits and non exits (ndnr1, DFRI0, DFRI2) several times today. First one spotted at Jun 14 00:26 CEST.

These are on Linux and FreeBSD, versions 0.2.5.4-alpha-dev and 0.2.5.2-alpha respectively.

Child Tickets

Change History (20)

comment:1 Changed 5 years ago by nickm

Milestone: Tor: 0.2.5.x-final

comment:2 Changed 5 years ago by arma

Comes from

/** Respond to an ESTABLISH_INTRO cell by checking the signed data and
 * setting the circuit's purpose and service pk digest.
 */
int
rend_mid_establish_intro()
  if (tor_memneq(expected_digest, request+2+asn1len, DIGEST_LEN)) {
    log_warn(LD_PROTOCOL, "Hash of session info was not as expected.");
    reason = END_CIRC_REASON_TORPROTOCOL;

Sounds like somebody somewhere in the world is working on implementing hidden service client support, and didn't do it right in this case.

The simple fix is to make this a LOG_PROTOCOL_WARN so it doesn't bother normal operators with bugs on the client side.

Another option though is that there actually is a bug in the mainline Tor client.

comment:3 Changed 5 years ago by nickm

Keywords: 025-backport added
Milestone: Tor: 0.2.5.x-finalTor: 0.2.6.x-final

comment:4 Changed 5 years ago by ln5

FWIW, I haven't seen this since Jun 20. Someone tried something for six days?

comment:5 Changed 5 years ago by nickm

Milestone: Tor: 0.2.6.x-finalTor: 0.2.???

comment:6 Changed 4 years ago by Sebastian

Saw this on my relay today.

comment:7 Changed 3 years ago by teor

Milestone: Tor: 0.2.???Tor: 0.3.???

Milestone renamed

comment:8 Changed 3 years ago by nickm

Keywords: tor-03-unspecified-201612 added
Milestone: Tor: 0.3.???Tor: unspecified

Finally admitting that 0.3.??? was a euphemism for Tor: unspecified all along.

comment:9 Changed 2 years ago by nickm

Keywords: tor-03-unspecified-201612 removed

Remove an old triaging keyword.

comment:10 Changed 2 years ago by nickm

Keywords: 025-backport removed

These are not ripe for 0.2.5, even if they do get fixed now

comment:11 Changed 2 years ago by nickm

Keywords: tor-hs easy logging added
Points: .1
Severity: Normal
Summary: Hash of session info was not as expected"Hash of session info was not as expected" should be log_protocol_warn

The right fix, given the timeline, is probably "log_protocol_warn".

comment:12 Changed 2 months ago by rl1987

Status: newneeds_review

comment:13 Changed 2 months ago by nickm

Milestone: Tor: unspecifiedTor: 0.4.2.x-final

comment:14 Changed 2 months ago by dgoulet

Reviewer: mikeperry

comment:15 Changed 3 weeks ago by dgoulet

Cc: asn added
Reviewer: mikeperrymikeperry, dgoulet
Status: needs_reviewmerge_ready

I think this is the right thing to do. Anyone can send bad cell content and trigger warnings by not following the tor protocol.

This is one of those instance so protowarn is legit.

comment:16 Changed 3 weeks ago by nickm

Should this be marked for backport?

comment:17 in reply to:  16 Changed 3 weeks ago by dgoulet

Replying to nickm:

Should this be marked for backport?

I believe we could. I guess it can fall under (from https://trac.torproject.org/projects/tor/wiki/org/teams/NetworkTeam/CoreTorReleases):

    Misleading documentation will get fixed.
    Smaller bugs that significantly impact user experience will get fixed. 

comment:18 Changed 2 weeks ago by nickm

Okay, I've made a branch ticket12399_035 that cherry-picks the commit from the original branch back on top of master. I've merged it to 0.4.1 and forward, and am marking it for backport. Its PR is https://github.com/torproject/tor/pull/1284

comment:19 Changed 2 weeks ago by nickm

Milestone: Tor: 0.4.2.x-finalTor: 0.4.0.x-final

comment:20 Changed 2 weeks ago by teor

Keywords: 035-backport 040-backport 041-backport consider-backport-after-0421 added

We'll backport this patch after it has had some testing in master.
It's very low risk, so if it needs to go sooner, let me know.

Note: See TracTickets for help on using tickets.