Security enhancement proposal for necko

classic Classic list List threaded Threaded
19 messages Options
Reply | Threaded
Open this post in threaded view
|

Security enhancement proposal for necko

Boris Zbarsky
I've been thinking about the interaction of security and necko recently, and I
would like to propose two new APIs for necko as follows (details are modifiable;
I'll explain what I'm trying to achieve below):

interface nsISecureIOService : nsIIOService {
   nsIChannel secureNewChannelFromURI(in nsIURI aURI, in nsISupports aContext);
   nsIChannel secureNewChannel(in AUTF8String aSpec,
                               in string aOriginCharset,
                               in nsIURI aBaseURI,
                               in nsISupports aContext);
};

interface nsISecureProtocolHandler : nsIProtocolHandler {
   // Not sure what the best api is here
};

The first api will solve the problem of callers forgetting to call CheckLoadURI.
  The idea is that the IOService will get some other service by contract (like
it does for channel redirects), or even have some category of services that are
interested, and notify them all that a new channel is being created for some URI
with some context.  The security manager can then QI the context to nsIPrincipal
and do a CheckLoadURI check.  This way all callers of secureNewChannelFromURI
are automatically safe as far as CheckLoadURI goes.  Note that this does require
that callers have the principal involved, which means that we may need api
additions to things like nsIWebNavigation too.  Or we could keep it as-is and
require nsIWebNav callers to do their own CheckLoadURI checks, I suppose...  But
I'd rather not do that.

I've been toying with having this method also call content policy, since there
is at least one content policy we have that's actually a security check.  That
would require passing a lot more data to necko, though, so I'm not sure how well
that would work....

The second API is intended to solve the problem of us hardcoding a list of
schemes in security manager, with all other schemes being allAccess for
CheckLoadURI purposes.  It would be better if we could interrogate the protocol
handlers themselves for what they think about who should be able to load them.
Most simply, this can be a single function that returns what capability is
needed to access the protocol (eg UniversalXPConnect, etc).  But maybe there's a
better api here?

Thoughts?  I'd really rather like to do this for 1.9 if we can...

-Boris
_______________________________________________
dev-tech-network mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-network
Reply | Threaded
Open this post in threaded view
|

Re: Security enhancement proposal for necko

Darin Fisher-2
A couple comments:

1) You could use thread local storage (TLS) for the context parameter,
and implement this today without any new APIs.

2) Why not use nsIProtocolHandler::protocolFlags to convey capability
requirements?

-Darin



On 3/3/06, Boris Zbarsky <[hidden email]> wrote:

> I've been thinking about the interaction of security and necko recently, and I
> would like to propose two new APIs for necko as follows (details are modifiable;
> I'll explain what I'm trying to achieve below):
>
> interface nsISecureIOService : nsIIOService {
>    nsIChannel secureNewChannelFromURI(in nsIURI aURI, in nsISupports aContext);
>    nsIChannel secureNewChannel(in AUTF8String aSpec,
>                                in string aOriginCharset,
>                                in nsIURI aBaseURI,
>                                in nsISupports aContext);
> };
>
> interface nsISecureProtocolHandler : nsIProtocolHandler {
>    // Not sure what the best api is here
> };
>
> The first api will solve the problem of callers forgetting to call CheckLoadURI.
>   The idea is that the IOService will get some other service by contract (like
> it does for channel redirects), or even have some category of services that are
> interested, and notify them all that a new channel is being created for some URI
> with some context.  The security manager can then QI the context to nsIPrincipal
> and do a CheckLoadURI check.  This way all callers of secureNewChannelFromURI
> are automatically safe as far as CheckLoadURI goes.  Note that this does require
> that callers have the principal involved, which means that we may need api
> additions to things like nsIWebNavigation too.  Or we could keep it as-is and
> require nsIWebNav callers to do their own CheckLoadURI checks, I suppose...  But
> I'd rather not do that.
>
> I've been toying with having this method also call content policy, since there
> is at least one content policy we have that's actually a security check.  That
> would require passing a lot more data to necko, though, so I'm not sure how well
> that would work....
>
> The second API is intended to solve the problem of us hardcoding a list of
> schemes in security manager, with all other schemes being allAccess for
> CheckLoadURI purposes.  It would be better if we could interrogate the protocol
> handlers themselves for what they think about who should be able to load them.
> Most simply, this can be a single function that returns what capability is
> needed to access the protocol (eg UniversalXPConnect, etc).  But maybe there's a
> better api here?
>
> Thoughts?  I'd really rather like to do this for 1.9 if we can...
>
> -Boris
> _______________________________________________
> dev-tech-network mailing list
> [hidden email]
> https://lists.mozilla.org/listinfo/dev-tech-network
>
_______________________________________________
dev-tech-network mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-network
Reply | Threaded
Open this post in threaded view
|

Re: Security enhancement proposal for necko

Darin Fisher-2
On 3/3/06, Darin Fisher <[hidden email]> wrote:
> A couple comments:
>
> 1) You could use thread local storage (TLS) for the context parameter,
> and implement this today without any new APIs.

Since NewURI is restricted to the main thread only, you could also
just use a global variable ;-)

-Darin
_______________________________________________
dev-tech-network mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-network
Reply | Threaded
Open this post in threaded view
|

Re: Security enhancement proposal for necko

Christian Biesinger
Darin Fisher wrote:
> Since NewURI is restricted to the main thread only, you could also
> just use a global variable ;-)

But that makes it much harder to catch cases where you forget to set the
right context. (And global variables across XPCOM components aren't so
easy...)

_______________________________________________
dev-tech-network mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-network
Reply | Threaded
Open this post in threaded view
|

Re: Security enhancement proposal for necko

Boris Zbarsky
In reply to this post by Boris Zbarsky
Darin Fisher wrote:
> 1) You could use thread local storage (TLS) for the context parameter,
> and implement this today without any new APIs.

How do I do this from an extension's JavaScript?  How do I do this if I'm
proxying my Necko access to the main thread from a background thread?

In any case, the point is to make it as hard as possible to screw up.  Having to
set a global (or thread-local) variable before making a newChannel call is just
as bad as having to call CheckLoadURI, from that point of view, whereas just
knowing that any newChannel call is insecure and that there is a simple more
secure option might work better...  I hope.

I really wish we'd built this into newChannel to start with.  :(

> 2) Why not use nsIProtocolHandler::protocolFlags to convey capability
> requirements?

We might be able to get away with that, yes.  At the moment, I just need to
differentiate between a small finite set of options (like 3-4).  If you're ok
with giving me a few bits to work with, I could probably put something together...

-Boris
_______________________________________________
dev-tech-network mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-network
Reply | Threaded
Open this post in threaded view
|

Re: Security enhancement proposal for necko

Darin Fisher-2
On 3/3/06, Boris Zbarsky <[hidden email]> wrote:
> Darin Fisher wrote:
> > 1) You could use thread local storage (TLS) for the context parameter,
> > and implement this today without any new APIs.
>
> How do I do this from an extension's JavaScript?

Well, there must be some API somewhere to get the right context
parameter to pass to necko, right?


> How do I do this if I'm
> proxying my Necko access to the main thread from a background thread?

Yes, good point.  You'd have to do more than just call NewURI.  You'd
have to setup the security context and then call NewURI.  Yeah, that
wouldn't be very nice.  Maybe we should extend the XPCOM proxy API to
accept a security context that is "pushed" into place for the scope of
the proxy call.


> In any case, the point is to make it as hard as possible to screw up.  Having to
> set a global (or thread-local) variable before making a newChannel call is just
> as bad as having to call CheckLoadURI, from that point of view, whereas just
> knowing that any newChannel call is insecure and that there is a simple more
> secure option might work better...  I hope.

So, this new API isn't useful unless a consumer knows how to get a
context in the first place.  Maybe it would help me to understand
where that comes from.


> > 2) Why not use nsIProtocolHandler::protocolFlags to convey capability
> > requirements?
>
> We might be able to get away with that, yes.  At the moment, I just need to
> differentiate between a small finite set of options (like 3-4).  If you're ok
> with giving me a few bits to work with, I could probably put something together...

Yes, this is something that I have always envisioned using those flags
for.  There are plenty of available bits.

-Darin
_______________________________________________
dev-tech-network mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-network
Reply | Threaded
Open this post in threaded view
|

Re: Security enhancement proposal for necko

Boris Zbarsky
In reply to this post by Boris Zbarsky
Darin Fisher wrote:
> Well, there must be some API somewhere to get the right context
> parameter to pass to necko, right?

There ought to be by the time we're done with 1.9, yes.  ;)

> Yes, good point.  You'd have to do more than just call NewURI.

Note that I want this in newChannel, not newURI.  We create plenty of URI
objects with no plans to load them (eg visited link checking).

> So, this new API isn't useful unless a consumer knows how to get a
> context in the first place.

Yes.

> Maybe it would help me to understand where that comes from.

"It depends".  In the simplest setup, the context is the nsIPrincipal that
should be used for the security check.  Where this comes from depends on the
details of what's being loaded, where, by whom, etc.

> Yes, this is something that I have always envisioned using those flags
> for.  There are plenty of available bits.

OK.  I'd been hoping to be able to output a string representing the capability
that needs to be enabled to load the URI in some cases, but I guess I can just
use bits to express the common cases (allow, deny, chrome only, need universal
xpconnect, need universal browser write, need universal browser read)...

-Boris
_______________________________________________
dev-tech-network mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-network
Reply | Threaded
Open this post in threaded view
|

Re: Security enhancement proposal for necko

Darin Fisher-2
On 3/5/06, Boris Zbarsky <[hidden email]> wrote:
> Darin Fisher wrote:
> > Well, there must be some API somewhere to get the right context
> > parameter to pass to necko, right?
>
> There ought to be by the time we're done with 1.9, yes.  ;)

I guess I was trying to get you to describe that mechanism since
getting the context is just as important as passing it along.
Ignoring the async proxy call case for a second, the complexity of
getting this context parameter and passing it to this API instead of
nsIIOService's version, is no more or less error prone than
remembering to push that context into a thread local store.

So, I think it's important to consider how this context parameter will be used.

Also, I think that this context API will require some modifications to
the channel implementations.  For example, any channel that issues a
redirect will need to forward the context, right?  What about
third-party protocol handlers?  What happens if a protocol handler
does not forward the context from one channel to the next?  Maybe we
could use nsIPropertyBag to solve this problem since we can make our
built-in channels support that interface and properties are
automatically copied to a channel created via redirect.


> > Yes, good point.  You'd have to do more than just call NewURI.
>
> Note that I want this in newChannel, not newURI.  We create plenty of URI
> objects with no plans to load them (eg visited link checking).

typo on my part.  i get what you're after here.


> > Maybe it would help me to understand where that comes from.
>
> "It depends".  In the simplest setup, the context is the nsIPrincipal that
> should be used for the security check.  Where this comes from depends on the
> details of what's being loaded, where, by whom, etc.

OK.... so perhaps what we need from a necko point-of-view is a way to
construct a channel with additional properties.  And, then we can have
that special new channel method call out to a global set of channel
creation observers that can have access to the channel and the
properties.


> > Yes, this is something that I have always envisioned using those flags
> > for.  There are plenty of available bits.
>
> OK.  I'd been hoping to be able to output a string representing the capability
> that needs to be enabled to load the URI in some cases, but I guess I can just
> use bits to express the common cases (allow, deny, chrome only, need universal
> xpconnect, need universal browser write, need universal browser read)...

hrm... can you take a moment to define what those mean for channels?
for example, an file channel configured to upload data (to write data
to disk) would seem to need universal browser write, whereas a file
channel configured to read data would seem to need universal browser
read.  In other words, it doesn't seem to be sufficient to express
those qualities on the singleton nsIProtocolHandler for the file
protocol.

-Darin
_______________________________________________
dev-tech-network mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-network
Reply | Threaded
Open this post in threaded view
|

Re: Security enhancement proposal for necko

Boris Zbarsky
In reply to this post by Boris Zbarsky
Darin Fisher wrote:
>>> Well, there must be some API somewhere to get the right context
>>> parameter to pass to necko, right?
>> There ought to be by the time we're done with 1.9, yes.  ;)
>
> I guess I was trying to get you to describe that mechanism since
> getting the context is just as important as passing it along.

Ah.  Well, the mechanism depends on what's going on, basically... if you're
taking data from a web page and loading it somewhere, the context is the
nsIPrincipal of that document or window or whatever node the data came from.
Basically, whatever place people get the URI from for CheckLoadURI right now.
Or I'd hope they're getting one; if they're not it's a bug.

> Ignoring the async proxy call case for a second, the complexity of
> getting this context parameter and passing it to this API instead of
> nsIIOService's version, is no more or less error prone than
> remembering to push that context into a thread local store.

It's no more or less error prone when writing the code, perhaps.  It's a lot
easier to catch when reviewing or auditing code.

With the new API, you check whether the new API is being used.  If not, it's
probably a security bug.  If it is, you check where the context is coming from
-- since it needs to be passed in explicitly, this is not so hard.

With the global or thread-local store, you have to figure out whether some
caller somewhere pushed the right thing.  It's a _lot_ harder to verify correctness.

Or at least this is how it seems to me....

> Also, I think that this context API will require some modifications to
> the channel implementations.

I'm not sure it would, actually.

> For example, any channel that issues a redirect will need to forward the context, right?

Actually, I don't think it would.  For a redirect, the principal should be that
of the document the channel would have loaded, not of the document that started
the load.  We're checking whether the server is allowed to redirect, not whether
the original page is allowed to open the new URI.

If the context is something more complicated than a principal, I'm not as sure
that the answer is "no"... ;)

> What about third-party protocol handlers?

If we keep the CheckLoadURI checks in nsIOService, I think they should be ok....

> OK.... so perhaps what we need from a necko point-of-view is a way to
> construct a channel with additional properties.  And, then we can have
> that special new channel method call out to a global set of channel
> creation observers that can have access to the channel and the
> properties.

Hmm.  That might work, yes....

>> OK.  I'd been hoping to be able to output a string representing the capability
>> that needs to be enabled to load the URI in some cases, but I guess I can just
>> use bits to express the common cases (allow, deny, chrome only, need universal
>> xpconnect, need universal browser write, need universal browser read)...
>
> hrm... can you take a moment to define what those mean for channels?
> for example, an file channel configured to upload data (to write data
> to disk) would seem to need universal browser write, whereas a file
> channel configured to read data would seem to need universal browser
> read.  In other words, it doesn't seem to be sufficient to express
> those qualities on the singleton nsIProtocolHandler for the file
> protocol.

I was thinking I'd go with the worst-case scenario, I think.  So a file channel
would require UniversalBrowserWrite or even UniversalXPConnect.  Maybe there
really aren't any useful use cases for anything but
UniversalXPConnect/deny/chromeonly/allow (and I'd argue that UniversalXPConnect
and chromeonly should be the same thing).

We have no idea what the channel is configured for because our security check is
"can we load this URI?"  I guess if we were doing the check at channel
construction time we could turn it into "can we load this channel?".  That would
actually be kinda nice...

-Boris
_______________________________________________
dev-tech-network mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-network
Reply | Threaded
Open this post in threaded view
|

Re: Security enhancement proposal for necko

Darin Fisher-2
On 3/6/06, Boris Zbarsky <[hidden email]> wrote:

> Darin Fisher wrote:
> >>> Well, there must be some API somewhere to get the right context
> >>> parameter to pass to necko, right?
> >> There ought to be by the time we're done with 1.9, yes.  ;)
> >
> > I guess I was trying to get you to describe that mechanism since
> > getting the context is just as important as passing it along.
>
> Ah.  Well, the mechanism depends on what's going on, basically... if you're
> taking data from a web page and loading it somewhere, the context is the
> nsIPrincipal of that document or window or whatever node the data came from.
> Basically, whatever place people get the URI from for CheckLoadURI right now.
> Or I'd hope they're getting one; if they're not it's a bug.
>
> > Ignoring the async proxy call case for a second, the complexity of
> > getting this context parameter and passing it to this API instead of
> > nsIIOService's version, is no more or less error prone than
> > remembering to push that context into a thread local store.
>
> It's no more or less error prone when writing the code, perhaps.  It's a lot
> easier to catch when reviewing or auditing code.
>
> With the new API, you check whether the new API is being used.  If not, it's
> probably a security bug.  If it is, you check where the context is coming from
> -- since it needs to be passed in explicitly, this is not so hard.
>
> With the global or thread-local store, you have to figure out whether some
> caller somewhere pushed the right thing.  It's a _lot_ harder to verify correctness.

ok


> > Also, I think that this context API will require some modifications to
> > the channel implementations.
>
> I'm not sure it would, actually.
>
> > For example, any channel that issues a redirect will need to forward the context, right?
>
> Actually, I don't think it would.  For a redirect, the principal should be that
> of the document the channel would have loaded, not of the document that started
> the load.  We're checking whether the server is allowed to redirect, not whether
> the original page is allowed to open the new URI.
>
> If the context is something more complicated than a principal, I'm not as sure
> that the answer is "no"... ;)

Remember, necko doesn't know about prinicipals.  What should a channel
implementation do when it wants to redirect?  What context does it
pass to the secureNewChannel method?


> > What about third-party protocol handlers?
>
> If we keep the CheckLoadURI checks in nsIOService, I think they should be ok....

Again, what about channels created by third-party protocol handlers?


> > OK.... so perhaps what we need from a necko point-of-view is a way to
> > construct a channel with additional properties.  And, then we can have
> > that special new channel method call out to a global set of channel
> > creation observers that can have access to the channel and the
> > properties.
>
> Hmm.  That might work, yes....
>
> >> OK.  I'd been hoping to be able to output a string representing the capability
> >> that needs to be enabled to load the URI in some cases, but I guess I can just
> >> use bits to express the common cases (allow, deny, chrome only, need universal
> >> xpconnect, need universal browser write, need universal browser read)...
> >
> > hrm... can you take a moment to define what those mean for channels?
> > for example, an file channel configured to upload data (to write data
> > to disk) would seem to need universal browser write, whereas a file
> > channel configured to read data would seem to need universal browser
> > read.  In other words, it doesn't seem to be sufficient to express
> > those qualities on the singleton nsIProtocolHandler for the file
> > protocol.
>
> I was thinking I'd go with the worst-case scenario, I think.  So a file channel
> would require UniversalBrowserWrite or even UniversalXPConnect.  Maybe there
> really aren't any useful use cases for anything but
> UniversalXPConnect/deny/chromeonly/allow (and I'd argue that UniversalXPConnect
> and chromeonly should be the same thing).

What does "deny" and "allow" mean?  What question is being asked that
these flags answer?


> We have no idea what the channel is configured for because our security check is
> "can we load this URI?"  I guess if we were doing the check at channel
> construction time we could turn it into "can we load this channel?".  That would
> actually be kinda nice...

What matters really is what happens before AsyncOpen is called.
Normally, a channel is constructed and then modified
(nsIUploadChannel.setUploadStream), and then AsyncOpen is called.
Once AsyncOpen is called, the configuration of the channel is
effectively frozen.

Perhaps you really want to intercept AsyncOpen?

-Darin
_______________________________________________
dev-tech-network mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-network
Reply | Threaded
Open this post in threaded view
|

Re: Security enhancement proposal for necko

Boris Zbarsky
In reply to this post by Boris Zbarsky
Darin Fisher wrote:
> Remember, necko doesn't know about prinicipals.  What should a channel
> implementation do when it wants to redirect?  What context does it
> pass to the secureNewChannel method?

It should call newChannel (with a comment explaining why) and then notify
redirect observers.  Security manager being one of those.  That's the setup we
have now and it's ok for this very special case, imo.

>>> What about third-party protocol handlers?
>> If we keep the CheckLoadURI checks in nsIOService, I think they should be ok....
>
> Again, what about channels created by third-party protocol handlers?

Ah, hmm...  _that_ is a good question.  Right now they just have no way of doing
meaningful security checks.  To enable them to we'd need to either pass a
security context for such checks to them or allow them to get it from somewhere.
  I don't have a strong preference on this, I think, if only because  I suspect
this is much rarer than people creating channels in general.  I think the
argument that an explicit context makes catching bugs easier still applies, but....

> What does "deny" and "allow" mean?  What question is being asked that
> these flags answer?

"Am I allowed to load this URI (or channel)?"

"deny" means "If you're not nsSystemPrincipal, then no" at the moment.  I think
it should mean "If you don't have UniversalXPConnect, then no".  In particular,
imo "deny" should mean "not even if you're from the same scheme, if you don't
have UniversalXPConnect".

"allow" means "yes, no matter who you are."

> What matters really is what happens before AsyncOpen is called.

That's the hope, yeah....  I'm not sure some channels don't do stuff before
asyncOpen, but you're right that they shouldn't do it.

> Perhaps you really want to intercept AsyncOpen?

That would work as well, possibly even better in terms of having complete
information, but asyncOpen doesn't go through a central place like the
IOService.  :(

-Boris
_______________________________________________
dev-tech-network mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-network
Reply | Threaded
Open this post in threaded view
|

Re: Security enhancement proposal for necko

Darin Fisher-2
On 3/6/06, Boris Zbarsky <[hidden email]> wrote:
> Darin Fisher wrote:
> > Remember, necko doesn't know about prinicipals.  What should a channel
> > implementation do when it wants to redirect?  What context does it
> > pass to the secureNewChannel method?
>
> It should call newChannel (with a comment explaining why) and then notify
> redirect observers.  Security manager being one of those.  That's the setup we
> have now and it's ok for this very special case, imo.

Yeah, that works.


> >>> What about third-party protocol handlers?
> >> If we keep the CheckLoadURI checks in nsIOService, I think they should be ok....
> >
> > Again, what about channels created by third-party protocol handlers?
>
> Ah, hmm...  _that_ is a good question.  Right now they just have no way of doing
> meaningful security checks.  To enable them to we'd need to either pass a
> security context for such checks to them or allow them to get it from somewhere.
>   I don't have a strong preference on this, I think, if only because  I suspect
> this is much rarer than people creating channels in general.  I think the
> argument that an explicit context makes catching bugs easier still applies, but....
>
> > What does "deny" and "allow" mean?  What question is being asked that
> > these flags answer?
>
> "Am I allowed to load this URI (or channel)?"

Hmm... ok.  When asking such a question, it would be nice to tell the
protocol handler what URI or channel is being queried.  With just
protocol flags, we can only ask questions about all URIs of a given
scheme, and not "this" particular URI.  Maybe that's okay.


> "deny" means "If you're not nsSystemPrincipal, then no" at the moment.  I think
> it should mean "If you don't have UniversalXPConnect, then no".  In particular,
> imo "deny" should mean "not even if you're from the same scheme, if you don't
> have UniversalXPConnect".
>
> "allow" means "yes, no matter who you are."

So, the question being asked is really about restrictions placed on
the consumer of the channel.


> > What matters really is what happens before AsyncOpen is called.
>
> That's the hope, yeah....  I'm not sure some channels don't do stuff before
> asyncOpen, but you're right that they shouldn't do it.
>
> > Perhaps you really want to intercept AsyncOpen?
>
> That would work as well, possibly even better in terms of having complete
> information, but asyncOpen doesn't go through a central place like the
> IOService.  :(

Our channels calls nsIIOService::AllowPort from AsyncOpen (see
NS_CheckPortSafety).  You could extend that hook, I suppose.

-Darin
_______________________________________________
dev-tech-network mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-network
Reply | Threaded
Open this post in threaded view
|

Re: Security enhancement proposal for necko

Boris Zbarsky
In reply to this post by Boris Zbarsky
Darin Fisher wrote:
> Hmm... ok.  When asking such a question, it would be nice to tell the
> protocol handler what URI or channel is being queried.

Yeah, true.  That would need API changes though, no?

> With just protocol flags, we can only ask questions about all URIs of a given
> scheme, and not "this" particular URI.  Maybe that's okay.

Well, it's what we have now, more or less.  I'm happy with making what we have
now better, of course.  ;)

> So, the question being asked is really about restrictions placed on
> the consumer of the channel.

Pretty much.  Or restrictions on whoever gave the consumer the URI to load.

> Our channels calls nsIIOService::AllowPort from AsyncOpen (see
> NS_CheckPortSafety).  You could extend that hook, I suppose.

You mean have it take the channel, etc?  Or something else?

-Boris
_______________________________________________
dev-tech-network mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-network
Reply | Threaded
Open this post in threaded view
|

Re: Security enhancement proposal for necko

Darin Fisher-2
On 3/6/06, Boris Zbarsky <[hidden email]> wrote:
> Darin Fisher wrote:
> > Hmm... ok.  When asking such a question, it would be nice to tell the
> > protocol handler what URI or channel is being queried.
>
> Yeah, true.  That would need API changes though, no?

Yeah, it would.  I'm not sure that it is something that we should do or not.


> > Our channels calls nsIIOService::AllowPort from AsyncOpen (see
> > NS_CheckPortSafety).  You could extend that hook, I suppose.
>
> You mean have it take the channel, etc?  Or something else?

Yeah, I was thinking of generalizing it to some kind of method call on
the IO service that passes the channel and gives the IO service a
chance to abort the load.  We have the http-on-modify-request
notification that can be used to implement this today for HTTP, and
I've wanted to extend that to a generic on-modify-request notification
sent by all channels (or at least most channels).

-Darin
_______________________________________________
dev-tech-network mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-network
Reply | Threaded
Open this post in threaded view
|

Re: Security enhancement proposal for necko

Boris Zbarsky
In reply to this post by Boris Zbarsky
Darin Fisher wrote:
> Yeah, I was thinking of generalizing it to some kind of method call on
> the IO service that passes the channel and gives the IO service a
> chance to abort the load.

That would work quite nicely for what I want, if the channel has the required
security context information attached to it (as you proposed).  That would just
require us to audit existing newChannel callers, which ought to be doable, I guess.

-Boris
_______________________________________________
dev-tech-network mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-network
Reply | Threaded
Open this post in threaded view
|

Re: Security enhancement proposal for necko

Darin Fisher-2
So, if channels support nsIPropertyBag and they somehow dispatch a
notification to observers from AsyncOpen, then we could get around
needing nsISecureIOService.

We could create a utility function for creating a channel with
principal that content and imagelib and other necko consumers could
all use.

This way, necko doesn't really need to change that much.  Moreover,
provided we make nsBaseChannel emit "on-modify-request" or some
similar callback from AsyncOpen, this will all work fine when
third-party protocol handlers use nsInputStreamChannel to satisfy
their NewChannel method.

-Darin


On 3/7/06, Boris Zbarsky <[hidden email]> wrote:

> Darin Fisher wrote:
> > Yeah, I was thinking of generalizing it to some kind of method call on
> > the IO service that passes the channel and gives the IO service a
> > chance to abort the load.
>
> That would work quite nicely for what I want, if the channel has the required
> security context information attached to it (as you proposed).  That would just
> require us to audit existing newChannel callers, which ought to be doable, I guess.
>
> -Boris
> _______________________________________________
> dev-tech-network mailing list
> [hidden email]
> https://lists.mozilla.org/listinfo/dev-tech-network
>
_______________________________________________
dev-tech-network mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-network
Reply | Threaded
Open this post in threaded view
|

Re: Security enhancement proposal for necko

Darin Fisher-2
FWIW, maybe we should deprecate nsIChannel::owner, and change the JAR
channel to implement nsIJARChannel w/ a principal getter.

-Darin



On 3/7/06, Darin Fisher <[hidden email]> wrote:

> So, if channels support nsIPropertyBag and they somehow dispatch a
> notification to observers from AsyncOpen, then we could get around
> needing nsISecureIOService.
>
> We could create a utility function for creating a channel with
> principal that content and imagelib and other necko consumers could
> all use.
>
> This way, necko doesn't really need to change that much.  Moreover,
> provided we make nsBaseChannel emit "on-modify-request" or some
> similar callback from AsyncOpen, this will all work fine when
> third-party protocol handlers use nsInputStreamChannel to satisfy
> their NewChannel method.
>
> -Darin
>
>
> On 3/7/06, Boris Zbarsky <[hidden email]> wrote:
> > Darin Fisher wrote:
> > > Yeah, I was thinking of generalizing it to some kind of method call on
> > > the IO service that passes the channel and gives the IO service a
> > > chance to abort the load.
> >
> > That would work quite nicely for what I want, if the channel has the required
> > security context information attached to it (as you proposed).  That would just
> > require us to audit existing newChannel callers, which ought to be doable, I guess.
> >
> > -Boris
> > _______________________________________________
> > dev-tech-network mailing list
> > [hidden email]
> > https://lists.mozilla.org/listinfo/dev-tech-network
> >
>
_______________________________________________
dev-tech-network mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-network
Reply | Threaded
Open this post in threaded view
|

Re: Security enhancement proposal for necko

Boris Zbarsky
In reply to this post by Boris Zbarsky
Darin Fisher wrote:
> So, if channels support nsIPropertyBag and they somehow dispatch a
> notification to observers from AsyncOpen, then we could get around
> needing nsISecureIOService.

Hmm... True.

> We could create a utility function for creating a channel with
> principal that content and imagelib and other necko consumers could
> all use.

Seems reasonable, I guess.  But what really worries me is JS.  Especially
extension JS.  It would be _very_ nice to have a simple litmus test for
extension authors:  Does your code use this method?  If so, it's insecure.  I'm
really not sure what a good way to deal with this is.  :(

> This way, necko doesn't really need to change that much.  Moreover,
> provided we make nsBaseChannel emit "on-modify-request" or some
> similar callback from AsyncOpen, this will all work fine when
> third-party protocol handlers use nsInputStreamChannel to satisfy
> their NewChannel method.

Assuming they propagate their property bag stuff to it, sure.

-Boris
_______________________________________________
dev-tech-network mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-network
Reply | Threaded
Open this post in threaded view
|

Re: Security enhancement proposal for necko

Boris Zbarsky
In reply to this post by Darin Fisher-2
Darin Fisher wrote:
> FWIW, maybe we should deprecate nsIChannel::owner, and change the JAR
> channel to implement nsIJARChannel w/ a principal getter.

We could, if we moved the other owner consumers to property bag or something.

For what it's worth, I think that it should just be possible to get a
"principal" (or something QIing to one?) from channels in general...  Then we
wouldn't need this thing where chrome channels use |owner| to mean their
intrinsic principals while javascript: and data: channels use it to mean the
principal they were opened with... and expect their callers to set it!

As in, for every channel it seems there are three principals around:

1)  The principal to use for the checkLoadURI check or equiv.
2)  The principal to use for the channel if nothing better is found (eg "caller
     principal" for javascript:)
3)  The final principal to use for a document loaded via that channel.

Not sure what the best setup is for dealing with these....

-Boris
_______________________________________________
dev-tech-network mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-network