Re: [dev] [surf] [PATCHES] (1) GConf URL schema handlers (2) delete _SURF_GO xprop (3) close stdout sending XID

2011-04-11 Thread Adam Strzelecki
 Security isn't kept.  This seems like more of a prevention of accidental 
 disclosure than real security.  (And therefore pointless...?)
 As an example, with this patch applied, run the following:
 The output in the spy terminal is:
 _SURF_GO: not found
 _SURF_GO(UTF8_STRING) = 0x61, 0x73, 0x64, 0x66   == asdf
 _SURF_GO: not found

You're absolutely right, thanks for noticing that flaw in my patch. Do you have 
any idea if it is possible to set up some write-only atom in X11 or we would 
need something else like unix socket, to prevent some other party spying what 
URLs are sent to *surf*?

Cheers,
-- 
Adam Strzelecki




Re: [dev] [surf] [PATCHES] (1) GConf URL schema handlers (2) delete _SURF_GO xprop (3) close stdout sending XID

2011-04-09 Thread Bjartur Thorlacius
On 4/7/11, Nick suckless-...@njw.me.uk wrote:
 Quoth Bjartur Thorlacius:
 On 4/7/11, Adam Strzelecki o...@java.pl wrote:
  (2) surf-2-delete-_SURF_GO-once-received.patch
 
  This xprop (atom) may be used to tell *surf* to go to specific URL. It
  is
  safer to remove this atom just after it is set in case we send some URL
  containing passwords or auth tokens such as
  http://login:mypassw...@myserver.com/
  Anyway _SURF_URI will represents current page URL, so keeping _SURF_GO
  makes
  no sense. In our case it is matter of safety to not expose this one.
 
 Is there no race condition inherent? What happens if you try to read
 _SURF_GO just after it's set?

 _SURF_GO shouldn't be read, though, it's only used for telling surf
 to load a new page. Unless I'm misunderstanding your point.

If it can't be read, then what's the original security breach?



Re: [dev] [surf] [PATCHES] (1) GConf URL schema handlers (2) delete _SURF_GO xprop (3) close stdout sending XID

2011-04-09 Thread Adam Strzelecki
 _SURF_GO just after it's set?
 
 _SURF_GO shouldn't be read, though, it's only used for telling surf
 to load a new page. Unless I'm misunderstanding your point.
 
 If it can't be read, then what's the original security breach?

Nick wrote it shouldn't be read, but it can. So right now you can set _SURF_GO 
and what you set is visible (including any passwords) using xprop, while 
_SURF_URI contains same URL but this time password-less.

-- 
Adam Strzelecki




Re: [dev] [surf] [PATCHES] (1) GConf URL schema handlers (2) delete _SURF_GO xprop (3) close stdout sending XID

2011-04-09 Thread Bjartur Thorlacius
Adam Strzelecki o...@java.pl wrote:
 It is safer to remove this atom just after it is set in case we send some URL 
 containing passwords or auth tokens
I'm confused as to the state between setting _SURF_GO and removing it.
It smells like a race condition to me, but then again I don't
understand X11 properties. I'd like a clarification as to how security
is kept in the meantime (between setting and removal of _SURF_GO).



Re: [dev] [surf] [PATCHES] (1) GConf URL schema handlers (2) delete _SURF_GO xprop (3) close stdout sending XID

2011-04-09 Thread Benjamin R. Haskell

On Sat, 9 Apr 2011, Bjartur Thorlacius wrote:


Adam Strzelecki wrote:
It is safer to remove this atom just after it is set in case we send 
some URL containing passwords or auth tokens
I'm confused as to the state between setting _SURF_GO and removing it. 
It smells like a race condition to me, but then again I don't 
understand X11 properties. I'd like a clarification as to how security 
is kept in the meantime (between setting and removal of _SURF_GO).


Security isn't kept.  This seems like more of a prevention of accidental 
disclosure than real security.  (And therefore pointless...?)


As an example, with this patch applied, run the following:

# start surf, grabbing its ID
$ surf -x
52428803

# in a 'spy' terminal
$ xprop -spy -id 52428803 _SURF_GO

# elsewhere, update _SURF_GO:
$ sprop 52428803 _SURF_GO asdf

The output in the spy terminal is:
_SURF_GO: not found
_SURF_GO(UTF8_STRING) = 0x61, 0x73, 0x64, 0x66   == asdf
_SURF_GO: not found

--
Best,
Ben



[dev] [surf] [PATCHES] (1) GConf URL schema handlers (2) delete _SURF_GO xprop (3) close stdout sending XID

2011-04-07 Thread Adam Strzelecki
Below you will find patches for latest HG head of *surf* which I am using in my 
project using integrated Linux station.

(1) surf-1-respect-GNOME-URL-handlers.patch

This patch makes *surf* respect URL schema handlers registered in 
GNOME/Gtk/GConf, when requested url is using other schemas than http: or https:.
Since anyway *surf* is Gtk program it was easy to add GConf handling inside to 
open other program registered on /desktop/gnome/url-handlers/schema/command, 
such as IRC, IMs.

(2) surf-2-delete-_SURF_GO-once-received.patch

This xprop (atom) may be used to tell *surf* to go to specific URL. It is safer 
to remove this atom just after it is set in case we send some URL containing 
passwords or auth tokens such as http://login:mypassw...@myserver.com/
Anyway _SURF_URI will represents current page URL, so keeping _SURF_GO makes no 
sense. In our case it is matter of safety to not expose this one.

(3) surf-3-close-and-reopen-stdout-when-sending-XID.patch

When using -x option making *surf* emit its window XID currently it does not 
close stdout so:

  SURFXID=`surf -x 2/dev/null `

will hand forever because no EOF is encountered.

So this patch reopens stdout to /dev/null just after sending XID using -x 
option, so caller process (shell) will receive EOF for stdout and return the 
control.

I hope these small patches gets accepted into HG repo so, we will be able to 
use *surf* out of the box in our project without need for custom patching.

Cheers,
-- 
Adam Strzelecki


surf-1-respect-GNOME-URL-handlers.patch
Description: Binary data


surf-2-delete-_SURF_GO-once-received.patch
Description: Binary data


surf-3-close-and-reopen-stdout-when-sending-XID.patch
Description: Binary data


Re: [dev] [surf] [PATCHES] (1) GConf URL schema handlers (2) delete _SURF_GO xprop (3) close stdout sending XID

2011-04-07 Thread Nick
On Thu, Apr 07, 2011 at 02:15:11PM +0200, Adam Strzelecki wrote:
 (2) surf-2-delete-_SURF_GO-once-received.patch
 
 This xprop (atom) may be used to tell *surf* to go to specific URL. It is 
 safer to remove this atom just after it is set in case we send some URL 
 containing passwords or auth tokens such as 
 http://login:mypassw...@myserver.com/
 Anyway _SURF_URI will represents current page URL, so keeping _SURF_GO makes 
 no sense. In our case it is matter of safety to not expose this one.

Cool, good idea. Can't _SURF_URI have login/password info in
too (I haven't checked)?



Re: [dev] [surf] [PATCHES] (1) GConf URL schema handlers (2) delete _SURF_GO xprop (3) close stdout sending XID

2011-04-07 Thread Adam Strzelecki
 Anyway _SURF_URI will represents current page URL, so keeping _SURF_GO makes 
 no sense. In our case it is matter of safety to not expose this one.
 
 Cool, good idea. Can't _SURF_URI have login/password info in
 too (I haven't checked)?

AFAIK no, that one is taken out from WebKit (webkit_web_view_get_uri) which 
does not return passwords here, event they are used. At least at WebKit GTK 
version I am using.
-- 
Adam Strzelecki




Re: [dev] [surf] [PATCHES] (1) GConf URL schema handlers (2) delete _SURF_GO xprop (3) close stdout sending XID

2011-04-07 Thread Bjartur Thorlacius
On 4/7/11, Adam Strzelecki o...@java.pl wrote:
 Below you will find patches for latest HG head of *surf* which I am using in
 my project using integrated Linux station.

 (1) surf-1-respect-GNOME-URL-handlers.patch

 This patch makes *surf* respect URL schema handlers registered in
 GNOME/Gtk/GConf, when requested url is using other schemas than http: or
 https:.
 Since anyway *surf* is Gtk program it was easy to add GConf handling inside
 to open other program registered on
 /desktop/gnome/url-handlers/schema/command, such as IRC, IMs.

Upstream should avoid depending on GConf. I wouldn't be surprised if
some here would prefer P9 plumbing.

 (2) surf-2-delete-_SURF_GO-once-received.patch

 This xprop (atom) may be used to tell *surf* to go to specific URL. It is
 safer to remove this atom just after it is set in case we send some URL
 containing passwords or auth tokens such as
 http://login:mypassw...@myserver.com/
 Anyway _SURF_URI will represents current page URL, so keeping _SURF_GO makes
 no sense. In our case it is matter of safety to not expose this one.

Is there no race condition inherent? What happens if you try to read
_SURF_GO just after it's set?

 (3) surf-3-close-and-reopen-stdout-when-sending-XID.patch

 When using -x option making *surf* emit its window XID currently it does not
 close stdout so:

   SURFXID=`surf -x 2/dev/null `

 will hand forever because no EOF is encountered.

 So this patch reopens stdout to /dev/null just after sending XID using -x
 option, so caller process (shell) will receive EOF for stdout and return the
 control.

Quite useful indeed.

 I hope these small patches gets accepted into HG repo so, we will be able to
 use *surf* out of the box in our project without need for custom patching.

Thanks for submitting, I'll patch locally, in the least.



Re: [dev] [surf] [PATCHES] (1) GConf URL schema handlers (2) delete _SURF_GO xprop (3) close stdout sending XID

2011-04-07 Thread Nick
Quoth Bjartur Thorlacius:
 On 4/7/11, Adam Strzelecki o...@java.pl wrote:
  (2) surf-2-delete-_SURF_GO-once-received.patch
 
  This xprop (atom) may be used to tell *surf* to go to specific URL. It is
  safer to remove this atom just after it is set in case we send some URL
  containing passwords or auth tokens such as
  http://login:mypassw...@myserver.com/
  Anyway _SURF_URI will represents current page URL, so keeping _SURF_GO makes
  no sense. In our case it is matter of safety to not expose this one.
 
 Is there no race condition inherent? What happens if you try to read
 _SURF_GO just after it's set?

_SURF_GO shouldn't be read, though, it's only used for telling surf 
to load a new page. Unless I'm misunderstanding your point.


pgpt6H8OXj5y6.pgp
Description: PGP signature