Page MenuHomePhabricator

HTTP redirect to TLS-encrypted connection on xtools.wmflabs.org
Closed, ResolvedPublic2 Story Points

Description

Hi, maybe forgotten but i advise you to redirect requests from http://xtools.wmflabs.org:80 via HTTP-301 to https://xtools.wmflabs.org:443 because you have TLS-support trough a wildcard X.509.

Regards --Keks

Event Timeline

Der_Keks created this task.Jul 18 2017, 9:05 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJul 18 2017, 9:05 PM
Bawolff added a subscriber: Bawolff.

You may also want to consider sending hsts headers

Der_Keks triaged this task as Low priority.Jul 18 2017, 9:15 PM

Yeah i didn't wrote that but checked https://www.ssllabs.com/ssltest/analyze.html?d=xtools.wmflabs.org
Better first go sure, that TLS is used :)
--Keks

Bawolff changed the visibility from "Custom Policy" to "Public (No Login Required)".Jul 18 2017, 9:48 PM

[Marking bug public as no issues in the bug require secrecy]

@Der_Keks Future reference, if you file a security issue only subscribers can see it. If you file one against XTools, please subscribe at least myself, MusikAnimal, or Samwilson.

Matthewrbowker moved this task from Inbox to Other on the XTools board.Jul 18 2017, 9:55 PM

Is there any progress on this?

Probably won’t see any progress until we move this task into “working” on our work board.

Restricted Application added a project: Security. · View Herald TranscriptSep 2 2017, 3:40 PM
Matthewrbowker moved this task from Other to Working on the XTools board.

Okay, I did some investigation into this. According to the Symfony docs, it is possible to force https via Symfony. However, this requires configuring each individual route. Unless there's an option I'm missing. http://symfony.com/doc/current/security/force_https.html

As our servers are configured with Puppet, this is theoretically feasible. But a huge developer burden to configure each individual route and maintain that list. Therefore, I propose closing this task as "wontfix" as we are not required to implement HTTPS and because of the burden.

Restricted Application added a project: User-Matthewrbowker. · View Herald TranscriptSep 3 2017, 6:25 AM

Man, I'm really tired. We have another option (and this might be the smartest), is just do an apache-side configuration. See https://wiki.apache.org/httpd/RedirectSSL for the simple way to do it.

Since we're already configuring things with Puppet, T170514: Set up XTools servers with Puppet might be the place to chase that down. @MaxSem since you did the initial puppet work (with gerrit 368101), could you make this a thing?

Yeah guys that i am thinking about since 2 Month but noone made this suggestion...
In Nginx i realized this on my own Nextcloud-Server without any CMS. I hate CMS *shaking*
http://xtools.wmflabs.org/ is using nginx too. So heavy to go into the configs and change it? Don't worry, you can test the config via shell: nginx -t

server {
	listen 80;
	server_name xtools.wmflabs.org;
	server_tokens off;
		location / {
			return 301 https://$host:443$request_uri;
		}
}

It is a simple operation and it takes sooo long?

This is what's done in IA Upload:

	if ( $request->headers->get( 'X-Forwarded-Proto' ) == 'http' ) {
		$uri = 'https://' . $request->getHost() . $request->headers->get( 'X-Original-URI' );
		return new RedirectResponse( $uri );
	}

Ok you get the host and the uri manually. The variable $host and $request_uri does this for you automatically, so "$request->getHost()" == "$host". You method has no HTTP Status Code to define. I defined 301, because Google and other SE will compensate this instead of 302 (downranking)

No, sorry, what I was getting at is that you have to examine the X-Forwarded-Proto header, rather than the port number, because all requests come in on port 80 from the proxy (because we can't see the original request, for privacy reasons).

(I think.)

Please do not remove my user project while I’m assigned to a task.

Matthewrbowker lowered the priority of this task from Low to Lowest.Sep 4 2017, 12:25 AM

@Der_Keks https://xtools.wmflabs.org is already available. We're going to continue investigating this, but as of right now you're welcome to use that link to browse securely.

Lowering priority as a workaround exists.

There is actually another option: https://www.owasp.org/index.php/HTTP_Strict_Transport_Security_Cheat_Sheet

This is what ACC uses.

The proper way to set headers in Symfony: https://symfony.com/doc/current/introduction/http_fundamentals.html - Hoping that we can set it based on subdomain.

Legoktm added a subscriber: Legoktm.Sep 6 2017, 4:59 AM

I feel like this discussion is over-complicating this - it should be pretty trivial to implement I think. For example, my HTTPS redirector in Python is exactly 4 lines of code: https://github.com/legoktm/toolforge/blob/master/toolforge/__init__.py#L99

The solution outlined in T170989#3575779 seems fine.

I feel like this discussion is over-complicating this - it should be pretty trivial to implement I think. For example, my HTTPS redirector in Python is exactly 4 lines of code: https://github.com/legoktm/toolforge/blob/master/toolforge/__init__.py#L99
The solution outlined in T170989#3575779 seems fine.

Except for the obvious problem that we don't use nginx. We use Apache. Hense my further investigation into this.

The solution outlined in T170989#3575779 seems fine.

Except for the obvious problem that we don't use nginx. We use Apache. Hense my further investigation into this.

I think I'm missing exactly what part of T170989#3575779 relies on nginx. The X-Forwarded-Proto proto header should be set by the proxy.

Matthewrbowker raised the priority of this task from Lowest to Low.
Matthewrbowker edited projects, added Community-Tech; removed User-Matthewrbowker.

As this appears to be part of the puppet configuration, I am transfering this task over to @Samwilson as discussed on IRC.

For such a small intervention wasted so much time O_o

For such a small intervention wasted so much time O_o

Yes, this is a relatively low priority change. We're putting it into our puppet configuration when it is finally built, which is ongoing.

This should have nothing to do with puppet. The code snippet pasted in T170989#3575779 should probably cover it.

I'll confirm what @Legoktm said. Cloud dynamicproxy sets the header, regardless of the backend server, apache or nginx.

kaldari set the point value for this task to 2.Oct 3 2017, 11:16 PM

This should have nothing to do with puppet. The code snippet pasted in T170989#3575779 should probably cover it.

We've opted to force a header via apache config. That's the easiest way to handle it, as Symfony has some strange configuration settings for forcing HTTPS (and we don't want those necessarily set for other users of XTools). That's also why it has to be puppetized (we maintain virtual 3 servers right now, with possibly more in the future depending on need)

I'll confirm what @Legoktm said. Cloud dynamicproxy sets the header, regardless of the backend server, apache or nginx.

Do you have any documentation on this "Cloud dynamicproxy" and how to inject headers into it? If so I'd love to see it.

kaldari moved this task from Untriaged to Estimated on the Community-Tech board.Oct 3 2017, 11:29 PM

Do you have any documentation on this "Cloud dynamicproxy" and how to inject headers into it? If so I'd love to see it.

I'm unaware of the documentation, but the code is at https://github.com/wikimedia/puppet/blob/production/modules/dynamicproxy/templates/domainproxy.conf#L90

MusikAnimal closed this task as Resolved.Oct 4 2017, 12:14 AM
MusikAnimal moved this task from Working to Complete on the XTools board.

Done with https://wikitech.wikimedia.org/w/index.php?title=Tool:XTools&diff=1771815&oldid=1771705

We'll add it to the puppet config whenever we actually create a puppet config :)

Sorry for the long wait.

MusikAnimal moved this task from Estimated to Archive on the Community-Tech board.Oct 4 2017, 12:14 AM