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

some certificate fingerprints cause decoding error #6524

Closed
diracdeltas opened this issue Jan 4, 2017 · 5 comments
Closed

some certificate fingerprints cause decoding error #6524

diracdeltas opened this issue Jan 4, 2017 · 5 comments
Labels
bug QA/checked-Linux QA/checked-Win32 QA/checked-Win64 QA/test-plan-specified release-notes/include
Milestone
0.13.2

Comments

@diracdeltas
Copy link
Member

reported by eric lawrence:
image uploaded from ios
@diracdeltas diracdeltas added the bug label Jan 4, 2017
diracdeltas added a commit that referenced this issue Jan 4, 2017
@diracdeltas
Handle cert fingerprints that contain '/'
9b9ea7f 
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
@diracdeltas diracdeltas mentioned this issue Jan 4, 2017
Merged
@bsclifton
Copy link
Member

@diracdeltas is this fixed by #6525?

@diracdeltas
Copy link
Member Author

i'm not 100% sure due to lack of the test certificate, but we can close this and reopen if not.

@bsclifton
Copy link
Member

bsclifton commented Jan 13, 2017 •
edited by luixxiul
Loading

Closing; if not fixed by #6525, let's re-open 😄

Test Plan

At least to do a regression test:

  1. go to a site like https://wrong.host.badssl.com/ with an invalid ssl cert
  2. click 'view certificate'
  3. the hex-encoded certificate fingerprint should appear

@luixxiul luixxiul added this to the 0.13.1 milestone Jan 13, 2017
@bsclifton
Copy link
Member

was gonna add the new QA/required tag but you beat me to it, @luixxiul 😄

bsclifton pushed a commit that referenced this issue Jan 14, 2017
@diracdeltas @bsclifton
Handle cert fingerprints that contain '/'
a5d1601 
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
bsclifton pushed a commit that referenced this issue Jan 17, 2017
@diracdeltas @bsclifton
Handle cert fingerprints that contain '/'
77e843d 
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
bsclifton pushed a commit that referenced this issue Jan 17, 2017
@diracdeltas @bsclifton
Handle cert fingerprints that contain '/'
afc6741 
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
bsclifton pushed a commit that referenced this issue Jan 18, 2017
@diracdeltas @bsclifton
Handle cert fingerprints that contain '/'
c853b90 
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
bsclifton pushed a commit that referenced this issue Jan 20, 2017
@diracdeltas @bsclifton
Handle cert fingerprints that contain '/'
1ec46f8 
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
NejcZdovc pushed a commit to NejcZdovc/browser-laptop that referenced this issue Jan 23, 2017
@diracdeltas @NejcZdovc
Handle cert fingerprints that contain '/'
631d378 
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 brave#6524.

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

Auditors: @darkdh
bsclifton pushed a commit that referenced this issue Jan 23, 2017
@diracdeltas @bsclifton
Handle cert fingerprints that contain '/'
4662735 
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
bsclifton pushed a commit that referenced this issue Jan 24, 2017
@diracdeltas @bsclifton
Handle cert fingerprints that contain '/'
0ce3709 
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
bsclifton pushed a commit that referenced this issue Jan 25, 2017
@diracdeltas @bsclifton
Handle cert fingerprints that contain '/'
d935623 
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
bsclifton pushed a commit that referenced this issue Jan 25, 2017
@diracdeltas @bsclifton
Handle cert fingerprints that contain '/'
5f07823 
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
This was referenced Feb 3, 2017
Closed
Closed
Closed
Closed
@bbondy bbondy mentioned this issue Feb 3, 2017
Closed
@bbondy
Copy link
Member

bbondy commented Feb 3, 2017

Test failed on macOS, I posed a follow up here:
#7042

@diracdeltas could you take a look?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
bug QA/checked-Linux QA/checked-Win32 QA/checked-Win64 QA/test-plan-specified release-notes/include
Projects
None yet
Milestone
0.13.2
Development

No branches or pull requests
6 participants
@aekeus @diracdeltas @bbondy @luixxiul @bsclifton @srirambv