go-fil-markets
go-fil-markets copied to clipboard
Consider limiting number of incoming streams to avoid DoS
The connection manager adds streams on each Deal that is received and disconnects the stream if there was no valid response from the client. Due to the connections being kept in a map of CID , it may be possible to generate a number of valid CID s and use this to fill connections and use memory resources on the provider machine.
https://github.com/filecoin-project/go-fil-markets/blob/0460d1d7e9ade671a1d60bbbd16357876c5efbe5/storagemarket/impl/connmanager/connmanager.go#L37-L48
We might want to add (libp2p) ConnMgr limits in the miner too and Protect peers with active transfers.
@vyzo AFAIK, go-data-transfer already protects peers with active transfers in the connmgr:
https://github.com/filecoin-project/go-data-transfer/search?q=protect&unscoped_q=protect
cc @hannahhoward
@raulk yes that is the case.
A follow-up for @schomatis :
The connections in ConnManager in the storage market are, at this point, extremely short lived. I propose a deal, and the provider puts the deal in the ConnManager only as long as it takes to evaluate the deal parameters to decide if it merits initiating data transfer. This should be a matter of milliseconds. Then a response is sent, the connection is closed, and the map is cleaned up. At another time these connections were held open for the lifespan of the deal -- this is no longer true. All storage market libp2p protocols open short live connections only as long as is needed to produce a response -- similar to way request/response protocols work in a traditional HTTP context. (the only long lived connections are those created by data transfer which does protect the libp2p connection) Given this, I am not clear if this memory map represents a security threat at this point -- except in as much as you could spam a provider with deals faster than it could evaluate and respond to them, cleaning up this map in the process.