1511151 - Add a preference to enable sending client certificates when Fetch's credentials mode would exclude them, even though the spec says not to

Status: RESOLVED FIXED

(Core :: Security: PSM, enhancement, P1)

Description

[jgoerzen]

Attachment: anon.diff

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Firefox/60.0

Steps to reproduce:

I experienced a situation in which the change introduced by bug #466080 interfered with TLS client auth to a service - in this case, Duo Trusted Endpoints, which is used for enterprise authentication in a number of situations.  This has rendered companies which use Trusted Endpoints incompatible with Firefox for any authenticated service.


Actual results:

I imported the TLS client certificate into Firefox, but was never prompted for it, despite being configured to do so.

I traced this down to the code reacting to 466080, which blocked TLS client certs when LOAD_ANONYMOUS is set.


Expected results:

The attached patch preserves the current behavior as a default, and adds a pref that allows the user to work around it if necessary.  I have tested this and verified it works.  Duo had been saying there was no way to make this work with Firefox - sigh.

Comment 1

[Junior [inactive]]

I'd like to know why we set the request with LOAD_ANONYMOUS.
Is it a fetch?
Fetch can be non-anonymous by setting {credentials:"include"}.

Comment 2

[jgoerzen]

This thing is a bunch of twisty little passages, not quite all alike.  It's loaded inside an iframe from other sides, and no doubt has a bunch of javascript doing stuff there.

I believe Chrome also has this issue, because https://duo.com/docs/trusted-endpoints-manual#import-the-certificate14 references the need to set AutoSelectCertificateForUrls in Chrome.  I will try to dive into it tomorrow, though I'm much more at home with SSL code than with Javascript bits.

Comment 3

[Junior [inactive]]

Given we have web-compat issue, transfer to security component, though IMO the workaround for cert isn't the best practice.

Comment 4

[Johann Hofmann [:johannh]]

I agree that adding a pref here might not be the ideal solution.

Dana, what do you think of the approach? Are you the right person to make this call?

Comment 5

[Dana Keeler (she/her) (use needinfo) [:keeler]]

I don't think we should make any changes without first determining why the LOAD_ANONYMOUS flag has been set (particularly if Chrome behaves the same way - that sounds like a website issue).

Comment 6

[jgoerzen]

Hi folks,

First, a correction.  I was wrong about Chrome.  It works fine with this.  The pref I mentioned caused it to not ask the user every time.

What info would be helpful to track this down?  I don't know what LOAD_ANONYMOUS was really intended for, but if there's something I could read about that, it might help point me in the proper direction - or if there's some easy way to track it in the console, etc.  Alternatively, one could sign up for a free trial on https://duo.com and enable the trusted endpoints, which would let those of you more familiar with this stuff dive into it directly, but I recognize that may require more effort and I'm happy to dive into this if I can as well.  Just point me in the right direction to gather info for you and I'll do it (might be delayed a few days due to some travels but I will get to it)

Comment 7

[J.C. Jones [:jcj] (he/they)]

How can we determine what load is opting into being anonymous that really shouldn't be anonymous? Maybe it's a CORS preflight?

Comment 8

[J.C. Jones [:jcj] (he/they)]

Christoph - This is either DOM or Networking, as we don't know enough about the relevant specs to know where to look as to how this behavior is ... misbehaving. Can you weigh in, and if need be move it on to Networking or wherever it belongs?

Comment 9

[Daniel Veditz [:dveditz]]

Anne: this appears to be some clash between CORS (anonymous requests) and a site wanting to use Client certs. Haven't tested myself but the reporter claims Chrome works in this case. What ought to happen here? Sicking explicitly disabled this in bug 466080

Comment 10

[Anne (:annevk)]

This sounds like a duplicate of bug 1019603. Chrome's bug is https://bugs.chromium.org/p/chromium/issues/detail?id=775438.

Comment 12

[jgoerzen]

Attachment: firefox.diff

Here is an updated diff, against Firefox 78.4.0esr.

Comment 13

[Dana Keeler (she/her) (use needinfo) [:keeler]]

Comment on attachment 9185647 [details] [diff] [review]
firefox.diff

Review of attachment 9185647 [details] [diff] [review]:
-----------------------------------------------------------------

We don't use splinter to review patches any more. Here's documentation on the current process: https://firefox-source-docs.mozilla.org/contributing/how_to_submit_a_patch.html

::: security/manager/ssl/nsNSSIOLayer.cpp
@@ +2528,4 @@
>    uint32_t flags = 0;
>    infoObject->GetProviderFlags(&flags);
> +  bool allowAnonymous = false;
> +  Preferences::GetBool("security.ssl.allow_personal_cert_when_anonymous", &allowAnonymous);

This function runs on the socket thread. This preferences api can't be accessed from any thread other than the main thread. It'll probably be best to use a static preference: https://searchfox.org/mozilla-central/source/modules/libpref/init/StaticPrefList.yaml

@@ +2532,2 @@
>    // Provide the client cert to HTTPS proxy no matter if it is anonymous.
> +  if (flags & nsISocketProvider::ANONYMOUS_CONNECT && !haveHTTPSProxy && (!allowAnonymous)) {

This allows sending a client certificate in *any* anonymous context, not just CORS preflights. It would be best for privacy to only do this for CORS. Here's some folks you can ask about how to do that: https://wiki.mozilla.org/Modules/All#Necko

Comment 14

[jgoerzen]

Thank you, Dana! I'm wondering if anyone is able to help me out here or take over the patch? I really am not an expert on the Firefox codebase, and have never worked with that CORS code; I just wrote this one patch two years ago, and my time to plumb the depths is limited this week. I am of course happy to build and test whatever is developed and help in any way I can (including learning more about the internal CORS code when time permits) but just being realistic about my schedule right now. Thanks!

Comment 15

[Dana Keeler (she/her) (use needinfo) [:keeler]]

Dragana, can you help out here? The issue is that, from my reading, if this preference causes Firefox to unconditionally ignore the ANONYMOUS_CONNECT flag, Firefox will send a client certificate in contexts other than just CORS preflights (private browsing, CSP reports, etc.). Is there a way to tell if a particular connection is for CORS?

Comment 16

[Dragana Damjanovic [:dragana]]

We do not have a way to distinguish it it was CORS preflight request in PSM layer.

Way to go around this is to add a flag:
PSM reads nsISocketProvider flags, so we need to add a flag there. That flag should be set around here.
That means that we need to add similar flag to nsISocketTransport. nsISocketTransport flags are set here
...
And just looking into this there is a side affect of this:
we will need to isolate a connection that is used for CORS preflight and that means that connection will only be used for such requests, otherwise we will reuse it for other requests.
Maybe it is easier for a necko person to write the patch. I am happy to guide you jgoerzen if you like, but it will be a lot of details. Otherwise I will write a patch.

Comment 17

[jgoerzen]

Please go ahead, Dragana. I'm not even a Firefox dev; I just learned enough about it to write the initial patch two years ago, so it would take quite a bit of time for me to get up to speed on all this. And THANK YOU!

Comment 18

[Dragana Damjanovic [:dragana]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=1cbffe57619dfa6c0d48fdf7e58086a66cc3e5f6

Comment 19

[Dragana Damjanovic [:dragana]]

Attachment: Add a flag to allow client certs on CORS preflight connections

This is only used for CORS preflight requests. It is controlled by a pref.
Connections that server such request will be isolated from other anonymous connections.

Comment 20

[Dragana Damjanovic [:dragana]]

Hope this one is better:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=9040b76dd352ad3101a160d3a373963466a93366

Comment 21

[Dragana Damjanovic [:dragana]]

Here the pref is off:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=5d9a4b99c228a11fc46fbc1e86e02b0b9604c764

With the pref turned on:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=b5d2879984d59915343cb4ce8777e8c62a3a065e

Comment 22

[Dragana Damjanovic [:dragana]]

jgoerzen, do you want to try if this patch fixes your issue?
I am not sure on which platform you are but you can download a binary from the try run and just run it without installing anything. Of course you can delete the binary afterwords.

For linux, the build is:
https://firefox-ci-tc.services.mozilla.com/api/queue/v1/task/N_S8jWO2Sqa2ba8_ttEyZw/runs/0/artifacts/public/build/target.tar.bz2

Comment 23

[Dragana Damjanovic [:dragana]]

You will need to type about:config in address bar and look network.cors_preflight.allow_client_cert and set it to true.

Comment 24

[jgoerzen]

Hello,

I set that pref, loaded my cert, and unfortunately it didn't work.

I'd be happy to do whatever debugging I can to help with this.

• John

Comment 25

[Dragana Damjanovic [:dragana]]

I remembered yesterday late evening that the flags are shared by 2 interfaces and that nsIRequest run out of flags. So flag that I set to be (1<<16) actually overlaps with another flag and messes all up. I need to think about how to resolved this, extending flags to be uint64_t will be a huge patch :(

Comment 26

[Anne (:annevk)]

Is it a major problem if the client certificate ends up being transmitted in other contexts? Even if we only did it for CORS preflights, that would still happen in private browsing mode, right? And any site could create a CORS preflight. If that's the simpler solution and this is off-by-default, maybe we should just go with that.

Comment 27

[Dana Keeler (she/her) (use needinfo) [:keeler]]

(In reply to Anne (:annevk) from comment #26)

Is it a major problem if the client certificate ends up being transmitted in other contexts? Even if we only did it for CORS preflights, that would still happen in private browsing mode, right? And any site could create a CORS preflight. If that's the simpler solution and this is off-by-default, maybe we should just go with that.

I would argue that that's a flaw in this patch, honestly. We should never send a client certificate in private browsing mode - that would be against the user's expectations.

Comment 28

[Dana Keeler (she/her) (use needinfo) [:keeler]]

Actually, I take that back - we currently already allow the user to choose to send client certificates in private browsing contexts (which I find a bit odd), so this patch doesn't change that behavior.

Comment 29

[jgoerzen]

Hi, just checking to see where this is, and if I can help in any way. Thanks!

Comment 30

[Anne (:annevk)]

Dragana, could you take another look at this?

Comment 31

[Dragana Damjanovic [:dragana]]

Ok, it looks like we may have a free bit for this flag.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=e5292203839d465c5017263a0b2e23fa3a18011e

Comment 32

[Dragana Damjanovic [:dragana]]

With the pref on:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=deed47eab6fb25a8b1ac024244d4e146d5dbe447

Comment 33

[Dragana Damjanovic [:dragana]]

jgoerzen, can you try this one? Thank you.

https://firefox-ci-tc.services.mozilla.com/api/queue/v1/task/JUH6_-o1RX2VuGcX79E1iQ/runs/0/artifacts/public/build/target.tar.bz2

Comment 34

[jgoerzen]

Gave it a try, verified network.cors_preflight.allow_client_cert is still true, verified my cert is imported, but it still didn't work, I'm afraid. Any debugging I can do to help out, just let me know.

Comment 35

[Dragana Damjanovic [:dragana]]

Sorry for a late replay, I was on PTO.
Ok, I double checked the patch and it does what it suppose to do.

Maybe it is something else, not preflight that is set to be anonymous and therefore it does not get client certs.

Can you make me a log:
https://developer.mozilla.org/en-US/docs/Mozilla/Debugging/HTTP_logging

please add ",pipnss:5" to MOZ_LOG

Thank you.

Comment 36

[jgoerzen]

Attachment: log.txt.gz

Hi,

Here is the log with the pipnss bits. If you need the log with the other components also, I do have that, but would like to find a less public way to share it with you.

Thanks,

John

Comment 37

[Dragana Damjanovic [:dragana]]

I will need the other components as well. Can you send it via email?

Comment 38

[Dragana Damjanovic [:dragana]]

I will look at the log to verify it, but it looks like allowing client certs only for preflights is not enough.

Anne and Dana, what do you think, can ve add a pref that allows client certs with LOAD_ANONYMOUS? It will be by default off.

Comment 39

[jgoerzen]

Sent you an email with the rest of the log. Thanks!

Comment 40

[Anne (:annevk)]

I see, so they have code such as fetch(someCrossOriginURL, { ... }) and do not pass { credentials: "include" }. As with CORS preflights it's not ideal to allow that, but I think for something that's disabled by default coupled with the fact that you need to have a client certificate configured which is very far from the norm, it's not too bad. And especially with Chrome not fixing their bug it's probably table stakes to at least offer this to enterprise users.

Comment 41

[Dana Keeler (she/her) (use needinfo) [:keeler]]

I'm not a fan of changing Firefox to contravene a specification, but I suppose since the de-facto standard (Chrome) doesn't seem interested in fixing this, then we don't have much of a choice.

Comment 42

[jgoerzen]

Attachment: 78.7.0esr.diff

In case it helps, here is my patch, updated against 78.7.0esr, that is solving it for me.

Comment 43

[markus.l2ll]

I'm a newcomer to this discussion, but I think I'm running into the same or similar bug:

• I do a cross-origin fetch with {credentials: "include"}. The method is PUT so this qualifies for requiring a preflight
• the (cross-origin) server is configured to always require client certificates;

=> the server (nginx) reports "client sent no required SSL certificate while reading client request headers"

Also, I access the cross-origin server in a GET request prior to the PUT, so the 2-way TLS connection should already be up. Firefox seems to explicitly create a new one for the preflight and explicitly sends no client certificate.

User-Agent is "Mozilla/5.0 (X11; Linux x86_64; rv:83.0) Gecko/20100101 Firefox/83.0", the server on another origin and requiring client certificates is nginx/1.18.0 with ssl_verify_client on.

I can happily provide more details!

(I also saw the other bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1019603, but am not sure if that bug reports not prompting for a client certificate when {credentials: "include"} is set, or when not.)

Comment 44

[Anne (:annevk)]

It's not a bug, but you're seeing the same issue. CORS preflights are supposed to never include credentials and servers that require credentials for successful CORS preflights are broken. Google Chrome unfortunately is also broken which is why we'll add an about:config option for this. (Also adding this back on Dragana's queue.)

Comment 45

[markus.l2ll]

(In reply to Anne (:annevk) from comment #44)

It's not a bug, but you're seeing the same issue. CORS preflights are supposed to never include credentials and servers that require credentials for successful CORS preflights are broken. Google Chrome unfortunately is also broken which is why we'll add an about:config option for this. (Also adding this back on Dragana's queue.)

I've searched for info on this issue, so much of it is repetitive to you (sorry!), but: how come I need to make client certificates optional (to allow preflights) on my client-certificate-secured-server where I could simply tell from the provided certificate whether the client is allowed in or not?

From the way things are, I either can't enforce client certificates (seems strictly less secure, plus there is additional opportunity to not get the configuration right), or, I can be clever with the cross-origin requests and use POST with one of the allowed content types (but where really PUT would be the correct verb, and json the preferable content type).

What is there a attack vector we are protecting ourselves against/why are we doing this?

Comment 46

[Anne (:annevk)]

Cross-origin GET/POST with credentials without CORS preflight are legacy exceptions all servers have to deal with due to lack of a security model in the early days of the web. CORS preflights are not and to avoid them being used in a confused deputy attack or some such they do not carry credentials.

Comment 47

[Dragana Damjanovic [:dragana]]

Can you try this build? You need to flip pref network.cors_preflight.allow_client_cert.

Comment 48

[Dragana Damjanovic [:dragana]]

Pref turned off: https://treeherder.mozilla.org/jobs?repo=try&revision=e605def4b3a483a0b2291ba498486cea7a88f4a0

Comment 49

[Dragana Damjanovic [:dragana]]

The pref turned on: https://treeherder.mozilla.org/#/jobs?repo=try&revision=350b020edc4f624bbb568729d82f5915dda8e6ee

Comment 50

[Dragana Damjanovic [:dragana]]

I forgot to pass a link....

This is the link to the build mentioned in comment #47.
https://firefox-ci-tc.services.mozilla.com/api/queue/v1/task/Of-AydMKQhKhXymMHhjXxg/runs/0/artifacts/public/build/target.tar.bz2

Comment 51

[adamcstephens]

The Mac build worked for me.

Comment 52

[jgoerzen]

That build worked for me. Excellent, thank you! I'm encouraged by this!

Comment 53

[Dragana Damjanovic [:dragana]]

Attachment: Add a test for pref network.cors_preflight.allow_client_cert

Comment 54

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/40dfa8a70d58
Add a flag to allow client certs on CORS preflight connections r=necko-reviewers,keeler,valentin,kershaw
https://hg.mozilla.org/integration/autoland/rev/2ba77e4585c4
Add a test for pref network.cors_preflight.allow_client_cert r=valentin

Comment 55

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/40dfa8a70d58
https://hg.mozilla.org/mozilla-central/rev/2ba77e4585c4

Comment 57

[Mike Kaply [:mkaply]]

This is probably worthy of enterprise release notes for 87. No rush, but if someone gets a chance, can you do a release notes summary?

Comment 58

[Anne (:annevk)]

Mike, what exactly are you looking for? Something like this perhaps:

Corporations that use TLS client certificates can flip the network.cors_preflight.allow_client_cert preference to get Google Chrome-compatible handling of the CORS protocol. In particular, contrary to the Fetch Standard, this will transmit TLS client certificates along with a CORS preflight.

Comment 59

[Mike Kaply [:mkaply]]

Perfect. Thank you!

Comment 60

[Hamish Willee]

FYI, FF87 MDN docs for this added here: https://github.com/mdn/content/pull/2558

Essentially this is a new section under CORS > Requests with credentials (see here) that says you shouldn't send the credentials in preflight requests, then has a note saying - but you can if you need to using this preference.

Comment 61

[Chris Mills [:cmills]]

Setting to DDC; thanks Hamish.