Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(780)

Side by Side Diff: chromeos/cert_loader.cc

Issue 2858113003: Enable device-wide EAP-TLS networks (Closed)
Patch Set: Addressed comments (and accidental rebase). Created 3 years, 7 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View unified diff | Download patch
OLDNEW
1 // Copyright (c) 2013 The Chromium Authors. All rights reserved. 1 // Copyright (c) 2013 The Chromium Authors. All rights reserved.
2 // Use of this source code is governed by a BSD-style license that can be 2 // Use of this source code is governed by a BSD-style license that can be
3 // found in the LICENSE file. 3 // found in the LICENSE file.
4 4
5 #include "chromeos/cert_loader.h" 5 #include "chromeos/cert_loader.h"
6 6
7 #include <algorithm> 7 #include <algorithm>
8 #include <utility> 8 #include <utility>
9 9
10 #include "base/bind.h" 10 #include "base/bind.h"
11 #include "base/location.h" 11 #include "base/location.h"
12 #include "base/memory/ptr_util.h" 12 #include "base/memory/ptr_util.h"
13 #include "base/strings/string_number_conversions.h" 13 #include "base/strings/string_number_conversions.h"
14 #include "base/task_scheduler/post_task.h" 14 #include "base/task_scheduler/post_task.h"
15 #include "crypto/nss_util.h" 15 #include "crypto/nss_util.h"
16 #include "crypto/scoped_nss_types.h" 16 #include "crypto/scoped_nss_types.h"
17 #include "net/cert/cert_database.h"
17 #include "net/cert/nss_cert_database.h" 18 #include "net/cert/nss_cert_database.h"
18 #include "net/cert/nss_cert_database_chromeos.h" 19 #include "net/cert/nss_cert_database_chromeos.h"
19 #include "net/cert/x509_certificate.h" 20 #include "net/cert/x509_certificate.h"
20 21
21 namespace chromeos { 22 namespace chromeos {
22 23
24 // Caches certificates from a NSSCertDatabase. Handles reloading of certificates
25 // on update notifications and provides status flags (loading / loaded).
26 // CertLoader can use multiple CertCaches to combine certificates from multiple
27 // sources.
28 class CertLoader::CertCache : public net::CertDatabase::Observer {
29 public:
30 explicit CertCache(base::RepeatingClosure certificates_updated_callback)
31 : certificates_updated_callback_(certificates_updated_callback),
32 cert_list_(base::MakeUnique<net::CertificateList>()),
33 weak_factory_(this) {}
34
35 ~CertCache() override {
36 net::CertDatabase::GetInstance()->RemoveObserver(this);
37 }
38
39 void StartWithNSSDB(net::NSSCertDatabase* nss_database) {
40 CHECK(!nss_database_);
emaxx 2017/05/11 02:57:46 nit: #include "base/logging.h"
pmarko 2017/05/11 11:49:18 Done.
41 nss_database_ = nss_database;
42
43 // Start observing cert database for changes.
44 // Observing net::CertDatabase is preferred over observing |nss_database_|
45 // directly, as |nss_database_| observers receive only events generated
46 // directly by |nss_database_|, so they may miss a few relevant ones.
47 // TODO(tbarzic): Once singleton NSSCertDatabase is removed, investigate if
48 // it would be OK to observe |nss_database_| directly; or change
49 // NSSCertDatabase to send notification on all relevant changes.
50 net::CertDatabase::GetInstance()->AddObserver(this);
51
52 LoadCertificates();
53 }
54
55 net::NSSCertDatabase* nss_database() { return nss_database_; }
56
57 // Trigger a certificate load. If a certificate loading task is already in
58 // progress, will start a reload once the current task is finished.
59 void LoadCertificates() {
emaxx 2017/05/11 02:57:46 nit: Make private?
pmarko 2017/05/11 11:49:18 Done.
60 CHECK(thread_checker_.CalledOnValidThread());
61 VLOG(1) << "LoadCertificates: " << certificates_update_running_;
62
63 if (certificates_update_running_) {
64 certificates_update_required_ = true;
65 return;
66 }
67
68 certificates_update_running_ = true;
69 certificates_update_required_ = false;
70
71 if (nss_database_) {
72 has_system_certificates_ =
73 static_cast<bool>(nss_database_->GetSystemSlot());
74 nss_database_->ListCerts(base::Bind(&CertCache::UpdateCertificates,
75 weak_factory_.GetWeakPtr()));
76 }
77 }
78
79 // Called if a certificate load task is finished.
80 void UpdateCertificates(std::unique_ptr<net::CertificateList> cert_list) {
emaxx 2017/05/11 02:57:46 nit: Make private?
pmarko 2017/05/11 11:49:18 Done.
81 CHECK(thread_checker_.CalledOnValidThread());
82 DCHECK(certificates_update_running_);
83 VLOG(1) << "UpdateCertificates: " << cert_list->size();
84
85 // Ignore any existing certificates.
86 cert_list_ = std::move(cert_list);
87
88 certificates_loaded_ = true;
89 certificates_updated_callback_.Run();
90
91 certificates_update_running_ = false;
92 if (certificates_update_required_)
93 LoadCertificates();
94 }
95
96 // net::CertDatabase::Observer
97 void OnCertDBChanged() override {
98 VLOG(1) << "OnCertDBChanged";
99 LoadCertificates();
100 }
101
102 const net::CertificateList& cert_list() const { return *cert_list_; }
103
104 bool CertificatesLoading() const {
emaxx 2017/05/11 02:57:46 nit: Rename to lowercase, as it's a simple accesso
pmarko 2017/05/11 11:49:18 Good point. I've chosen "initial_load_running" to
105 return nss_database_ && !certificates_loaded_;
106 }
107
108 bool certificates_loaded() const { return certificates_loaded_; }
109 bool has_system_certificates() const { return has_system_certificates_; }
110
111 private:
112 // To be called when certificates have been updated.
113 base::RepeatingClosure certificates_updated_callback_;
114
115 bool has_system_certificates_ = false;
116
117 // Flags describing current CertLoader state.
emaxx 2017/05/11 02:57:46 nit: Remove the reference to the CertLoader class.
pmarko 2017/05/11 11:49:18 Done. Also renamed certificates_loaded_ to initial
118 bool certificates_loaded_ = false;
119 bool certificates_update_required_ = false;
120 bool certificates_update_running_ = false;
121
122 // The user-specific NSS certificate database from which the certificates
emaxx 2017/05/11 02:57:46 nit: The "user-specific" characteristic is not act
pmarko 2017/05/11 11:49:18 Done.
123 // should be loaded.
124 net::NSSCertDatabase* nss_database_ = nullptr;
125
126 // Cached Certificates loaded from the database.
127 std::unique_ptr<net::CertificateList> cert_list_;
emaxx 2017/05/11 02:57:46 Is the smart pointer actually needed there?
pmarko 2017/05/11 11:49:18 Hrm. NSSCertDatabase->ListCerts calls its callback
emaxx 2017/05/11 14:36:52 I'm not insisting, but to me ridding of these smar
128
129 base::ThreadChecker thread_checker_;
130
131 base::WeakPtrFactory<CertCache> weak_factory_;
132 };
emaxx 2017/05/11 02:57:46 nit: DISALLOW_COPY_AND_ASSIGN (for consistency).
pmarko 2017/05/11 11:49:18 Done.
133
23 namespace { 134 namespace {
24 135
25 // Checks if |certificate| is on the given |slot|. 136 // Checks if |certificate| is on the given |slot|.
26 bool IsCertificateOnSlot(const net::X509Certificate* certificate, 137 bool IsCertificateOnSlot(const net::X509Certificate* certificate,
27 PK11SlotInfo* slot) { 138 PK11SlotInfo* slot) {
28 crypto::ScopedPK11SlotList slots_for_cert( 139 crypto::ScopedPK11SlotList slots_for_cert(
29 PK11_GetAllSlotsForCert(certificate->os_cert_handle(), nullptr)); 140 PK11_GetAllSlotsForCert(certificate->os_cert_handle(), nullptr));
30 if (!slots_for_cert) 141 if (!slots_for_cert)
31 return false; 142 return false;
32 143
(...skipping 57 matching lines...) Expand 10 before | Expand all | Expand 10 after
90 CHECK(g_cert_loader) << "CertLoader::Get() called before Initialize()"; 201 CHECK(g_cert_loader) << "CertLoader::Get() called before Initialize()";
91 return g_cert_loader; 202 return g_cert_loader;
92 } 203 }
93 204
94 // static 205 // static
95 bool CertLoader::IsInitialized() { 206 bool CertLoader::IsInitialized() {
96 return g_cert_loader; 207 return g_cert_loader;
97 } 208 }
98 209
99 CertLoader::CertLoader() 210 CertLoader::CertLoader()
100 : certificates_loaded_(false), 211 : pending_initial_load_(true),
101 certificates_update_required_(false), 212 system_cert_cache_(base::MakeUnique<CertCache>(
102 certificates_update_running_(false), 213 base::BindRepeating(&CertLoader::CacheUpdated,
103 database_(nullptr), 214 base::Unretained(this)))),
215 user_cert_cache_(base::MakeUnique<CertCache>(
216 base::BindRepeating(&CertLoader::CacheUpdated,
217 base::Unretained(this)))),
104 all_certs_(base::MakeUnique<net::CertificateList>()), 218 all_certs_(base::MakeUnique<net::CertificateList>()),
219 system_certs_(base::MakeUnique<net::CertificateList>()),
105 weak_factory_(this) {} 220 weak_factory_(this) {}
106 221
107 CertLoader::~CertLoader() { 222 CertLoader::~CertLoader() {
108 net::CertDatabase::GetInstance()->RemoveObserver(this);
109 } 223 }
110 224
111 void CertLoader::StartWithNSSDB(net::NSSCertDatabase* database) { 225 void CertLoader::StartWithSystemNSSDB(
112 CHECK(!database_); 226 net::NSSCertDatabase* system_slot_database) {
113 database_ = database; 227 system_cert_cache_->StartWithNSSDB(system_slot_database);
228 }
114 229
115 // Start observing cert database for changes. 230 void CertLoader::StartWithUserNSSDB(net::NSSCertDatabase* user_database) {
116 // Observing net::CertDatabase is preferred over observing |database_| 231 user_cert_cache_->StartWithNSSDB(user_database);
117 // directly, as |database_| observers receive only events generated directly
118 // by |database_|, so they may miss a few relevant ones.
119 // TODO(tbarzic): Once singleton NSSCertDatabase is removed, investigate if
120 // it would be OK to observe |database_| directly; or change NSSCertDatabase
121 // to send notification on all relevant changes.
122 net::CertDatabase::GetInstance()->AddObserver(this);
123
124 LoadCertificates();
125 } 232 }
126 233
127 void CertLoader::AddObserver(CertLoader::Observer* observer) { 234 void CertLoader::AddObserver(CertLoader::Observer* observer) {
128 observers_.AddObserver(observer); 235 observers_.AddObserver(observer);
129 } 236 }
130 237
131 void CertLoader::RemoveObserver(CertLoader::Observer* observer) { 238 void CertLoader::RemoveObserver(CertLoader::Observer* observer) {
132 observers_.RemoveObserver(observer); 239 observers_.RemoveObserver(observer);
133 } 240 }
134 241
135 // static 242 // static
136 bool CertLoader::IsCertificateHardwareBacked(const net::X509Certificate* cert) { 243 bool CertLoader::IsCertificateHardwareBacked(const net::X509Certificate* cert) {
137 if (g_force_hardware_backed_for_test) 244 if (g_force_hardware_backed_for_test)
138 return true; 245 return true;
139 PK11SlotInfo* slot = cert->os_cert_handle()->slot; 246 PK11SlotInfo* slot = cert->os_cert_handle()->slot;
140 return slot && PK11_IsHW(slot); 247 return slot && PK11_IsHW(slot);
141 } 248 }
142 249
143 bool CertLoader::CertificatesLoading() const { 250 bool CertLoader::CertificatesLoading() const {
144 return database_ && !certificates_loaded_; 251 return system_cert_cache_->CertificatesLoading() ||
252 user_cert_cache_->CertificatesLoading();
253 }
254
255 bool CertLoader::certificates_loaded() const {
256 return system_cert_cache_->certificates_loaded() ||
257 user_cert_cache_->certificates_loaded();
145 } 258 }
146 259
147 // static 260 // static
148 void CertLoader::ForceHardwareBackedForTesting() { 261 void CertLoader::ForceHardwareBackedForTesting() {
149 g_force_hardware_backed_for_test = true; 262 g_force_hardware_backed_for_test = true;
150 } 263 }
151 264
152 // static 265 // static
153 // 266 //
154 // For background see this discussion on dev-tech-crypto.lists.mozilla.org: 267 // For background see this discussion on dev-tech-crypto.lists.mozilla.org:
(...skipping 21 matching lines...) Expand all
176 std::string pkcs11_id; 289 std::string pkcs11_id;
177 if (sec_item) { 290 if (sec_item) {
178 pkcs11_id = base::HexEncode(sec_item->data, sec_item->len); 291 pkcs11_id = base::HexEncode(sec_item->data, sec_item->len);
179 SECITEM_FreeItem(sec_item, PR_TRUE); 292 SECITEM_FreeItem(sec_item, PR_TRUE);
180 } 293 }
181 SECKEY_DestroyPrivateKey(priv_key); 294 SECKEY_DestroyPrivateKey(priv_key);
182 295
183 return pkcs11_id; 296 return pkcs11_id;
184 } 297 }
185 298
186 void CertLoader::LoadCertificates() { 299 void CertLoader::CacheUpdated() {
187 DCHECK(thread_checker_.CalledOnValidThread()); 300 DCHECK(thread_checker_.CalledOnValidThread());
188 VLOG(1) << "LoadCertificates: " << certificates_update_running_; 301 VLOG(1) << "CacheUpdated";
189 302
190 if (certificates_update_running_) { 303 if (user_cert_cache_->has_system_certificates()) {
191 certificates_update_required_ = true; 304 // The user's cert cache also contains system certificates.
192 return; 305 // Filter those.
306 crypto::ScopedPK11Slot system_slot =
307 user_cert_cache_->nss_database()->GetSystemSlot();
308 DCHECK(system_slot);
309 std::unique_ptr<net::CertificateList> all_certs =
emaxx 2017/05/11 02:57:46 Can this logic lead to problems in the case when u
pmarko 2017/05/11 11:49:18 Very good find! I'd say that this line can stay as
310 base::MakeUnique<net::CertificateList>(user_cert_cache_->cert_list());
311 base::PostTaskWithTraitsAndReplyWithResult(
312 FROM_HERE,
313 {base::MayBlock(), base::TaskShutdownBehavior::CONTINUE_ON_SHUTDOWN},
314 base::BindOnce(&FilterSystemTokenCertificates,
315 base::Unretained(all_certs.get()),
316 std::move(system_slot)),
317 base::BindOnce(&CertLoader::UpdateCertificates,
318 weak_factory_.GetWeakPtr(), std::move(all_certs)));
319 } else {
320 // The user's cert cache does not contain system certificates.
321 std::unique_ptr<net::CertificateList> system_certs =
emaxx 2017/05/11 02:57:46 nit: BTW, wouldn't it be possible to drop the smar
pmarko 2017/05/11 11:49:18 The only part where this is actually useful is wit
322 base::MakeUnique<net::CertificateList>(system_cert_cache_->cert_list());
323 std::unique_ptr<net::CertificateList> all_certs =
324 base::MakeUnique<net::CertificateList>(user_cert_cache_->cert_list());
325 all_certs->insert(all_certs->end(), system_certs->begin(),
326 system_certs->end());
327 UpdateCertificates(std::move(all_certs), std::move(system_certs));
193 } 328 }
194
195 certificates_update_running_ = true;
196 certificates_update_required_ = false;
197
198 database_->ListCerts(
199 base::Bind(&CertLoader::CertificatesLoaded, weak_factory_.GetWeakPtr()));
200 }
201
202 void CertLoader::CertificatesLoaded(
203 std::unique_ptr<net::CertificateList> all_certs) {
204 DCHECK(thread_checker_.CalledOnValidThread());
205 VLOG(1) << "CertificatesLoaded: " << all_certs->size();
206
207 crypto::ScopedPK11Slot system_slot = database_->GetSystemSlot();
208 base::PostTaskWithTraitsAndReplyWithResult(
209 FROM_HERE,
210 {base::MayBlock(), base::TaskShutdownBehavior::CONTINUE_ON_SHUTDOWN},
211 base::BindOnce(&FilterSystemTokenCertificates,
212 base::Unretained(all_certs.get()), std::move(system_slot)),
213 base::BindOnce(&CertLoader::UpdateCertificates,
214 weak_factory_.GetWeakPtr(), std::move(all_certs)));
215 } 329 }
216 330
217 void CertLoader::UpdateCertificates( 331 void CertLoader::UpdateCertificates(
218 std::unique_ptr<net::CertificateList> all_certs, 332 std::unique_ptr<net::CertificateList> all_certs,
219 std::unique_ptr<net::CertificateList> system_certs) { 333 std::unique_ptr<net::CertificateList> system_certs) {
220 DCHECK(thread_checker_.CalledOnValidThread()); 334 DCHECK(thread_checker_.CalledOnValidThread());
221 DCHECK(certificates_update_running_); 335 bool initial_load = pending_initial_load_;
336 pending_initial_load_ = false;
337
222 VLOG(1) << "UpdateCertificates: " << all_certs->size() << " (" 338 VLOG(1) << "UpdateCertificates: " << all_certs->size() << " ("
223 << system_certs->size() << " on system slot)"; 339 << system_certs->size() << " on system slot)"
340 << ", initial_load=" << initial_load;
224 341
225 // Ignore any existing certificates. 342 // Ignore any existing certificates.
226 all_certs_ = std::move(all_certs); 343 all_certs_ = std::move(all_certs);
227 system_certs_ = std::move(system_certs); 344 system_certs_ = std::move(system_certs);
228 345
229 bool initial_load = !certificates_loaded_;
230 certificates_loaded_ = true;
231 NotifyCertificatesLoaded(initial_load); 346 NotifyCertificatesLoaded(initial_load);
232
233 certificates_update_running_ = false;
234 if (certificates_update_required_)
235 LoadCertificates();
236 } 347 }
237 348
238 void CertLoader::NotifyCertificatesLoaded(bool initial_load) { 349 void CertLoader::NotifyCertificatesLoaded(bool initial_load) {
239 for (auto& observer : observers_) 350 for (auto& observer : observers_)
240 observer.OnCertificatesLoaded(*all_certs_, initial_load); 351 observer.OnCertificatesLoaded(*all_certs_, initial_load);
241 } 352 }
242 353
243 void CertLoader::OnCertDBChanged() {
244 VLOG(1) << "OnCertDBChanged";
245 LoadCertificates();
246 }
247
248 } // namespace chromeos 354 } // namespace chromeos
OLDNEW

Powered by Google App Engine
This is Rietveld 408576698