Обсуждение: Docs: Encourage strong server verification with SCRAM

Hi all,

As touched on in past threads, our SCRAM implementation is slightly
nonstandard and doesn't always protect the entirety of the
authentication handshake:

- the username in the startup packet is not covered by the SCRAM
crypto and can be tampered with if channel binding is not in effect,
or by an attacker holding only the server's key

- low iteration counts accepted by the client make it easier than it
probably should be for a MITM to brute-force passwords (note that
PG16's scram_iterations GUC, being server-side, does not mitigate
this)

- by default, a SCRAM exchange can be exited by the server prior to
sending its verifier, skipping the client's server authentication step
(this is mitigated by requiring channel binding, and PG16 adds
require_auth=scram-sha-256 to help as well)

These aren't currently considered security vulnerabilities, but it'd
be good for the documentation to call them out, considering mutual
authentication is one of the design goals of the SCRAM RFC. (I'd also
like to shore up these problems, eventually, to make SCRAM-based
mutual authn viable with Postgres. But that work has stalled a bit on
my end.)

Here's a patch to explicitly warn people away from SCRAM as a form of
server authentication, and nudge them towards a combination with
verified TLS or gssenc. I've tried to keep the text version-agnostic,
to make a potential backport easier. Is this a good place for the
warning to go? Should I call out that GSS can't use channel binding,
or promote the use of TLS versus GSS for SCRAM, or just keep it
simple?

Thanks,
--Jacob

Greetings,

* Jacob Champion (jchampion@timescale.com) wrote:
> As touched on in past threads, our SCRAM implementation is slightly
> nonstandard and doesn't always protect the entirety of the
> authentication handshake:
>
> - the username in the startup packet is not covered by the SCRAM
> crypto and can be tampered with if channel binding is not in effect,
> or by an attacker holding only the server's key

Encouraging channel binding when using TLS certainly makes a lot of
sense, particularly in environments where the trust anchors are not
strongly managed (that is- if you trust the huge number of root
certificates that a system may have installed).  Perhaps explicitly
encouraging users to *not* trust every root server installed on their
client for their PG connections would also be a good idea.  We should
probably add language to that effect to this section.

We do default to having channel binding being used though.  Breaking
down exactly the cases at risk (perhaps including what version certain
things were introduced) and what direct action administrators can take
to ensure their systems are as secure as possible (and maybe mention
what things that doesn't address still, so they're aware of what risks
still exist) would be good.

> - low iteration counts accepted by the client make it easier than it
> probably should be for a MITM to brute-force passwords (note that
> PG16's scram_iterations GUC, being server-side, does not mitigate
> this)

This would be good to improve on.

> - by default, a SCRAM exchange can be exited by the server prior to
> sending its verifier, skipping the client's server authentication step
> (this is mitigated by requiring channel binding, and PG16 adds
> require_auth=scram-sha-256 to help as well)

Yeah, encouraging this would also be good and should be mentioned as
something to do when one is using SCRAM.  Clearly that would go into a
PG16 version of this.

> These aren't currently considered security vulnerabilities, but it'd
> be good for the documentation to call them out, considering mutual
> authentication is one of the design goals of the SCRAM RFC. (I'd also
> like to shore up these problems, eventually, to make SCRAM-based
> mutual authn viable with Postgres. But that work has stalled a bit on
> my end.)

Improving the documentation certainly is a good plan.

> Here's a patch to explicitly warn people away from SCRAM as a form of
> server authentication, and nudge them towards a combination with
> verified TLS or gssenc. I've tried to keep the text version-agnostic,
> to make a potential backport easier. Is this a good place for the
> warning to go? Should I call out that GSS can't use channel binding,
> or promote the use of TLS versus GSS for SCRAM, or just keep it
> simple?

Adding channel binding to GSS (which does support it, we just don't
implement it today..) when using TLS or another encryption method for
transport would be a good improvement to make, particularly right now as
we don't yet support encryption with SSPI, meaning that you need to use
TLS to get session encryption when one of the systems is on Windows.  I
do hope to add support for encryption with SSPI during this next release
cycle and would certainly welcome help from anyone else who is
interested in this.

I have to admit that the patch as presented strikes me as a warning
without really providing steps for how to address the issues mentioned
though; there's a reference to what was just covered at the bottom but
nothing really new.  The current documentation leads with the warnings
and then goes into steps to take to address the risk, and I'd say it
would make more sense to put this wording about SCRAM not being a
complete solution to this issue above the steps that one can take to
reduce the spoofing risk, similar to how the documentation currently is.

Perhaps more succinctly- maybe we should be making adjustments to the
current language instead of just adding a new paragraph.

Thanks,

Stephen

On Tue, May 23, 2023 at 05:02:50PM -0400, Stephen Frost wrote:
> * Jacob Champion (jchampion@timescale.com) wrote:
>> As touched on in past threads, our SCRAM implementation is slightly
>> nonstandard and doesn't always protect the entirety of the
>> authentication handshake:
>>
>> - the username in the startup packet is not covered by the SCRAM
>> crypto and can be tampered with if channel binding is not in effect,
>> or by an attacker holding only the server's key
>
> Encouraging channel binding when using TLS certainly makes a lot of
> sense, particularly in environments where the trust anchors are not
> strongly managed (that is- if you trust the huge number of root
> certificates that a system may have installed).  Perhaps explicitly
> encouraging users to *not* trust every root server installed on their
> client for their PG connections would also be a good idea.  We should
> probably add language to that effect to this section.

Currently the user name is not treated by the backend, like that in
auth-scram.c:

    /*
     * Read username.  Note: this is ignored.  We use the username from the
     * startup message instead, still it is kept around if provided as it
     * proves to be useful for debugging purposes.
     */
    state->client_username = read_attr_value(&p, 'n');

Could it make sense to cross-check that with the contents of the
startup package instead, with or without channel binding?

>> - low iteration counts accepted by the client make it easier than it
>> probably should be for a MITM to brute-force passwords (note that
>> PG16's scram_iterations GUC, being server-side, does not mitigate
>> this)
>
> This would be good to improve on.

Hmm.  Interesting.

> > - by default, a SCRAM exchange can be exited by the server prior to
> > sending its verifier, skipping the client's server authentication step
> > (this is mitigated by requiring channel binding, and PG16 adds
> > require_auth=scram-sha-256 to help as well)
>
> Yeah, encouraging this would also be good and should be mentioned as
> something to do when one is using SCRAM.  Clearly that would go into a
> PG16 version of this.

Agreed to improve the docs about ways to mitigate any risks.
--
Michael

Greetings,

* Michael Paquier (michael@paquier.xyz) wrote:
> On Tue, May 23, 2023 at 05:02:50PM -0400, Stephen Frost wrote:
> > * Jacob Champion (jchampion@timescale.com) wrote:
> >> As touched on in past threads, our SCRAM implementation is slightly
> >> nonstandard and doesn't always protect the entirety of the
> >> authentication handshake:
> >>
> >> - the username in the startup packet is not covered by the SCRAM
> >> crypto and can be tampered with if channel binding is not in effect,
> >> or by an attacker holding only the server's key
> >
> > Encouraging channel binding when using TLS certainly makes a lot of
> > sense, particularly in environments where the trust anchors are not
> > strongly managed (that is- if you trust the huge number of root
> > certificates that a system may have installed).  Perhaps explicitly
> > encouraging users to *not* trust every root server installed on their
> > client for their PG connections would also be a good idea.  We should
> > probably add language to that effect to this section.
>
> Currently the user name is not treated by the backend, like that in
> auth-scram.c:
>
>     /*
>      * Read username.  Note: this is ignored.  We use the username from the
>      * startup message instead, still it is kept around if provided as it
>      * proves to be useful for debugging purposes.
>      */
>     state->client_username = read_attr_value(&p, 'n');
>
> Could it make sense to cross-check that with the contents of the
> startup package instead, with or without channel binding?

Not without breaking things we support today and for what seems like an
unclear benefit given that we've got channel binding today (though
perhaps we need to make sure there's ways to force it on both sides to
be on and to encourage everyone to do that- which is what this change is
generally about, I think).

As I recall, the reason we do it the way we do is because the SASL spec
that SCRAM is implemented under requires the username to be utf8 encoded
while we support other encodings, and I don't think we want to break
non-utf8 usage.

Thanks,

Stephen

On Tue, May 23, 2023 at 09:46:58PM -0400, Stephen Frost wrote:
> Not without breaking things we support today and for what seems like an
> unclear benefit given that we've got channel binding today (though
> perhaps we need to make sure there's ways to force it on both sides to
> be on and to encourage everyone to do that- which is what this change is
> generally about, I think).
>
> As I recall, the reason we do it the way we do is because the SASL spec
> that SCRAM is implemented under requires the username to be utf8 encoded
> while we support other encodings, and I don't think we want to break
> non-utf8 usage.

Yup, I remember this one, the encoding not being enforced by the
protocol has been quite an issue when this was implemented, still I
was wondering whether that's something that could be worth revisiting
at some degree.
--
Michael

Greetings,

* Michael Paquier (michael@paquier.xyz) wrote:
> On Tue, May 23, 2023 at 09:46:58PM -0400, Stephen Frost wrote:
> > Not without breaking things we support today and for what seems like an
> > unclear benefit given that we've got channel binding today (though
> > perhaps we need to make sure there's ways to force it on both sides to
> > be on and to encourage everyone to do that- which is what this change is
> > generally about, I think).
> >
> > As I recall, the reason we do it the way we do is because the SASL spec
> > that SCRAM is implemented under requires the username to be utf8 encoded
> > while we support other encodings, and I don't think we want to break
> > non-utf8 usage.
>
> Yup, I remember this one, the encoding not being enforced by the
> protocol has been quite an issue when this was implemented, still I
> was wondering whether that's something that could be worth revisiting
> at some degree.

To the extent that there was an issue when it was implemented ... it's
now been implemented and so that was presumably overcome (though I don't
really specifically recall what the issues were there?  Seems like it
wouldn't matter at this point though), so I don't understand why we
would wish to revisit it.

Thanks,

Stephen

On Tue, May 23, 2023 at 10:01:03PM -0400, Stephen Frost wrote:
> To the extent that there was an issue when it was implemented ... it's
> now been implemented and so that was presumably overcome (though I don't
> really specifically recall what the issues were there?  Seems like it
> wouldn't matter at this point though), so I don't understand why we
> would wish to revisit it.

Well, there are IMO two sides issues here:
1) libpq sends an empty username, which can be an issue if attempting
to make this layer more pluggable with other things that are more
compilation than PG with the SCRAM exchange (OpenLDAP is one, there
could be others).
2) The backend ignoring the username means that it is not mixed in the
hashes.

Relying on channel binding mitigates the range of problems for 2), now
one thing is that the default sslmode does not enforce an SSL
connection, either, though this default has been discussed a lot.  So
I am really wondering whether there wouldn't be some room for a more
compliant, strict HBA option with scram-sha-256 where we'd only allow
UTF-8 compliant strings.  Just some food for thoughts.

And we could do better about the current state that's 1), anyway.
--
Michael

> On 23 May 2023, at 23:02, Stephen Frost <sfrost@snowman.net> wrote:
> * Jacob Champion (jchampion@timescale.com) wrote:

>> - low iteration counts accepted by the client make it easier than it
>> probably should be for a MITM to brute-force passwords (note that
>> PG16's scram_iterations GUC, being server-side, does not mitigate
>> this)
>
> This would be good to improve on.

The mechanics of this are quite straighforward, the problem IMHO lies in how to
inform and educate users what a reasonable iteration count is, not to mention
what an iteration count is in the first place.

> Perhaps more succinctly- maybe we should be making adjustments to the
> current language instead of just adding a new paragraph.

+1

--
Daniel Gustafsson

On 5/24/23 05:04, Daniel Gustafsson wrote:
>> On 23 May 2023, at 23:02, Stephen Frost <sfrost@snowman.net> wrote:
>> * Jacob Champion (jchampion@timescale.com) wrote:
> 
>>> - low iteration counts accepted by the client make it easier than it
>>> probably should be for a MITM to brute-force passwords (note that
>>> PG16's scram_iterations GUC, being server-side, does not mitigate
>>> this)
>>
>> This would be good to improve on.
> 
> The mechanics of this are quite straighforward, the problem IMHO lies in how to
> inform and educate users what a reasonable iteration count is, not to mention
> what an iteration count is in the first place.

Yeah, the education around this is tricky.
>> Perhaps more succinctly- maybe we should be making adjustments to the
>> current language instead of just adding a new paragraph.
> 
> +1

I'm 100% on board for doing that as well, but the "instead of"
suggestion makes me think I didn't explain my goal very well. For
example, Stephen, you said

> I have to admit that the patch as presented strikes me as a warning
> without really providing steps for how to address the issues mentioned
> though; there's a reference to what was just covered at the bottom but
> nothing really new.

but the last sentence of my patch *is* the suggested step:

> +      ... It's recommended to employ strong server
> +      authentication, using one of the above anti-spoofing measures, to prevent
> +      these attacks.

In other words: if you're thinking about authenticating the server via
SCRAM, over an otherwise anonymous key exchange, you should probably
reconsider. The current architecture of libpq doesn't really seem to be
set up for this, and my conversations with security@ have been focusing
around the argument that people who want strong guarantees should be
authenticating the server using a TLS certificate before doing anything
else, so we don't need to consider our departures from the spec to be
vulnerabilities. I think that's a reasonable stance to take, as long as
we're also explicitly warning people away from the mutual-auth use case.

To change this, aspects of the code that we don't currently consider
security problems would probably need to be reclassified. If we're
depending on SCRAM for server authentication, we can't trust a single
byte that the server sends to us until after the SCRAM verifier comes
back and checks out. Compared to all the other authentication types,
that's unusual; I don't think it's really been a focus for the project
before.

--Jacob

On 5/23/23 21:37, Michael Paquier wrote:
> On Tue, May 23, 2023 at 10:01:03PM -0400, Stephen Frost wrote:
>> To the extent that there was an issue when it was implemented ... it's
>> now been implemented and so that was presumably overcome (though I don't
>> really specifically recall what the issues were there?  Seems like it
>> wouldn't matter at this point though), so I don't understand why we
>> would wish to revisit it.
> 
> Well, there are IMO two sides issues here:
> 1) libpq sends an empty username, which can be an issue if attempting
> to make this layer more pluggable with other things that are more
> compilation than PG with the SCRAM exchange (OpenLDAP is one, there
> could be others).
> 2) The backend ignoring the username means that it is not mixed in the
> hashes.

+1

> Relying on channel binding mitigates the range of problems for 2),

(Except for username tampering with possession of the server key alone.
As spec'd, that shouldn't be possible. But for the vast majority of
users, I think that's probably not on the list of plausible concerns.)

> now
> one thing is that the default sslmode does not enforce an SSL
> connection, either, though this default has been discussed a lot.  So
> I am really wondering whether there wouldn't be some room for a more
> compliant, strict HBA option with scram-sha-256 where we'd only allow
> UTF-8 compliant strings.  Just some food for thoughts.
> 
> And we could do better about the current state that's 1), anyway.

I would definitely like to see improvements in this area. I don't think
we'd need to break compatibility with clients, either (unless the server
operator explicitly wanted to). The hard part is mainly on the server
side, when dealing with a mismatch between the startup packet and the
SCRAM header.

--Jacob

Greetings,

* Jacob Champion (jchampion@timescale.com) wrote:
> On 5/24/23 05:04, Daniel Gustafsson wrote:
> >> On 23 May 2023, at 23:02, Stephen Frost <sfrost@snowman.net> wrote:
> >> Perhaps more succinctly- maybe we should be making adjustments to the
> >> current language instead of just adding a new paragraph.
> >
> > +1
>
> I'm 100% on board for doing that as well, but the "instead of"
> suggestion makes me think I didn't explain my goal very well. For
> example, Stephen, you said
>
> > I have to admit that the patch as presented strikes me as a warning
> > without really providing steps for how to address the issues mentioned
> > though; there's a reference to what was just covered at the bottom but
> > nothing really new.
>
> but the last sentence of my patch *is* the suggested step:
>
> > +      ... It's recommended to employ strong server
> > +      authentication, using one of the above anti-spoofing measures, to prevent
> > +      these attacks.

I was referring specifically to that ordering as not being ideal or in
line with the rest of the flow of that section.  We should integrate the
concerns higher in the section where we outline the reason these things
matter and then follow those with the specific steps for how to address
them, not give a somewhat unclear reference from the very bottom back up
to something above.

Thanks,

Stephen

On Thu, May 25, 2023 at 10:29 AM Stephen Frost <sfrost@snowman.net> wrote:
> I was referring specifically to that ordering as not being ideal or in
> line with the rest of the flow of that section.  We should integrate the
> concerns higher in the section where we outline the reason these things
> matter and then follow those with the specific steps for how to address
> them, not give a somewhat unclear reference from the very bottom back up
> to something above.

Ah, I misunderstood. I'll give that a shot.

Thanks!
--Jacob

On 5/25/23 1:29 PM, Stephen Frost wrote:
> Greetings,
> 
> * Jacob Champion (jchampion@timescale.com) wrote:
>> On 5/24/23 05:04, Daniel Gustafsson wrote:
>>>> On 23 May 2023, at 23:02, Stephen Frost <sfrost@snowman.net> wrote:
>>>> Perhaps more succinctly- maybe we should be making adjustments to the
>>>> current language instead of just adding a new paragraph.
>>>
>>> +1
>>
>> I'm 100% on board for doing that as well, but the "instead of"
>> suggestion makes me think I didn't explain my goal very well. For
>> example, Stephen, you said
>>
>>> I have to admit that the patch as presented strikes me as a warning
>>> without really providing steps for how to address the issues mentioned
>>> though; there's a reference to what was just covered at the bottom but
>>> nothing really new.
>>
>> but the last sentence of my patch *is* the suggested step:
>>
>>> +      ... It's recommended to employ strong server
>>> +      authentication, using one of the above anti-spoofing measures, to prevent
>>> +      these attacks.
> 
> I was referring specifically to that ordering as not being ideal or in
> line with the rest of the flow of that section.  We should integrate the
> concerns higher in the section where we outline the reason these things
> matter and then follow those with the specific steps for how to address
> them, not give a somewhat unclear reference from the very bottom back up
> to something above.

Caught up on this exchange. Some of my comments may be out-of-order.

Overall, +1 to tightening the language around the docs in this area.

However, to paraphrase Stephen, I think the language, as currently 
written, makes the problem sound scarier than it actually is, and I 
agree that we should just inline it above. There may also be some 
follow-up development work we can do to mitigate the issues.

I think it's fine for us to recommend using channel binding, but if 
we're concerned about server spoofing / rogue servers, we should also 
recommend using sslmode=verify-full to ensure server-identity. That 
doesn't necessarily help in the case the server itself has gone rogue, 
but that mitigates the MITM risk. The SCRAM RFC is also very clear that 
you should be using TLS[1].

I really don't think the "startup packet" is an actual issue, but I 
think recommending good TLS / channel binding mitigates this.

For the iteration count, I'm generally ambivalent here. I think again, 
if you're using good TLS, this is most likely mitigated. If you're 
connecting to a rogue server using good TLS, you likely have other 
issues to deal with. However, there may be a libpq feature here that 
lets a client specify a minimum iteration count it will accept for 
purposes of SCRAM.

Thanks,

Jonathan

[1] https://www.rfc-editor.org/rfc/rfc5802#section-9

On Thu, May 25, 2023 at 10:48 AM Jonathan S. Katz <jkatz@postgresql.org> wrote:
> Overall, +1 to tightening the language around the docs in this area.
>
> However, to paraphrase Stephen, I think the language, as currently
> written, makes the problem sound scarier than it actually is, and I
> agree that we should just inline it above.

How does v2 look? I more or less divided the current text into a local
section and a network section. (I'm still not clear on where in the
current text you're wanting me to inline a sudden aside on SCRAM; it
doesn't really seem to fit in any of the existing paragraphs.)

--Jacob

On 5/25/23 3:27 PM, Jacob Champion wrote:
> On Thu, May 25, 2023 at 10:48 AM Jonathan S. Katz <jkatz@postgresql.org> wrote:
>> Overall, +1 to tightening the language around the docs in this area.
>>
>> However, to paraphrase Stephen, I think the language, as currently
>> written, makes the problem sound scarier than it actually is, and I
>> agree that we should just inline it above.
> 
> How does v2 look? I more or less divided the current text into a local
> section and a network section. (I'm still not clear on where in the
> current text you're wanting me to inline a sudden aside on SCRAM; it
> doesn't really seem to fit in any of the existing paragraphs.)

I read through the proposal and like this much better. I missed 
Stephen's point on the "where" to put it in this section; I actually 
don't know if I agree with that (he says while painting the bikeshed), 
given the we spend two paragraphs describing how to prevent spoofing in 
general over the network, vs. just during SCRAM authentication.

I rewrote this to just focus on server spoofing that can occur with 
SCRAM authentication and did some wordsmithing. I was torn on keeping in 
the part of offline analysis of an intercepted hash, given one can do 
this with md5 as well, but perhaps it helps elaborate on the consequences.

Thanks,

Jonathan

On Thu, May 25, 2023 at 6:10 PM Jonathan S. Katz <jkatz@postgresql.org> wrote:
> I read through the proposal and like this much better.

Great!

> I rewrote this to just focus on server spoofing that can occur with
> SCRAM authentication and did some wordsmithing. I was torn on keeping in
> the part of offline analysis of an intercepted hash, given one can do
> this with md5 as well, but perhaps it helps elaborate on the consequences.

This part:

> +   To prevent server spoofing from occurring when using
> +   <link linkend="auth-password">scram-sha-256</link> password authentication
> +   over a network, you should ensure you are connecting using SSL.

seems to backtrack on the recommendation -- you have to use
sslmode=verify-full, not just SSL, to avoid handing a weak(er) hash to
an untrusted party.

--Jacob

On 5/26/23 6:47 PM, Jacob Champion wrote:
> On Thu, May 25, 2023 at 6:10 PM Jonathan S. Katz <jkatz@postgresql.org> wrote:

>> +   To prevent server spoofing from occurring when using
>> +   <link linkend="auth-password">scram-sha-256</link> password authentication
>> +   over a network, you should ensure you are connecting using SSL.
> 
> seems to backtrack on the recommendation -- you have to use
> sslmode=verify-full, not just SSL, to avoid handing a weak(er) hash to
> an untrusted party.

The above assumes that the reader reviewed the previous paragraph and 
followed the guidelines there. However, we can make it explicit. Please 
see attached.

Thanks,

Jonathan

On Sun, May 28, 2023 at 02:21:53PM -0400, Jonathan S. Katz wrote:
> The above assumes that the reader reviewed the previous paragraph and
> followed the guidelines there. However, we can make it explicit. Please see
> attached.

Yeah, I was under the same impression as Jacob that we don't insist
enough in this new paragraph about what sslmode ought to be when I
initially read the patch.  However, looking at the html page produced,
what you have to refer to the previous paragraph looks OK to me.
--
Michael

On Sun, May 28, 2023 at 2:22 PM Jonathan S. Katz <jkatz@postgresql.org> wrote:
> The above assumes that the reader reviewed the previous paragraph and
> followed the guidelines there. However, we can make it explicit. Please
> see attached.

LGTM!

Thanks,
--Jacob

On Wed, May 31, 2023 at 10:08:39AM -0400, Jacob Champion wrote:
> LGTM!

Okay.  Does anybody have any comments and/or objections?
--
Michael

> On 31 May 2023, at 23:14, Michael Paquier <michael@paquier.xyz> wrote:
> On Wed, May 31, 2023 at 10:08:39AM -0400, Jacob Champion wrote:

>> LGTM!
> 
> Okay.  Does anybody have any comments and/or objections? 

LGTM. As a small nitpick, I think this sentence is a little bit misleading:

    "..can use offline analysis to determine the hashed password from
     the client"

It's true that an attacker kan use offline analysis but it makes it sound
easier than it might be in practice.  I would have written "to potentially
determine".

--
Daniel Gustafsson

On Thu, Jun 01, 2023 at 10:22:28AM +0200, Daniel Gustafsson wrote:
> It's true that an attacker kan use offline analysis but it makes it sound
> easier than it might be in practice.  I would have written "to potentially
> determine".

Hmm.  Okay by me.
--
Michael

On Thu, Jun 01, 2023 at 08:28:32AM -0400, Michael Paquier wrote:
> Hmm.  Okay by me.

Took me some time to get back to it, but applied this way.
--
Michael

On Sat, Jun 3, 2023 at 2:50 PM Michael Paquier <michael@paquier.xyz> wrote:
> Took me some time to get back to it, but applied this way.

Thanks all!

--Jacob

On 6/5/23 11:22 AM, Jacob Champion wrote:
> On Sat, Jun 3, 2023 at 2:50 PM Michael Paquier <michael@paquier.xyz> wrote:
>> Took me some time to get back to it, but applied this way.
> 
> Thanks all!

+1; thank you!

Jonathan