Opened 4 weeks ago

Closed 3 weeks ago

#31460 closed defect (fixed)

Don't reveal proxy IDs in broker /debug

Reported by: phw Owned by: cohosh
Priority: Medium Milestone:
Component: Circumvention/Snowflake Version:
Severity: Normal Keywords:
Cc: serna, arlolra, cohosh, phw, dcf Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

We just had the following discussion on IRC.

serna> If there are two proxies with the same sessionID
serna> When the broker does the proxyAnswers it does the idToSnowflake which proxy would it return?
serna> Suppose I'm an attacker, I would go to the broker's /debug page, scrape all the IDs and start sending requests to /proxy with those IDs continuously
  phw> cohosh, dcf1: ^
  phw> that's an interesting point. i'm not familiar enough with the code to answer this question but i'll forward it to snowflake's maintainers
serna> phw: I did a little PoC with two proxies sending the same id and the broker didnt care, but the dangerous part is when an offer is accepted by the proxy and it sends the answer
[...]
  phw> serna: this would effectively be a DoS issue, right? it may allow you to disable a given proxy.
serna> phw: yes I believe it would be. If it works like I think it could disable every proxy connected to the broker

Is this an issue in our broker implementation?

Child Tickets

Change History (6)

comment:1 Changed 4 weeks ago by dcf

Summary: Can attackers disable proxies by using their ID?Don't reveal proxy IDs in broker /debug

Yes, I think it is a security bug that /debug reveals proxy IDs. We should be scrubbing those somehow, by reporting xxxxxxxx, hashing them, or just reporting a total count.

comment:2 Changed 4 weeks ago by cohosh

Owner: set to cohosh
Status: newassigned

Thanks for this. The metrics team also expressed discomfort at displaying the round trip time average here.

I propose a short term fix of only displaying an available snowflake count on the debug page, and a long term plan of displaying the graphs produced by the safe metrics data. The only thing we'd be missing which is somewhat useful if we went to just counts would be the number of proxy-go vs. webextension snowflakes which is visible in the lengths of the IDs. I'll see if there's some way I can capture that still.

comment:3 Changed 4 weeks ago by cohosh

Here's a fix that does the short term solution described above: https://github.com/cohosh/snowflake/pull/6

I want to note that distinguishing between browser proxies and standalones is somewhat of a hack and I think that's okay for now. The browser-based proxies generate IDs as follows:

  static genSnowflakeID() {
    return Math.random().toString(36).substring(2);
  }

The standalone instances generate IDs as follows:

func genSessionID() string {
        buf := make([]byte, sessionIDLength)
        _, err := rand.Read(buf)
        if err != nil {
                panic(err.Error())
        }
        return strings.TrimRight(base64.StdEncoding.EncodeToString(buf), "=")
}

The strings for both are of variable length. The browser-based proxy ID length depends on the IEEE 754 floating point number chosen and the standalone ID length depends on what the 16 byte session ID's base64 encoding is (note: this is generally more than 16 characters long and the browser proxies are typically less than 12). I think 16 bytes is a reasonable cutoff.

comment:4 Changed 4 weeks ago by cohosh

Status: assignedneeds_review

comment:5 Changed 3 weeks ago by dcf

Status: needs_reviewmerge_ready

It works for me.

comment:6 Changed 3 weeks ago by cohosh

Resolution: fixed
Status: merge_readyclosed

Merged in 00eb4aadf5 and deployed as of 2019/08/27 14:06:07

Note: See TracTickets for help on using tickets.