Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

Handle cert fingerprints that contain '/' #6525

Merged
merged 1 commit into from
Jan 13, 2017
Merged

Conversation

diracdeltas
Copy link
Member

@diracdeltas diracdeltas commented Jan 4, 2017

The cert fingerprint emitted by Muon/Electron is base64 encoded [1], so it may contain '/'. I think we were truncating the certificate fingerprint when this was the case, which may have caused the decoding error in #6524.

[1] https://cs.chromium.org/chromium/src/net/base/hash_value.h?q=net::HashValue&sq=package:chromium&l=58

Should fix #6524

Auditors: @darkdh

The cert fingerprint emitted by Muon/Electron is base64 encoded [1], so it may contain '/'. I think we were truncating the certificate fingerprint when this was the case, which may have caused the decoding error in #6524.

[1] https://cs.chromium.org/chromium/src/net/base/hash_value.h?q=net::HashValue&sq=package:chromium&l=58

Auditors: @darkdh
Copy link
Member

@darkdh darkdh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm!
Can we ask Eric Lawrence to provide the url of the site whose certificate contains "/" fingerprint in base64? Just wanna record it in this PR.

@diracdeltas
Copy link
Member Author

^ pinging @ericlaw1979

@luixxiul luixxiul added this to the 0.13.1 milestone Jan 6, 2017
@ericlaw1979
Copy link

ericlaw1979 commented Jan 6, 2017

Can we ask Eric Lawrence to provide the url of the site whose certificate contains "/" fingerprint in base64? Just wanna record it in this PR.

This wasn't a live site, but instead a self-generated certificate (like one a bad guy could generate for his own server).

My repro used Fiddler. Enable HTTPS decryption, then click
Rules > Customize Rules. Inside OnBeforeRequest, set the desired CN value, e.g.

static function OnBeforeRequest(oSession: Session) { oSession["X-overridecertCN"]= "<b>alert(1)";

Save the file, then visit any HTTPS site to have the self-generated certificate with the bad CN value returned to the client.

@bsclifton bsclifton changed the base branch from master to 0.13.1-branch January 13, 2017 18:49
@bsclifton
Copy link
Member

Last travis-ci run had an error; I re-ran this test locally and it's working as expected 😄 Merging...

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants