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

Admin console sends referrer everywhere #2432

Closed
tobozo opened this Issue Sep 12, 2018 · 10 comments

Comments

Projects
None yet
3 participants
@tobozo

tobozo commented Sep 12, 2018

Found this in functions-html.php <meta name="referrer" content="always" />

As a result the full URL to the admin console is sent every time someone tests a link from there, and this can't be bypassed by HTTP "referrer-policy" headers.

In some circumstances (e.g. no multi factor auth implemented) this can create an attack surface (DDoS, Brute force) by making the admin console address known by the owner of target URL's site.

What are the drawbacks of using <meta name="referrer" content="none" /> in the admin console instead?
Will this break some plugins or core functionalities ?

I've shortened this URL as a practical test.

@ozh

This comment has been minimized.

Show comment
Hide comment
@ozh

ozh Sep 12, 2018

Member

Hmmm. I added this because of #1762 but cannot really understand why now...

Member

ozh commented Sep 12, 2018

Hmmm. I added this because of #1762 but cannot really understand why now...

@tobozo

This comment has been minimized.

Show comment
Hide comment
@tobozo

tobozo Sep 12, 2018

Maybe the HTML template is used outside the admin console, and maybe some parts of the admin console are intended to be exposed (e.g. multi user environment), and in this case the referrer would matter ?

tobozo commented Sep 12, 2018

Maybe the HTML template is used outside the admin console, and maybe some parts of the admin console are intended to be exposed (e.g. multi user environment), and in this case the referrer would matter ?

@dgw

This comment has been minimized.

Show comment
Hide comment
@dgw

dgw Sep 13, 2018

Collaborator

I can't think of a situation in which it would be desirable to send YOURLS' admin console URL as a referrer. Doesn't mean there exists no such situation, just that I can't imagine it. 😸

The value "always" is deprecated for <meta name="referrer"> anyway. I think YOURLS shouldn't use it any more.

If passing along the referrer is important for short URLs, I think that's doable by sending another header along with redirections: Referrer-Policy: unsafe-url (header docs). I need to verify that the header applies to 3xx response codes or when a Location header is also sent, but I don't see why it wouldn't.

Collaborator

dgw commented Sep 13, 2018

I can't think of a situation in which it would be desirable to send YOURLS' admin console URL as a referrer. Doesn't mean there exists no such situation, just that I can't imagine it. 😸

The value "always" is deprecated for <meta name="referrer"> anyway. I think YOURLS shouldn't use it any more.

If passing along the referrer is important for short URLs, I think that's doable by sending another header along with redirections: Referrer-Policy: unsafe-url (header docs). I need to verify that the header applies to 3xx response codes or when a Location header is also sent, but I don't see why it wouldn't.

@tobozo

This comment has been minimized.

Show comment
Hide comment
@tobozo

tobozo Sep 13, 2018

Referrer-Policy: unsafe-url is the way I chose to go along with not using HSTS so the referrer is properly forwarded with all 3xx responses.

But the /admin/ folder requires exactly the opposite, should this be be taken care of in the core instead of web server rules like I did?

tobozo commented Sep 13, 2018

Referrer-Policy: unsafe-url is the way I chose to go along with not using HSTS so the referrer is properly forwarded with all 3xx responses.

But the /admin/ folder requires exactly the opposite, should this be be taken care of in the core instead of web server rules like I did?

@dgw

This comment has been minimized.

Show comment
Hide comment
@dgw

dgw Sep 13, 2018

Collaborator

I was thinking of sending no-referrer on admin pages and unsafe-url with redirects, yes. Mostly waiting for @ozh's input before I actually implement anything.

Collaborator

dgw commented Sep 13, 2018

I was thinking of sending no-referrer on admin pages and unsafe-url with redirects, yes. Mostly waiting for @ozh's input before I actually implement anything.

@ozh

This comment has been minimized.

Show comment
Hide comment
@ozh

ozh Sep 13, 2018

Member

Reading https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Referrer-Policy I think unsafe-url are not ideal either.

To be honest I really don't know what I had in mind back then. Now, I think we should not try to enforce anything regarding referrer and let this open to plugin and user choice (ie not send any header but allow plugins to add any via a filter)

Member

ozh commented Sep 13, 2018

Reading https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Referrer-Policy I think unsafe-url are not ideal either.

To be honest I really don't know what I had in mind back then. Now, I think we should not try to enforce anything regarding referrer and let this open to plugin and user choice (ie not send any header but allow plugins to add any via a filter)

@dgw

This comment has been minimized.

Show comment
Hide comment
@dgw

dgw Sep 13, 2018

Collaborator

Should it be a filter, or an action? A filter implies passing each plugin that registers for the hook an array of headers and allowing removal or modification before YOURLS finally sends the whole batch of headers at once. I think an action is simpler, since it only lets plugins add headers.

Since YOURLS itself will send only basic headers for most requests, allowing plugins to add more should be enough in most cases (and is much simpler to implement than a full-on filter hook). In fact, we already have the admin_headers hook:

// Force no cache for all admin pages
if( yourls_is_admin() && !headers_sent() ) {
header( 'Expires: Thu, 23 Mar 1972 07:00:00 GMT' );
header( 'Last-Modified: ' . gmdate( 'D, d M Y H:i:s' ) . ' GMT' );
header( 'Cache-Control: no-cache, must-revalidate, max-age=0' );
header( 'Pragma: no-cache' );
yourls_content_type_header( yourls_apply_filter( 'html_head_content-type', 'text/html' ) );
yourls_do_action( 'admin_headers', $context, $title );
}

So assuming I didn't miss another hook in my Ctrl-F through the hook list for "head", all that's needed is a hypothetical redirect_headers (or go_headers, etc.) action to let plugins send their own. Thoughts?

Collaborator

dgw commented Sep 13, 2018

Should it be a filter, or an action? A filter implies passing each plugin that registers for the hook an array of headers and allowing removal or modification before YOURLS finally sends the whole batch of headers at once. I think an action is simpler, since it only lets plugins add headers.

Since YOURLS itself will send only basic headers for most requests, allowing plugins to add more should be enough in most cases (and is much simpler to implement than a full-on filter hook). In fact, we already have the admin_headers hook:

// Force no cache for all admin pages
if( yourls_is_admin() && !headers_sent() ) {
header( 'Expires: Thu, 23 Mar 1972 07:00:00 GMT' );
header( 'Last-Modified: ' . gmdate( 'D, d M Y H:i:s' ) . ' GMT' );
header( 'Cache-Control: no-cache, must-revalidate, max-age=0' );
header( 'Pragma: no-cache' );
yourls_content_type_header( yourls_apply_filter( 'html_head_content-type', 'text/html' ) );
yourls_do_action( 'admin_headers', $context, $title );
}

So assuming I didn't miss another hook in my Ctrl-F through the hook list for "head", all that's needed is a hypothetical redirect_headers (or go_headers, etc.) action to let plugins send their own. Thoughts?

dgw added a commit to dgw/YOURLS that referenced this issue Sep 14, 2018

Remove referrer meta tag from admin head
It's not clear why this tag was added, and it's even less clear what
benefit there would be to keeping it. Its only real function seems to be
leaking YOURLS' admin panel location (see #2432), which is not desirable.

A hook (or hooks) will likely be added to let plugins control referrer
behavior, by adding the relevant HTTP header(s) and/or meta tag(s).
@ozh

This comment has been minimized.

Show comment
Hide comment
@ozh

ozh Sep 14, 2018

Member

The action hook admin_headers is indeed enough to add what's needed if needed on admin pages. Upon redirection, we already have redirect_shorturl:

// Force no cache for all admin pages
if( yourls_is_admin() && !headers_sent() ) {
header( 'Expires: Thu, 23 Mar 1972 07:00:00 GMT' );
header( 'Last-Modified: ' . gmdate( 'D, d M Y H:i:s' ) . ' GMT' );
header( 'Cache-Control: no-cache, must-revalidate, max-age=0' );
header( 'Pragma: no-cache' );
yourls_content_type_header( yourls_apply_filter( 'html_head_content-type', 'text/html' ) );
yourls_do_action( 'admin_headers', $context, $title );
}

And there's even pre_redirect for internal redirection. I think we're covered here then ?

Member

ozh commented Sep 14, 2018

The action hook admin_headers is indeed enough to add what's needed if needed on admin pages. Upon redirection, we already have redirect_shorturl:

// Force no cache for all admin pages
if( yourls_is_admin() && !headers_sent() ) {
header( 'Expires: Thu, 23 Mar 1972 07:00:00 GMT' );
header( 'Last-Modified: ' . gmdate( 'D, d M Y H:i:s' ) . ' GMT' );
header( 'Cache-Control: no-cache, must-revalidate, max-age=0' );
header( 'Pragma: no-cache' );
yourls_content_type_header( yourls_apply_filter( 'html_head_content-type', 'text/html' ) );
yourls_do_action( 'admin_headers', $context, $title );
}

And there's even pre_redirect for internal redirection. I think we're covered here then ?

@dgw

This comment has been minimized.

Show comment
Hide comment
@dgw

dgw Sep 14, 2018

Collaborator

Sounds like it.

I'll close this, since referrer won't be sent from admin console any more except for HTTP-to-HTTP (and hosting YOURLS admin without HTTPS is pretty bad in 2018).

Collaborator

dgw commented Sep 14, 2018

Sounds like it.

I'll close this, since referrer won't be sent from admin console any more except for HTTP-to-HTTP (and hosting YOURLS admin without HTTPS is pretty bad in 2018).

@dgw dgw closed this Sep 14, 2018

@ozh

This comment has been minimized.

Show comment
Hide comment
@ozh

ozh Sep 14, 2018

Member

Anyway someone on HTTP can totally have a plugin that blocks referrer then, if needed

Member

ozh commented Sep 14, 2018

Anyway someone on HTTP can totally have a plugin that blocks referrer then, if needed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment