Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Commit

Permalink
Use https connection (#7503)
Browse files Browse the repository at this point in the history 
Use https when connecting to etherscan.io API for price-info
  • Loading branch information
@computerality @debris
computerality authored and debris committed Jan 9, 2018
1 parent bdc3d6f commit ab7335d
Showing 1 changed file with 2 additions and 2 deletions.
4 changes: 2 additions & 2 deletions price-info/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ impl<F> cmp::PartialEq for Client<F> {
impl<F: Fetch> Client<F> {
/// Creates a new instance of the `Client` given a `fetch::Client`.
pub fn new(fetch: F) -> Client<F> {
let api_endpoint = "http://api.etherscan.io/api?module=stats&action=ethprice".to_owned();
let api_endpoint = "https://api.etherscan.io/api?module=stats&action=ethprice".to_owned();
Client { api_endpoint, fetch }
}

Expand Down Expand Up @@ -144,7 +144,7 @@ mod test {
type Result = FutureResult<fetch::Response, fetch::Error>;
fn new() -> Result<Self, fetch::Error> where Self: Sized { Ok(FakeFetch(None, Default::default())) }
fn fetch_with_abort(&self, url: &str, _abort: fetch::Abort) -> Self::Result {
assert_eq!(url, "http://api.etherscan.io/api?module=stats&action=ethprice");
assert_eq!(url, "https://api.etherscan.io/api?module=stats&action=ethprice");
let mut val = self.1.lock();
*val = *val + 1;
if let Some(ref response) = self.0 {
Expand Down

3 comments on commit ab7335d

@danuker
Copy link

@danuker danuker commented on ab7335d Jun 12, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Privacy note: ~~~~this doesn't do much;~~~~ the MITM still sees that you're connecting to Etherscan.io (and therefore, that you're interested in Ethereum). Edit: They can still see that you're running an Ethereum client, based on connections to other known Ethereum nodes.

However, now they don't know that you're checking the PRICE of Ethereum.

Edit: They are now also unable to alter the response, which is a significant improvement.

@kirushik
Copy link
Collaborator

@kirushik kirushik commented on ab7335d Jun 12, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

@danuker I still find it a valuable improvement — with https enabled we can (to some reasonable extent) rely on authenticity of the prices fetched from Etherscan.
And this authenticity closes, for example, the "price-manipulation" attack vectors, where the client is tricked into sending more money than they should, because some third party was able to alter the content of the Etherscan response mid-flight.

I also think that connecting to Etherscan is far from being the only indicator of the user's interest in Ethereum, given they run a Parity node — and I don't think that it's too hard to determine the fact of running Parity if one have a capability to eavesdrop on the client's traffic.

@danuker
Copy link

Choose a reason for hiding this comment

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

@kirushik Wow! I didn't think of authenticity. You are absolutely right.

You are also right about detection, I suppose it would be easy to tell that a user is running Parity, or any other client, given their traffic. I guess I had a long day and I'm tired for not realizing that.

Please sign in to comment.