Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1876683002:
Implement SDCH Dictionary domain matching as per RFC 2965
|
Created:
4 years, 8 months ago by bacek_yandex Modified:
3 years, 6 months ago CC:
cbentzel+watch_chromium.org, chromium-reviews Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionImplement SDCH Dictionary domain matching as per RFC 2965
Default GURL::DomainIs gives wrong results for
* "http://example.com/" won't match ".example.com"
* "http://www.example.com" will match "example.com"
R=rdsmith@chromium.org, mmenke@chromium.org
Patch Set 1 #
Total comments: 1
Patch Set 2 : Factor out IsDomainMatch from CanonicalCookie and use it in SdchDictionary. #
Messages
Total messages: 27 (4 generated)
DomainMatch is roughly based on DomainIs but in less "pure C" way.
On 2016/04/12 23:19:07, bacek_yandex wrote: > DomainMatch is roughly based on DomainIs but in less "pure C" way. Does the SDCH spec really specify using the fundamentally broken RFC 2965 for domain matching?
On 2016/04/13 01:02:55, mmenke wrote: > On 2016/04/12 23:19:07, bacek_yandex wrote: > > DomainMatch is roughly based on DomainIs but in less "pure C" way. > > Does the SDCH spec really specify using the fundamentally broken RFC 2965 for > domain matching? http://lists.w3.org/Archives/Public/ietf-http-wg/2008JulSep/att-0441/Shared_D... <quote> The scope is specified by a domain attribute and path attribute that are patterned after the same named attributes from the HTTP State Management Specification [RFC2965]. </quote> Is there any better alternatives? Our problem with current codebase that "Domain: yandex.ru" will match all subdomains. And "Domain: .yandex.ru" won't match TLD at all.
On 2016/04/13 01:09:59, bacek_yandex wrote: > On 2016/04/13 01:02:55, mmenke wrote: > > On 2016/04/12 23:19:07, bacek_yandex wrote: > > > DomainMatch is roughly based on DomainIs but in less "pure C" way. > > > > Does the SDCH spec really specify using the fundamentally broken RFC 2965 for > > domain matching? > > > http://lists.w3.org/Archives/Public/ietf-http-wg/2008JulSep/att-0441/Shared_D... > > <quote> > The scope is specified by a domain attribute and > path attribute that are patterned after the same named attributes from the HTTP > State > Management Specification [RFC2965]. > </quote> > > Is there any better alternatives? > > Our problem with current codebase that "Domain: yandex.ru" will match all > subdomains. And "Domain: .yandex.ru" won't match TLD at all. I'm no expert on security, but RFC 2965 doesn't consider HTTP and HTTPS the same origin, and doesn't consider port, both of which are generally used to distinguish origin. It's sufficiently broken that there have been multiple RFCs attempting to hack in ways to actually use cookies securely. Most other things have a notion or origin that includes scheme and port. I'm not sure how subdomains fit in, though.
On 2016/04/13 01:18:08, mmenke wrote: > On 2016/04/13 01:09:59, bacek_yandex wrote: > > On 2016/04/13 01:02:55, mmenke wrote: > > > On 2016/04/12 23:19:07, bacek_yandex wrote: > > > > DomainMatch is roughly based on DomainIs but in less "pure C" way. > > > > > > Does the SDCH spec really specify using the fundamentally broken RFC 2965 > for > > > domain matching? > > > > > > > http://lists.w3.org/Archives/Public/ietf-http-wg/2008JulSep/att-0441/Shared_D... > > > > <quote> > > The scope is specified by a domain attribute and > > path attribute that are patterned after the same named attributes from the > HTTP > > State > > Management Specification [RFC2965]. > > </quote> > > > > Is there any better alternatives? > > > > Our problem with current codebase that "Domain: yandex.ru" will match all > > subdomains. And "Domain: .yandex.ru" won't match TLD at all. > > I'm no expert on security, but RFC 2965 doesn't consider HTTP and HTTPS the same > origin, and doesn't consider port, both of which are generally used to > distinguish origin. It's sufficiently broken that there have been multiple RFCs > attempting to hack in ways to actually use cookies securely. > > Most other things have a notion or origin that includes scheme and port. I'm > not sure how subdomains fit in, though. Sorry, typo there...RFC 2965 doesn't consider scheme part of the origin, so cookies are shared between HTTP and HTTPS. It adds a "Secure" option as a hack, but unsecure cookies can overwrite secure ones, and servers can't tell when it happens.
On 2016/04/13 01:28:24, mmenke wrote: > On 2016/04/13 01:18:08, mmenke wrote: > > On 2016/04/13 01:09:59, bacek_yandex wrote: > > > On 2016/04/13 01:02:55, mmenke wrote: > > > > On 2016/04/12 23:19:07, bacek_yandex wrote: > > > > > DomainMatch is roughly based on DomainIs but in less "pure C" way. > > > > > > > > Does the SDCH spec really specify using the fundamentally broken RFC 2965 > > for > > > > domain matching? > > > > > > > > > > > > http://lists.w3.org/Archives/Public/ietf-http-wg/2008JulSep/att-0441/Shared_D... > > > > > > <quote> > > > The scope is specified by a domain attribute and > > > path attribute that are patterned after the same named attributes from the > > HTTP > > > State > > > Management Specification [RFC2965]. > > > </quote> > > > > > > Is there any better alternatives? > > > > > > Our problem with current codebase that "Domain: yandex.ru" will match all > > > subdomains. And "Domain: .yandex.ru" won't match TLD at all. > > > > I'm no expert on security, but RFC 2965 doesn't consider HTTP and HTTPS the > same > > origin, and doesn't consider port, both of which are generally used to > > distinguish origin. It's sufficiently broken that there have been multiple > RFCs > > attempting to hack in ways to actually use cookies securely. > > > > Most other things have a notion or origin that includes scheme and port. I'm > > not sure how subdomains fit in, though. > > Sorry, typo there...RFC 2965 doesn't consider scheme part of the origin, so > cookies are shared between HTTP and HTTPS. It adds a "Secure" option as a hack, > but unsecure cookies can overwrite secure ones, and servers can't tell when it > happens. I don't think that it's really relevant for SDCH Dictionary's Domain attribute. Just because we don't use dictionaries downloaded via HTTP on HTTPS requests. And vice versa.
Vasily: Is this a real problem you're running into, or is this cleanup code? If the latter, a) Thanks!, and b) I tend to think (agree with Matt) that we should clean up the spec before we start fine tuning the code to match it precisely. I've been running into specification problems on a other CLs (https://codereview.chromium.org/1880283002, https://codereview.chromium.org/1871383005) which makes me think that we need to put some energy in the spec direction.
On 2016/04/13 17:35:16, Randy Smith - Not in Fridays wrote: > Vasily: Is this a real problem you're running into, or is this cleanup code? If > the latter, a) Thanks!, and b) I tend to think (agree with Matt) that we should > clean up the spec before we start fine tuning the code to match it precisely. > I've been running into specification problems on a other CLs > (https://codereview.chromium.org/1880283002, > https://codereview.chromium.org/1871383005) which makes me think that we need to > put some energy in the spec direction. It's our real problem. We do serve multiple projects on yandex.ru domain. And we are having problems with SDCH dictionaries bound to yandex.ru "TLD" when browser tries to announce Avail-Dictionaries for all subdomain and at least blacklisting them. OTOH, I'm all in for cleaning up spec. At least add "Used-Dictionary: <server_id>" header. And fix quite a few other glitches. BTW, what is the real reason of blocking redirects on SDCH fetches? I don't have access to original bug report.
Description was changed from ========== Implement SDCH Dictionary domain matching as per RFC 2965 Default GURL::DomainIs gives wrong results for * "http://example.com/" won't match ".example.com" * "http://www.example.com" will match "example.com" R=rdsmith@chromium.org, mmenke@chromium.org ========== to ========== Implement SDCH Dictionary domain matching as per RFC 2965 Default GURL::DomainIs gives wrong results for * "http://example.com/" won't match ".example.com" * "http://www.example.com" will match "example.com" R=rdsmith@chromium.org, mmenke@chromium.org ==========
rdsmith@chromium.org changed reviewers: + mkwst@chromium.org
On 2016/04/13 21:27:36, bacek_yandex wrote: > On 2016/04/13 17:35:16, Randy Smith - Not in Fridays wrote: > > Vasily: Is this a real problem you're running into, or is this cleanup code? > If > > the latter, a) Thanks!, and b) I tend to think (agree with Matt) that we > should > > clean up the spec before we start fine tuning the code to match it precisely. > > I've been running into specification problems on a other CLs > > (https://codereview.chromium.org/1880283002, > > https://codereview.chromium.org/1871383005) which makes me think that we need > to > > put some energy in the spec direction. > > It's our real problem. We do serve multiple projects on yandex.ru domain. And we > are > having problems with SDCH dictionaries bound to yandex.ru "TLD" when browser > tries > to announce Avail-Dictionaries for all subdomain and at least blacklisting them. > > OTOH, I'm all in for cleaning up spec. At least add "Used-Dictionary: > <server_id>" > header. And fix quite a few other glitches. If you'd like to take a pass at the spec, I'm ok with that. I'd advocate for making the various bits of comparison semantics more precise before playing around with adding extra headers. Adding extra headers requires versioning the protocol, and probably getting in a lot more buy in from everyone that uses it. Possibly we should try to get this in the hands of a standards body--it's been an ad-hoc spec thrown out there by Google for too long. > BTW, what is the real reason of blocking redirects on SDCH fetches? I don't have > access > to original bug report. I've cc'd you on the bug report; let me know if you have any questions (feel free to post them on the bug). TL;DR: Minor bug, if the fix is going to cause problems, please raise those problems. I'm also pulling in Mike West as reviewer on this bug, as he's the expert on origin security issues, and the domain match here is somewhat close to that. Mike: I don't know if this is in your area of expertise or not, but it seems likely enough that it is to ask you to take a look.
On 2016/04/13 21:40:59, Randy Smith - Not in Fridays wrote: > > OTOH, I'm all in for cleaning up spec. At least add "Used-Dictionary: > > <server_id>" > > header. And fix quite a few other glitches. > > If you'd like to take a pass at the spec, I'm ok with that. I'd advocate for > making the various bits of comparison semantics more precise before playing > around with adding extra headers. Adding extra headers requires versioning the > protocol, and probably getting in a lot more buy in from everyone that uses it. Changing of protocol will require versioning. Independent of size of the change. I'll try to gather our thoughts on possible spec improvement. It will take some time though. > Possibly we should try to get this in the hands of a standards body--it's been > an ad-hoc spec thrown out there by Google for too long. Wasn't it originally proposed via w3c but didn't get much traction? > > BTW, what is the real reason of blocking redirects on SDCH fetches? I don't > have > > access > > to original bug report. > > I've cc'd you on the bug report; let me know if you have any questions (feel > free to post them on the bug). TL;DR: Minor bug, if the fix is going to cause > problems, please raise those problems. > > I'm also pulling in Mike West as reviewer on this bug, as he's the expert on > origin security issues, and the domain match here is somewhat close to that. > Mike: I don't know if this is in your area of expertise or not, but it seems > likely enough that it is to ask you to take a look.
> I don't think that it's really relevant for SDCH Dictionary's Domain attribute. > Just because we don't use dictionaries downloaded via HTTP on HTTPS requests. And > vice versa. I see a scheme check in `CanUse`, but not in `CanSet`. I don't know the SDCH code well enough to tell you if that's a problem, but it seems like it might be.
https://codereview.chromium.org/1876683002/diff/1/net/base/sdch_dictionary.cc File net/base/sdch_dictionary.cc (right): https://codereview.chromium.org/1876683002/diff/1/net/base/sdch_dictionary.cc... net/base/sdch_dictionary.cc:155: /* RFC 2965: 1. This RFC has been updated by https://tools.ietf.org/html/rfc6265, so you probably want to reference https://tools.ietf.org/html/rfc6265#section-5.1.3. 2. This replicates logic from CanonicalCookie::IsDomainMatch; I'd suggest either copy/pasting from that method, or extracting the algorithm out somewhere usable by both.
On 2016/04/14 07:32:49, Mike West wrote: > https://codereview.chromium.org/1876683002/diff/1/net/base/sdch_dictionary.cc > File net/base/sdch_dictionary.cc (right): > > https://codereview.chromium.org/1876683002/diff/1/net/base/sdch_dictionary.cc... > net/base/sdch_dictionary.cc:155: /* RFC 2965: > 1. This RFC has been updated by https://tools.ietf.org/html/rfc6265, so you > probably want to reference https://tools.ietf.org/html/rfc6265#section-5.1.3. > > 2. This replicates logic from CanonicalCookie::IsDomainMatch; I'd suggest either > copy/pasting from that method, or extracting the algorithm out somewhere usable > by both. I have a bad feelings about CanonicalCookie::IsDomainMatch. Looks like "evilexample.com" will match with ".example.com". And what about handling of trailing dot in host/domain?
On 2016/04/14 09:31:24, bacek_yandex wrote: > I have a bad feelings about CanonicalCookie::IsDomainMatch. Looks like > "evilexample.com" will match with ".example.com". And what about handling of > trailing > dot in host/domain? Oops. evilexample.com won't match. My bad. But trailing dot is still not handled.
mkwst@chromium.org changed reviewers: + rsleevi@chromium.org
On 2016/04/14 at 09:39:40, Bacek wrote: > On 2016/04/14 09:31:24, bacek_yandex wrote: > > > I have a bad feelings about CanonicalCookie::IsDomainMatch. Looks like > > "evilexample.com" will match with ".example.com". And what about handling of > > trailing > > dot in host/domain? > > Oops. evilexample.com won't match. My bad. But trailing dot is still not handled. 1. Haven't we canonicalized the trailing dot away by this point in the code? 2. `https://example.com.` is not the same origin as `https://example.com` in Blink. Don't see any good reason to treat it differently here. +Ryan: Adding you for the narrow point in #2. Opinions?
On 2016/04/14 09:51:19, Mike West wrote: > On 2016/04/14 at 09:39:40, Bacek wrote: > > On 2016/04/14 09:31:24, bacek_yandex wrote: > > > > > I have a bad feelings about CanonicalCookie::IsDomainMatch. Looks like > > > "evilexample.com" will match with ".example.com". And what about handling of > > > trailing > > > dot in host/domain? > > > > Oops. http://evilexample.com won't match. My bad. But trailing dot is still not > handled. > > 1. Haven't we canonicalized the trailing dot away by this point in the code? I'm not quite sure about it. afaik there is no url canonization in sdch code at all. > 2. `https://example.com.` is not the same origin as `https://example.com` in > Blink. Don't see any good reason to treat it differently here. > > +Ryan: Adding you for the narrow point in #2. Opinions?
I'd like to push on this without a spec. I don't believe we should be adding or changing features without that. Also, huge +1 to Mike's remarks (both points - refactor and address the difference)
On 2016/04/14 16:24:07, Ryan Sleevi wrote: > I'd like to push on this without a spec. I don't believe we should be adding or > changing features without that. I think you mean you'd like to not push on this without a spec? Based on c#8 (i.e. Yandex is running into real problems with this code), would you be open to a targeted fix for those problems without nailing down the spec? > > Also, huge +1 to Mike's remarks (both points - refactor and address the > difference)
On 2016/04/14 17:37:15, Randy Smith - Not in Fridays wrote: > On 2016/04/14 16:24:07, Ryan Sleevi wrote: > > I'd like to push on this without a spec. I don't believe we should be adding > or > > changing features without that. > > I think you mean you'd like to not push on this without a spec? I meant push back on this, yes :) Meaning "Spec before change" > Based on c#8 (i.e. Yandex is running into real problems with this code), would > you be open to a targeted fix for those problems without nailing down the spec? Not particularly, especially given the issues highlighted here. That is, despite there being a real problem and use case, I think without a spec nailing it down, I would lean on the conservative approach here.
On 2016/04/14 18:08:24, Ryan Sleevi wrote: > On 2016/04/14 17:37:15, Randy Smith - Not in Fridays wrote: > > On 2016/04/14 16:24:07, Ryan Sleevi wrote: > > > I'd like to push on this without a spec. I don't believe we should be adding > > or > > > changing features without that. > > > > I think you mean you'd like to not push on this without a spec? > > I meant push back on this, yes :) > > Meaning "Spec before change" > > > Based on c#8 (i.e. Yandex is running into real problems with this code), would > > you be open to a targeted fix for those problems without nailing down the > spec? > > Not particularly, especially given the issues highlighted here. That is, despite > there being a real problem and use case, I think without a spec nailing it down, > I would lean on the conservative approach here. Hang on a second. We do have it in spec. Even not perfect but it's still there. <quote> The scope is specified by a domain attribute and path attribute that are patterned after the same named attributes from the HTTP State Management Specification [RFC2965]. </quote> RFC2965 was replaced by RFC6265, but algorithm is basically the same.
On 2016/04/14 20:54:56, bacek_yandex wrote: > Hang on a second. We do have it in spec. Even not perfect but it's still there. We don't have a spec that matches the past several years of changes, nor that has the ability to be changed. That's the maintenance burden; unless and until there is an actively maintained/developed spec, I'm very much opposed to any changes in behaviour, and very much in support of removing this code, as has been discussed on other CLs as well. > <quote> > The scope is specified by a domain attribute and > path attribute that are patterned after the same named attributes from the HTTP > State > Management Specification [RFC2965]. > </quote> > > RFC2965 was replaced by RFC6265, but algorithm is basically the same. As Matt expressed, the algorithm for RFC 6265 is basically insane for other purposes. Which is part of the challenge - there's no place to "fix" the spec to not rely on that behaviour, nor to gauge other implementor's interests, nor to balance the security concerns. There's no place to describe how SDCH fits into the Fetch() narrative, which came up recently with respect to the redirect handling - does SDCH respect CORS or not? It doesn't as implemented. Should it? Probably. There's unquestionably a burden in supporting this code, but without an actively maintained, pursuing standardization feature, I'm very much opposed to changing functionality or, in this case, greatly widening the scope. Concretely, if you want to actively contribute, the best next steps are to 'resurrect' the spec, find an appropriate venue (if not the IETF, then incubate in the W3C's WICG), find rough consensus and gather broad implementor feedback, and then start aligning our implementation to the changing spec. Absent that, I think it's realistic that we'll be looking at an "Intent to Deprecate" the SDCH support, because it's non-standard, vendor-specific, and, judging by the recent efforts, both insufficient (the SDCH2 spec work) and not as secure as desirable.
On 2016/04/14 22:41:28, Ryan Sleevi wrote: > On 2016/04/14 20:54:56, bacek_yandex wrote: > > Hang on a second. We do have it in spec. Even not perfect but it's still > there. > > We don't have a spec that matches the past several years of changes, nor that > has the ability to be changed. Can you elaborate about "ability to be changed"? > That's the maintenance burden; unless and until there is an actively > maintained/developed spec, I'm very much opposed to any changes in behaviour, > and very much in support of removing this code, as has been discussed on other > CLs as well. I'm totally agree with maintenance cost. But after Mike's suggestion there is only few lines changed in SdchDictionary. Just use IsDomainMatch instead of DomainIs helper. And it's resolving of years old TODO item. Which is currently preventing us to deploy SDCH properly. fsvo "properly". I didn't see any discussions about "removing this code". Is it "removing sdch support at all and declare it dead"? > > <quote> > > The scope is specified by a domain attribute and > > path attribute that are patterned after the same named attributes from the > HTTP > > State > > Management Specification [RFC2965]. > > </quote> > > > > RFC2965 was replaced by RFC6265, but algorithm is basically the same. > > As Matt expressed, the algorithm for RFC 6265 is basically insane for other > purposes. Which is part of the challenge - there's no place to "fix" the spec to > not rely on that behaviour, nor to gauge other implementor's interests, nor to > balance the security concerns. There's no place to describe how SDCH fits into > the Fetch() narrative, which came up recently with respect to the redirect > handling - does SDCH respect CORS or not? It doesn't as implemented. Should it? > Probably. Current SDCH spec isn't using RFC6265 for anything apart domain matching algorithm. Nothing at all. And yes, whole fetching/security/caching/etc part is highly underspecced. > There's unquestionably a burden in supporting this code, but without an actively > maintained, pursuing standardization feature, I'm very much opposed to changing > functionality or, in this case, greatly widening the scope. Erm... How is this "greatly widening the scope"? > Concretely, if you want to actively contribute, the best next steps are to > 'resurrect' the spec, find an appropriate venue (if not the IETF, then incubate > in the W3C's WICG), find rough consensus and gather broad implementor feedback, > and then start aligning our implementation to the changing spec. Absent that, I > think it's realistic that we'll be looking at an "Intent to Deprecate" the SDCH > support, because it's non-standard, vendor-specific, and, judging by the recent > efforts, both insufficient (the SDCH2 spec work) and not as secure as desirable. I'm all for doing this stuff properly. And I will give it a go to do it through ietf. But it will take a few years to finalize. Meanwhile we are having problems with sdch deployment right now. Which can be considered as "implementor feedback" actually.
On 2016/04/15 20:07:37, bacek_yandex wrote: > Can you elaborate about "ability to be changed"? There's no actively maintained spec that, if it turns out there are spec bugs (and there always are), we can respond - either during development of the spec or in release. > I'm totally agree with maintenance cost. But after Mike's suggestion there is > only few lines changed in SdchDictionary. Just use IsDomainMatch instead of > DomainIs helper. And it's resolving of years old TODO item. Which is currently > preventing us to deploy SDCH properly. fsvo "properly". Right. And while it may sound harsh, my suggestion is that if you're unable to deploy SDCH "properly" because Chrome's implementation is lacking, then it may be worthwhile to prioritize resources to build consensus and iterate on the specification and gather feedback. I'm not comfortable supporting this change - I believe it's a "spec bug" that it shares the cookie construct, and were this a new specification, we would flag it as such and push back on the spec, for all the reasons mentioned. > I didn't see any discussions about "removing this code". Is it "removing sdch > support at all and declare it dead"? That is what an "Intent to Deprecate" for vendor-specific features is all about, yes. If we don't have the resources to maintain something, if we don't have the resources to spec something, if we don't have consensus from other implementors that this is a worthwhile and useful feature of the Web platform with some degree of interest, then we should remove support, reduce the burden, and reduce the divergence in the Web platform. This is, understandably, seemingly "drastic". But arguably, we should not have introduced this in the first place in the way we did, and the Blink process is meant to ensure we don't do it in the future. When we encounter issues like this - where historically we've done it, but we don't have the staffing and resources to maintain it - we do the Web a disservice. For example, if, on the basis of Yandex supporting SDCH (e.g. we fixed this as you deployed it), Firefox or Edge wanted to implemented SDCH support, could they, without reverse engineering Chrome's implementation? If the answer is no (and it clearly is), then we should be working towards one of two paths - provide a spec or remove the implementation. While I realize the aforementioned linked spec exist, it doesn't match what implementations (such as Chrome) do. That's deeply troubling, and suggests our first-order priorities should be in remedying that situation, such as the paths I mentioned - WICG for the W3C path or IETF for the IETF path. We should be tracking interest from implementors and gaining feedback from developers. If there were resources committed to that, I'd be supportive of changes to this code. But it's more important that we pursue an open web platform than it is that we encourage deployment of vendor-specific features, which this is. > Current SDCH spec isn't using RFC6265 for anything apart domain matching > algorithm. Nothing at all. And that is still problematic, and I'm sorry that's not being communicated clearly. Plainly put, the RFC 6265 language for domain matching - ignoring the notion of origin - is bad. With refactoring out, though a useful suggestion, is missing the underlying premise that nobody "but cookies" should be using RFC 6265 language for domain matching. I'm going to put a Not LGTM on this on that basis. > And yes, whole fetching/security/caching/etc part is > highly underspecced. > Erm... How is this "greatly widening the scope"? Changing how domains are matched. More pragmatically, we should be matching origin, rather than allowing the subdomain usage here. > I'm all for doing this stuff properly. And I will give it a go to do it through > ietf. But it will take a few years to finalize. Meanwhile we are having problems > with sdch deployment right now. Which can be considered as "implementor > feedback" actually. Agreed. But where's the spec to discuss the problems and security risks with RFC 6265 language? Where's the forum to build consensus that it's the right thing to do? Where's the analysis of the security risks relevant to the Web platform, and the violations here of the Same Origin Policy with respect to cross-origin resource fetching? How do we effectively address these concerns? I'm aware this is limiting your deployment. While it may sound cruel, I see this as a good thing - it provides the correct incentives to help us do this the right way. But I think our default state should be towards moving to SDCH disabled, as the forcing function to ensure the right resources and right committment to an open web platform are pursued.
mmenke@chromium.org changed reviewers: - mmenke@chromium.org |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
