+++ This bug was initially created as a clone of Bug #775499 +++ We use wpa_supplicant for 802.11 support, including WPA Enterprise. wpa_supplicant has its own TLS implementation and apparently uses OpenSSL for certificate validation by default. This means: 1. The WPA enteprise support cannot use the same (root) certificate database that we use for the rest of B2G. 2. We must track and respond to all security bugs in the wpa_supplicant and/or OpenSSL implementations, in addition to NSS. Basically, we should just integrate wpa_supplicant with NSS. According to the wpa_supplicant documentation, this should be straightforward to do because wpa_supplicant has a plug-in architecture for the TLS implementation. See https://bugzilla.redhat.com/show_bug.cgi?id=348471 for Red Hat's bug tracking the integration of wpa_supplicant with NSS. Maybe we can work together on this, and/or push the NSS integration to the wpa_supplicant upstream.

Brian, should this be a basecamp blocker?

This is a blocker to WPA Enterprise support. We should disable WPA Enterprise until this bug and bug 775499 are fixed. I do not think re-enabling WPA Enterprise needs to be a basecamp blocker.

Thanks, Brian. I filed bug 790054 to explicitly disable WPA Enterprise support and bug 790056 to enable it post-Basecamp.

For us it's important to ship with WPA Enterprise support. Even a broken support would be better than no support at all. In any case, we can take this bug at TEF if you wish to try and ship it in V1. Note that besides integrating NSS we should/would add a way to add user provided certificates since most companies (before somebody asks, that means all the companies I know about here) use internal certificates for their WPA Enteprise servers, so just integrating this with NSS would leave all those companies out of luck.

As mentioned to Brian during a past NSS conference call, there's some interest from ChromiumOS on this as well, as it also uses wpa_supplicant. One of the things we view as blocking is the ability to have more granular trust settings. It should be possible to trust a root for WPA without having that trust transitively apply to other verification. Bug 816853 (alternatively/along with Brian's Bug 764973 ) facilitate that effort, although that relies on application-specific effort and doesn't allow the trust settings to be stored in the central NSS DB. Both Microsoft and Apple have more defined concepts of trust (Microsoft via cert attributes, Apple via trust policies), which facilitate the central storage/management of certs.

The left green part represents current way that wpa_supplicant get certfication key in android - the key is managed by a daemon called "keystore"[1], and wap_supplicant get key from "keystore" through a local socket[2]. This flow also exists on current B2G wpa_supplicant, but we don't actually manage keys by "keystore" Considering our partners might use wpa_supplicant from vendors, and it seems supporting "keystore" is a must in android[3]. The proposed plan is left wpa_supplicant untouched, and replace "keystore" of android with a service in gecko, and read keys from NSS.(the orange part) The key manage part will be done in bug 867899 with an Web API, so it should be enough for the keystore in gecko support only read function. What the partner needs to do is disable keystore in init.rc, and maybe patch the corresponding local socket protocol is they use different protocol as [2]. [1] https://android.googlesource.com/platform/system/security/+/tools_r21/keystore/keystore.cpp [2] https://android.googlesource.com/platform/system/security/+/tools_r21/keystore/keystore_get.h [3] http://hostap.epitest.fi/gitweb/gitweb.cgi?p=hostap.git;a=blob;f=src/crypto/tls_openssl.c;h=8600f5f0aa80a8cd34e2159140a56acfb9ae8a4d;hb=HEAD#l1368

Run Setup() in server mode of unix socket.

Implement keystore ipc to replace keystore daemon in android.

Disable keystore daemon of android. Target file is B2G/system/core/rootdir/init.rc. This is affected on emulator but not on unagi. Not tested on other devices.

Use http://www.pki.gva.es/gestcert/rootca.crt as test CA

To test test keystore function, use command: - Get hardcoded CA : adb shell keystore_cli g CACERT_test_der - Get RAW CA(transformed to DER format) adb shell keystore_cli g CACERT_test_raw - GET CA in NSS(transformed to DER format) adb shell keystore_cli g CACERT_test_nss To test WAP-EAP connection, do the following steps: 1. adb pull /data/misc/wifi/wpa_supplicant.conf 2. Modify wpa_supplicant.conf with WPA-EAP setting, with setting ca_cert="keystore://CACERT_test_der" or ca_cert="keystore://CACERT_test_raw" or ca_cert="keystore://CACERT_test_nss" 3. adb push wpa_supplicant.conf /data/misc/wifi/wpa_supplicant.conf 4. adb shell wpa_cli reconfigure 5. check connection state.

Rebase. Remove "FORCE_STATIC_LIB = 1" in makefile.

Rebase.

1. Rebase. 2. Replace MOZ_NOT_REACHED() with MOZ_CRASH(). 3. Add code to read cert from NSS by name.

Rebase.

Rebase.

Rebase.

Rebase.

Comment on attachment 768799 [details] [diff] [review] Test Only - Disable keystore daemon. Review of attachment 768799 [details] [diff] [review]: ----------------------------------------------------------------- After test, replaced keystore still works without this patch. But we should suggest partners to disable keystore from android.

New IPC to replace android's keystore daemon, for reading CA in NSS to wpa_supplicant.

Server socket will be closed after client disconnect, so we should setup socket on every accept.

Implement keystore daemon in gecko, but only support get command. Because it uses NSS API to read CA from NSS DB, so also need bsmith's review on that part.

Bring up keystore ipc.

Rebase.

Rebase.

Rebase.

Renew make scripts.

Bad copy & paste...

Comment on attachment 805857 [details] [diff] [review] 0002. Setup socket after bind. I think it would be better if the other Kyle reviewed this. I'm not familiar with UnixSocket.

Comment on attachment 805857 [details] [diff] [review] 0002. Setup socket after bind. Review of attachment 805857 [details] [diff] [review]: ----------------------------------------------------------------- ::: ipc/unixsocket/UnixSocket.cpp @@ +526,5 @@ > + nsRefPtr<OnSocketEventTask> t = > + new OnSocketEventTask(this, OnSocketEventTask::CONNECT_ERROR); > + NS_DispatchToMainThread(t); > + return; > + } Pretty sure this would break bluetooth. SetUp (which is a really bad name, should probably be changed to something more descriptive :( ) is only meant to be called on the final, connected socket, not on the listening bound socket. If we need to prepare a socket after a bind, this might be a good place to add yet another virtual function to the Connector class.

Comment on attachment 805859 [details] [diff] [review] 0003. Implement keystore. Review of attachment 805859 [details] [diff] [review]: ----------------------------------------------------------------- I think the other Kyle should review this. I don't know much about UnixSocket. ::: ipc/keystore/KeyStore.cpp @@ +15,5 @@ > +#define LOG(args...) printf(args); > +#endif > + > +#include "jsfriendapi.h" > +#include "nsThreadUtils.h" // For NS_IsMainThread. NS_IsMainThread lives in MainThreadUtils.h now. @@ +16,5 @@ > +#endif > + > +#include "jsfriendapi.h" > +#include "nsThreadUtils.h" // For NS_IsMainThread. > +#include "KeyStore.h" Please include KeyStore.h first, before any other includes. @@ +22,5 @@ > +#include "plbase64.h" > +#include "cert.h" > +#include "certdb.h" > + > +USING_WORKERS_NAMESPACE Why do you need this? I don't see any worker code being used here. @@ +54,5 @@ > +bool > +KeyStoreConnector::CreateAddr(bool aIsServer, > + socklen_t& aAddrSize, > + sockaddr_any& aAddr, > + const char* aAddress) Line up the parameters please. ::: ipc/keystore/KeyStore.h @@ +6,5 @@ > + > +#ifndef mozilla_ipc_KeyStore_h > +#define mozilla_ipc_KeyStore_h 1 > + > +#include <mozilla/dom/workers/Workers.h> Why do you need this header? I don't think you're doing anything with web workers here. @@ +7,5 @@ > +#ifndef mozilla_ipc_KeyStore_h > +#define mozilla_ipc_KeyStore_h 1 > + > +#include <mozilla/dom/workers/Workers.h> > +#include <mozilla/ipc/UnixSocket.h> Usual style is "Foo.h" for mozilla headers.

Comment on attachment 805859 [details] [diff] [review] 0003. Implement keystore. Review of attachment 805859 [details] [diff] [review]: ----------------------------------------------------------------- ::: ipc/keystore/KeyStore.cpp @@ +275,5 @@ > +// Data response > +void > +KeyStore::SendData(const uint8_t *aData, int aLength) > +{ > + UnixSocketRawData* length = new UnixSocketRawData(2); Super-Nit: UnixSocketRawData has a constructor that will take void*'s and do the copy for you. Might save you some lines here and elsewhere.

On second thought, build unixsocket and keystore ipc while RIL or BT enabled(which indicates firefox OS), in case it affects other version.

Oops, wrong file..

Find a way which keystore works without modifying unix socket, and address reviewers' comments.

Add reviewer's name, and change index number.

Rebase.

Rebase.

Rebase.

Comment on attachment 809091 [details] [diff] [review] 0002. Implement keystore. V2 Review of attachment 809091 [details] [diff] [review]: ----------------------------------------------------------------- r=me ::: ipc/keystore/KeyStore.cpp @@ +232,5 @@ > + > +// Transform base64 certification data into DER format > +void > +KeyStore::FormatCaData(const uint8_t *caData, int caDataLength, const char *name, > + const uint8_t **formatData, int &formatDataLength) Supernit: We prefix arguments with "a" everywhere else in the file, but not here.

Rebase.

Address comment 44.

Rebase.

Try : https://tbpl.mozilla.org/?tree=Try&rev=ee1d021da390

https://hg.mozilla.org/integration/b2g-inbound/rev/fc2078bb8fb1 https://hg.mozilla.org/integration/b2g-inbound/rev/f671257977b2 https://hg.mozilla.org/integration/b2g-inbound/rev/1192f75d0d22

https://hg.mozilla.org/mozilla-central/rev/fc2078bb8fb1 https://hg.mozilla.org/mozilla-central/rev/f671257977b2 https://hg.mozilla.org/mozilla-central/rev/1192f75d0d22

Failed to building b2g-desktop when compiling ipc/keystore/KeyStore.cpp: ipc/keystore/KeyStore.cpp:40:30: error: 'unlink' was not declared in this scope ipc/keystore/KeyStore.h:37:31: error: 'NAME_MAX' was not declared in this scope

Comment on attachment 812433 [details] [diff] [review] 00001. Add skeleton for keystore. V2 Review of attachment 812433 [details] [diff] [review]: ----------------------------------------------------------------- ::: ipc/moz.build @@ +17,5 @@ > if CONFIG['MOZ_B2G_BT']: > DIRS += ['dbus'] > > if CONFIG['MOZ_B2G_RIL'] or CONFIG['MOZ_B2G_BT']: > + DIRS += ['unixsocket', 'keystore'] Don't know if there is other work-arounds, but I think moving keystore to |MOZ_WIDGET_TOOKIT == 'gonk'| section would be a better idea.