gridcf / gct

Grid Community Toolkit
Apache License 2.0
46 stars 30 forks source link

`guc -dcpriv` does not work when talking to dCache #62

Open onnozweers opened 5 years ago

onnozweers commented 5 years ago

Globus Toolkit has a security vulnerability: when globs-url-copy -dcpriv is specified, and connecting to a dCache instance (which does not support data channel privacy), globus-url-copy silently decides to send the data in plain text anyway. The user is not warned and may assume the data is transferred privately while in fact it is not.

A bug report is here: https://github.com/globus/globus-toolkit/issues/121

Is this bug present in GCT too?

Kind regards, Onno

msalle commented 5 years ago

Hi Onno, the short answer is yes, since the gct version of globus-url-copy is currently fully backwards compatible with the globus-toolkit 6.0 version. We had a long discussion about the issue in the spring but there is no easy way to fix it in ways that will not unexpectedly break things, and furthermore, would require simultaneous changes in multiple clients (also uberftp for example) and servers. That basically where the discussion stalled... Perhaps others (such as Paul) have a good idea on how to got forward.

onnozweers commented 5 years ago

Hi msalle, nice to meet you here! :-)

I'm very curious why it is so difficult to fix this. I would think, it's better to break some things than to secretly expose private data. But perhaps I'm overlooking some complications that make it more difficult than such a simple choice.

I'd like to add to the discussion that since this issue was first reported, the GDPR has become effective. So perhaps with this bug, people might be (unknowingly) violating the GDPR. But I'm not a law expert.

Kind regards, Onno

paulmillar commented 5 years ago

I don't think this is really that difficult.

There are only two ways to fix this (that make sense):

  1. fix -dcpriv so transfers fail if they are not encrypted.
  2. make -dcpriv a no-op. Document that it does nothing and it only remains for backwards compatibility. Introduce a new option that enforces encryption.

The decision between 1. and 2. is choosing whether it is better to fail people who rely on -dcpriv not working correctly (who include it by mistake), or fail people who rely on -dcpriv working correctly (who didn't change the option).

Put another way, choosing option 1. will (possibly) result in failed transfers that should succeed; choosing option 2. will (possibly) result in successful transfers that should have failed.

From a security point-of-view, only 1. makes sense, because 2. opens a fresh security problem: people who didn't update to the new option.

onnozweers commented 5 years ago

Nothing seems to happen. 😭

Could this have a higher urgency please?

fscheiner commented 5 years ago

@onnozweers @paulmillar As per #100 a warning was added to the guc manpage about this problem. I'd recommend to add something similar to the dCache documentation for the time being.

onnozweers commented 5 years ago

Hi @fscheiner, thanks for doing something! 😺

There is a whole chapter about transport security in the dCache documentation: https://github.com/dCache/dcache/blob/master/docs/TheBook/src/main/markdown/cookbook-transport-security.md

This bug is mentioned in the first paragraph.

paulmillar commented 5 years ago

@onnozweers IIRC, you are the main (sole?) contributor to this chapter, so thanks for helping document this problem!

We now have a new document that is targeted at dCache users (rather than admins). The REST API is fairly well documented, but the rest largely contains only place-holder information. One chapter is on FTP (in general), so I think this would be a natural place to document this problem.

I've created an issue (dcache/dcache#5039) to track this, to make sure it doesn't get forgotten.