Re: [Qemu-devel] [PATCH 1/1] slirp: add SOCKS5 support

[Laurent Vivier <laurent@vivier.eu>]

[-- Attachment #1: Type: text/plain, Size: 2056 bytes --]

Le 27/03/2017 à 20:41, Eric Blake a écrit :
> On 03/27/2017 01:21 PM, Laurent Vivier wrote:
>> When the VM is used behind a firewall, This allows
>> to use a SOCKS5 proxy server to connect the VM IP stack
> 
> "allows to $verb" is not idiomatic English; the correct forms are
> generally "allows $subject to $verb" or "allows ${verb}ing".  In this
> case, I'd lean towards "this allows the use of a SOCKS5 proxy server"

Thank you, and sorry.

> 
>> directly to the Internet.
>>
>> This implementation doesn't manage UDP packets, so they
>> are simply dropped (as with restrict=on), except for
>> the localhost as we need it for DNS.
>>
>> Signed-off-by: Laurent Vivier <laurent@vivier.eu>
>> ---
> 
>> +++ b/qapi-schema.json
>> @@ -3680,6 +3680,9 @@
>>      '*ipv6-dns':         'str',
>>      '*smb':       'str',
>>      '*smbserver': 'str',
>> +    '*proxy-server': 'str',
>> +    '*proxy-user':   'str',
>> +    '*proxy-passwd': 'str',
> 
> Why can't we spell this out as password, instead of abbreviating?

because of unix command "passwd" ;) . I will fix.

> Should this hook into the "secrets object" framework so that someone
> does not have to pass the password in plaintext?
> 
>>      '*hostfwd':   ['String'],
>>      '*guestfwd':  ['String'] } }
> 
> Missing documentation.

I will fix.

> Do we want all three proxy elements to be in a substruct? The difference
> is between:
> 
> { ... "smb": "foo", "proxy-server": "bar", "proxy-user": "noone",
> "proxy-passwd": "hello" }
> 
> and a substruct:
> 
> { ... "smb": "foo", "proxy": { "server": "bar", "user", "noone",
> "passwd": "hello" } }

yes, substruct looks better.

> 
>>  
>> +@item proxy-server=@var{addr}:@var{port}[,proxy-user=@var{user},proxy-passwd=@var{passwd}]]
> 
> Yes, you DEFINITELY need to hook into the "secrets object" framework to
> avoid having to pass a password in plaintext on the command line.  Dan
> Berrange may have more advice on doing that.
> 

OK

Thank you for the review.

Laurent


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]