Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Suborigin header must be lowercase #3209

Closed
hobofan opened this issue Sep 12, 2016 · 14 comments
Closed

Suborigin header must be lowercase #3209

hobofan opened this issue Sep 12, 2016 · 14 comments
Labels
help wanted Seeking public contribution on this issue kind/bug A bug in existing code (including security flaws) status/deferred Conscious decision to pause or backlog topic/gateway Topic gateway topic/security Topic security

Comments

@hobofan
Copy link

hobofan commented Sep 12, 2016

Version/Platform/Processor information (from ipfs version --all):

go-ipfs version: 0.4.4-dev-0e2b4eb
Repo version: 4
System version: amd64/linux
Golang version: go1.7.1

Type (bug, feature, meta, test failure, question): feature/bug
Area (api, commands, daemon, fuse, etc): gateway
Priority (from P0: operations on fire, to P4: functioning): P2

Description:

Using Chromium (Version 53.0.2785.101 (64-bit)) and the current Chrome for Android version I get the error Error with Suborigin header: Invalid character 'Q' in suborigin. when opening any path on the gateway. It appears that this error stops any JS on the site from executing. I am testing it with a single page web app that works fine when the assets are served from a static webserver (e.g. python -m SimpleHTTPServer).

The request as made with curl:

curl https://ipfs.io/ipfs/QmSA76DiRmaBGDvW2J3CQtS94tC7UdLouKV11b4QWnTG9f/ --verbose
*   Trying 104.236.179.241...
* TCP_NODELAY set
* Connected to ipfs.io (104.236.179.241) port 443 (#0)
* ALPN, offering http/1.1
* Cipher selection: ALL:!EXPORT:!EXPORT40:!EXPORT56:!aNULL:!LOW:!RC4:@STRENGTH
* successfully set certificate verify locations:
*   CAfile: /etc/ssl/certs/ca-certificates.crt
  CApath: none
* TLSv1.2 (OUT), TLS header, Certificate Status (22):
* TLSv1.2 (OUT), TLS handshake, Client hello (1):
* TLSv1.2 (IN), TLS handshake, Server hello (2):
* NPN, negotiated HTTP1.1
* TLSv1.2 (IN), TLS handshake, Certificate (11):
* TLSv1.2 (IN), TLS handshake, Server key exchange (12):
* TLSv1.2 (IN), TLS handshake, Server finished (14):
* TLSv1.2 (OUT), TLS handshake, Client key exchange (16):
* TLSv1.2 (OUT), TLS change cipher, Client hello (1):
* TLSv1.2 (OUT), TLS handshake, Unknown (67):
* TLSv1.2 (OUT), TLS handshake, Finished (20):
* TLSv1.2 (IN), TLS change cipher, Client hello (1):
* TLSv1.2 (IN), TLS handshake, Finished (20):
* SSL connection using TLSv1.2 / ECDHE-RSA-AES128-GCM-SHA256
* ALPN, server did not agree to a protocol
* Server certificate:
*  subject: C=US; ST=California; L=San Francisco; O=Protocol Labs, Inc.; CN=ipfs.io
*  start date: May  2 23:18:54 2016 GMT
*  expire date: May  2 23:18:54 2019 GMT
*  subjectAltName: host "ipfs.io" matched cert's "ipfs.io"
*  issuer: C=IL; O=StartCom Ltd.; OU=StartCom Certification Authority; CN=StartCom Class 3 OV Server CA
*  SSL certificate verify ok.
> GET /ipfs/QmSA76DiRmaBGDvW2J3CQtS94tC7UdLouKV11b4QWnTG9f/ HTTP/1.1
> Host: ipfs.io
> User-Agent: curl/7.50.2
> Accept: */*
> 
< HTTP/1.1 200 OK
< Date: Mon, 12 Sep 2016 12:11:21 GMT
< Content-Type: text/html; charset=utf-8
< Content-Length: 609
< Connection: keep-alive
< Accept-Ranges: bytes
< Access-Control-Allow-Headers: X-Stream-Output, X-Chunked-Output
< Access-Control-Allow-Methods: GET
< Access-Control-Allow-Origin: *
< Access-Control-Expose-Headers: X-Stream-Output, X-Chunked-Output
< Cache-Control: public, max-age=29030400, immutable
< Etag: QmSA76DiRmaBGDvW2J3CQtS94tC7UdLouKV11b4QWnTG9f
< Last-Modified: Thu, 01 Jan 1970 00:00:01 GMT
< Suborigin: QmSA76DiRmaBGDvW2J3CQtS94tC7UdLouKV11b4QWnTG9f
< X-Ipfs-Path: /ipfs/QmSA76DiRmaBGDvW2J3CQtS94tC7UdLouKV11b4QWnTG9f/
< Strict-Transport-Security: max-age=15768000; includeSubDomains; preload
< 
* Curl_http_done: called premature == 0
* Connection #0 to host ipfs.io left intact
<!doctype html><html lang="en"><head><title>React Redux Starter Kit</title><meta charset="utf-8"><meta name="viewport" content="width=device-width,initial-scale=1"><!-- <link rel="stylesheet" href="https://maxcdn.bootstrapcdn.com/bootstrap/3.3.6/css/bootstrap.min.css"> --><link rel="shortcut icon" href="favicon.ico"><link href="app.fe38df8410d0f61eaeed7f6efb62db7b.css" rel="stylesheet"></head><body><div id="root" style="height: 100%"></div><script type="text/javascript" src="vendor.a61f8c39b69bacbfc7e3.js"></script><script type="text/javascript" src="app.459e2a80c10fe5a96640.js"></script></body></html>%                                                                      

From what I can gather from #651 the suborigin header is still in a very experimental stage on both the IPFS and the browser side. Thus I would propose a config flag that disables the Suborigin header being sent from the gateway, so it can be turned off until it is more stabilized.


This is for you! Please read, and then delete this text before posting it.
The go-ipfs issues are only for bug reports and directly actionable features.
Check https://github.com/ipfs/community/blob/master/contributing.md#reporting-issues if that doesn't fit.
Check https://github.com/ipfs/go-ipfs/blob/master/docs/github-issue-guide.md if you are not sure how to fill this issue.

@ghost ghost added kind/bug A bug in existing code (including security flaws) topic/gateway Topic gateway topic/security Topic security labels Sep 12, 2016
@ghost
Copy link

ghost commented Sep 12, 2016

wtf chromium!!! -- thanks for reporting :) I'll write up some IPFS feedback for the Suborigin working group soon and will take a look.

@ghost ghost self-assigned this Sep 12, 2016
@whyrusleeping whyrusleeping added the help wanted Seeking public contribution on this issue label Sep 14, 2016
@jbenet
Copy link
Member

jbenet commented Sep 15, 2016

Wow, that standard is going to be a wasted opportunity if we don't step in, @lgierth thank you-- will be a huge help if we can get Suborigins to behave like we need them to (basically like a different domain, and use any valid path)

@whyrusleeping
Copy link
Member

@lgierth Any update on the suborigin writeup? Lets not let this one slip away and be forgotten

@ghost
Copy link

ghost commented Sep 20, 2016

Right now I'm reading through the mailing list and current draft: https://w3c.github.io/webappsec-suborigins/

Tracking my progress in ipfs/specs#131

@ghost
Copy link

ghost commented Sep 21, 2016

Okay I think we can fix this by using HSHCA for the suborigin value, in order to make it lower case.

@ghost ghost changed the title Add config option to turn off Suborigin response header of gateway Suborigin header must be lowercase Sep 21, 2016
@Kubuxu
Copy link
Member

Kubuxu commented Sep 21, 2016

Please see my other comment under the specs repo but in short therms: we should encode whole two segments of the path not only the multihash so it also works for ipns and other future protocols (ipld, view transformations and so on).

@flyingzumwalt flyingzumwalt added the status/deferred Conscious decision to pause or backlog label Sep 26, 2016
@ghost
Copy link

ghost commented Sep 29, 2016

The other day a discussion started about further restricting the characters allowed for suborigin names, and I expressed a preference for "ban starting with a number"/"disallow numerals" over "only lowercase a-z".

w3c/webappsec-suborigins#47

@OmgImAlexis
Copy link

I'm also getting this on the /ipfs/webui path in Chrome itself.

@ghost
Copy link

ghost commented Oct 27, 2016

w3c/webappsec-suborigins#62 updates the rules for suborigin names:

LOWERALPHA *( LOWERALPHA / DIGIT )

Note: A suborigin name must start with a lowercase character, but after the
first character, the name may contain lowercase characters or numerals. This
is to avoid potential confusion when the origin is serialized if the
serialization started with a number.

@ghost ghost removed their assignment Oct 27, 2016
@Kubuxu
Copy link
Member

Kubuxu commented Oct 27, 2016

The standard also allows some special values (null).

This means that we probably should have constant prefix so special values can't be hit by encoded hash.

@ghost
Copy link

ghost commented Oct 27, 2016

This means that we probably should have constant prefix so special values can't be hit by encoded hash.

Yes agreed, it could be an unencoded ipfs or cid.

we should encode whole two segments of the path not only the multihash so it also works for ipns and other future protocols (ipld, view transformations and so on).

/ipns and /ipfs are naturally separated because there will never be anything within /ipfs with the same multihash or cid as something within /ipns and vice versa. For anything else that might come in the future, we can apply still apply prefixes without having to change the ones for /ipfs and /ipns. But I really don't have strong feelings about it, and this value is supposed to be opaque anyhow.

dnslink

I can't find it anymore, but someone brought up suborigins and dnslink.

  1. We'll have to figure out how to turn /ipns/doman.tld into a suborigin.
  2. Cases where the gateway uses the Host header and turns it into /ipns/hostvalue don't use suborigin at the moment, and don't need to in the future.

@Kubuxu
Copy link
Member

Kubuxu commented May 10, 2017

To fix it properly we need to decide on suborigin prefix and add cid.EncodeWithBase for chosen base encoding of CIDs.

jes added a commit to jes/go-ipfs that referenced this issue May 10, 2017
This existed before but was disabled in 912a972 because the Suborigin spec
changed and it became incompatible.

This commit updates the generated Suborigin header to be conformant with the
latest spec.

License: MIT
Signed-off-by: James Stanley <[email protected]>
jes added a commit to jes/go-ipfs that referenced this issue May 13, 2017
This existed before but was disabled in 912a972 because the Suborigin spec
changed and it became incompatible.

This commit updates the generated Suborigin header to be conformant with the
latest spec.

License: MIT
Signed-off-by: James Stanley <[email protected]>
dgrisham pushed a commit to dgrisham/go-ipfs that referenced this issue Jun 15, 2017
This existed before but was disabled in 912a972 because the Suborigin spec
changed and it became incompatible.

This commit updates the generated Suborigin header to be conformant with the
latest spec.

License: MIT
Signed-off-by: James Stanley <[email protected]>
dgrisham pushed a commit to dgrisham/go-ipfs that referenced this issue Jun 16, 2017
This existed before but was disabled in 912a972 because the Suborigin spec
changed and it became incompatible.

This commit updates the generated Suborigin header to be conformant with the
latest spec.

License: MIT
Signed-off-by: James Stanley <[email protected]>
@lidel
Copy link
Member

lidel commented Aug 22, 2018

Quick update in the context of move to CIDv1 Base32 (https://github.com/ipfs/ipfs/issues/337, ipfs/in-web-browsers#66 (comment)).

CIDv1

Base32 is case-insensitive, so just prefix and use CID as-is:

Location: https://ipfs.io/ipfs/bafkreigh2akiscaildcqabsyg3dfr6chu3fgpregiymsck7e7aqa4s52zy
Suborigin: ipfs000bafkreigh2akiscaildcqabsyg3dfr6chu3fgpregiymsck7e7aqa4s52zy

CIDv0

This one is bit smelly, right now its just a CIDv0 encoded in Base32:

Location: https://ipfs.io/ipfs/QmbWqxBEKC3P8tqsKc98xmWNzrzDtRLMiMPL8wBuTGsMnR
Suborigin: ipfs000bciqmhrdth3ek77igz6pj75ip7rv42lwiljqxaaclw4ewnhbr32kdsgq

We can leave it as-is, or normalize it to CIDv1 Base32 to have the same string length everywhere.

IPNS

Case-insensitive representation of PeerIDs is an open issue: #5287

@Stebalien
Copy link
Member

This was fixed by simply encoding the raw CID as base32. The new format is: ipfs000base32cid and/or ipns000base32cid.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Seeking public contribution on this issue kind/bug A bug in existing code (including security flaws) status/deferred Conscious decision to pause or backlog topic/gateway Topic gateway topic/security Topic security
Projects
None yet
Development

No branches or pull requests

8 participants