Skip to content

Commit a2d08d5

Browse files
RaisinTendanielleadams
RaisinTen
authored and
danielleadams
committed
crypto: add internal error codes
PR-URL: #37650 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent e55058f commit a2d08d5

File tree

6 files changed

+110
-53
lines changed

6 files changed

+110
-53
lines changed

‎src/crypto/crypto_cipher.h

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -227,18 +227,18 @@ class CipherJob final : public CryptoJob<CipherTraits> {
227227
// Success!
228228
return;
229229
}
230-
CryptoErrorVector* errors = CryptoJob<CipherTraits>::errors();
230+
CryptoErrorStore* errors = CryptoJob<CipherTraits>::errors();
231231
errors->Capture();
232-
if (errors->empty()) {
232+
if (errors->Empty()) {
233233
switch (status) {
234234
case WebCryptoCipherStatus::OK:
235235
UNREACHABLE();
236236
break;
237237
case WebCryptoCipherStatus::INVALID_KEY_TYPE:
238-
errors->emplace_back("Invalid key type.");
238+
errors->Insert(NodeCryptoError::INVALID_KEY_TYPE);
239239
break;
240240
case WebCryptoCipherStatus::FAILED:
241-
errors->emplace_back("Cipher job failed.");
241+
errors->Insert(NodeCryptoError::CIPHER_JOB_FAILED);
242242
break;
243243
}
244244
}
@@ -248,17 +248,17 @@ class CipherJob final : public CryptoJob<CipherTraits> {
248248
v8::Local<v8::Value>* err,
249249
v8::Local<v8::Value>* result) override {
250250
Environment* env = AsyncWrap::env();
251-
CryptoErrorVector* errors = CryptoJob<CipherTraits>::errors();
251+
CryptoErrorStore* errors = CryptoJob<CipherTraits>::errors();
252252
if (out_.size() > 0) {
253-
CHECK(errors->empty());
253+
CHECK(errors->Empty());
254254
*err = v8::Undefined(env->isolate());
255255
*result = out_.ToArrayBuffer(env);
256256
return v8::Just(!result->IsEmpty());
257257
}
258258

259-
if (errors->empty())
259+
if (errors->Empty())
260260
errors->Capture();
261-
CHECK(!errors->empty());
261+
CHECK(!errors->Empty());
262262
*result = v8::Undefined(env->isolate());
263263
return v8::Just(errors->ToException(env).ToLocal(err));
264264
}

‎src/crypto/crypto_context.cc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -551,7 +551,7 @@ void SecureContext::SetEngineKey(const FunctionCallbackInfo<Value>& args) {
551551

552552
CHECK_EQ(args.Length(), 2);
553553

554-
CryptoErrorVector errors;
554+
CryptoErrorStore errors;
555555
Utf8Value engine_id(env->isolate(), args[1]);
556556
EnginePointer engine = LoadEngineById(*engine_id, &errors);
557557
if (!engine) {
@@ -987,7 +987,7 @@ void SecureContext::SetClientCertEngine(
987987
// support multiple calls to SetClientCertEngine.
988988
CHECK(!sc->client_cert_engine_provided_);
989989

990-
CryptoErrorVector errors;
990+
CryptoErrorStore errors;
991991
const Utf8Value engine_id(env->isolate(), args[0]);
992992
EnginePointer engine = LoadEngineById(*engine_id, &errors);
993993
if (!engine) {

‎src/crypto/crypto_keygen.h

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -82,10 +82,10 @@ class KeyGenJob final : public CryptoJob<KeyGenTraits> {
8282
// Success!
8383
break;
8484
case KeyGenJobStatus::FAILED: {
85-
CryptoErrorVector* errors = CryptoJob<KeyGenTraits>::errors();
85+
CryptoErrorStore* errors = CryptoJob<KeyGenTraits>::errors();
8686
errors->Capture();
87-
if (errors->empty())
88-
errors->push_back(std::string("Key generation job failed"));
87+
if (errors->Empty())
88+
errors->Insert(NodeCryptoError::KEY_GENERATION_JOB_FAILED);
8989
}
9090
}
9191
}
@@ -94,17 +94,17 @@ class KeyGenJob final : public CryptoJob<KeyGenTraits> {
9494
v8::Local<v8::Value>* err,
9595
v8::Local<v8::Value>* result) override {
9696
Environment* env = AsyncWrap::env();
97-
CryptoErrorVector* errors = CryptoJob<KeyGenTraits>::errors();
97+
CryptoErrorStore* errors = CryptoJob<KeyGenTraits>::errors();
9898
AdditionalParams* params = CryptoJob<KeyGenTraits>::params();
9999
if (status_ == KeyGenJobStatus::OK &&
100100
LIKELY(!KeyGenTraits::EncodeKey(env, params, result).IsNothing())) {
101101
*err = Undefined(env->isolate());
102102
return v8::Just(true);
103103
}
104104

105-
if (errors->empty())
105+
if (errors->Empty())
106106
errors->Capture();
107-
CHECK(!errors->empty());
107+
CHECK(!errors->Empty());
108108
*result = Undefined(env->isolate());
109109
return v8::Just(errors->ToException(env).ToLocal(err));
110110
}

‎src/crypto/crypto_keys.h

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -347,18 +347,18 @@ class KeyExportJob final : public CryptoJob<KeyExportTraits> {
347347
// Success!
348348
return;
349349
}
350-
CryptoErrorVector* errors = CryptoJob<KeyExportTraits>::errors();
350+
CryptoErrorStore* errors = CryptoJob<KeyExportTraits>::errors();
351351
errors->Capture();
352-
if (errors->empty()) {
352+
if (errors->Empty()) {
353353
switch (status) {
354354
case WebCryptoKeyExportStatus::OK:
355355
UNREACHABLE();
356356
break;
357357
case WebCryptoKeyExportStatus::INVALID_KEY_TYPE:
358-
errors->emplace_back("Invalid key type.");
358+
errors->Insert(NodeCryptoError::INVALID_KEY_TYPE);
359359
break;
360360
case WebCryptoKeyExportStatus::FAILED:
361-
errors->emplace_back("Cipher job failed.");
361+
errors->Insert(NodeCryptoError::CIPHER_JOB_FAILED);
362362
break;
363363
}
364364
}
@@ -368,17 +368,17 @@ class KeyExportJob final : public CryptoJob<KeyExportTraits> {
368368
v8::Local<v8::Value>* err,
369369
v8::Local<v8::Value>* result) override {
370370
Environment* env = AsyncWrap::env();
371-
CryptoErrorVector* errors = CryptoJob<KeyExportTraits>::errors();
371+
CryptoErrorStore* errors = CryptoJob<KeyExportTraits>::errors();
372372
if (out_.size() > 0) {
373-
CHECK(errors->empty());
373+
CHECK(errors->Empty());
374374
*err = v8::Undefined(env->isolate());
375375
*result = out_.ToArrayBuffer(env);
376376
return v8::Just(!result->IsEmpty());
377377
}
378378

379-
if (errors->empty())
379+
if (errors->Empty())
380380
errors->Capture();
381-
CHECK(!errors->empty());
381+
CHECK(!errors->Empty());
382382
*result = v8::Undefined(env->isolate());
383383
return v8::Just(errors->ToException(env).ToLocal(err));
384384
}

‎src/crypto/crypto_util.cc

Lines changed: 25 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -187,44 +187,52 @@ void TestFipsCrypto(const v8::FunctionCallbackInfo<v8::Value>& args) {
187187
args.GetReturnValue().Set(enabled);
188188
}
189189

190-
void CryptoErrorVector::Capture() {
191-
clear();
192-
while (auto err = ERR_get_error()) {
190+
void CryptoErrorStore::Capture() {
191+
errors_.clear();
192+
while (const uint32_t err = ERR_get_error()) {
193193
char buf[256];
194194
ERR_error_string_n(err, buf, sizeof(buf));
195-
push_back(buf);
195+
errors_.emplace_back(buf);
196196
}
197-
std::reverse(begin(), end());
197+
std::reverse(std::begin(errors_), std::end(errors_));
198198
}
199199

200-
MaybeLocal<Value> CryptoErrorVector::ToException(
200+
bool CryptoErrorStore::Empty() const {
201+
return errors_.empty();
202+
}
203+
204+
MaybeLocal<Value> CryptoErrorStore::ToException(
201205
Environment* env,
202206
Local<String> exception_string) const {
203207
if (exception_string.IsEmpty()) {
204-
CryptoErrorVector copy(*this);
205-
if (copy.empty()) copy.push_back("no error"); // But possibly a bug...
208+
CryptoErrorStore copy(*this);
209+
if (copy.Empty()) {
210+
// But possibly a bug...
211+
copy.Insert(NodeCryptoError::OK);
212+
}
206213
// Use last element as the error message, everything else goes
207214
// into the .opensslErrorStack property on the exception object.
215+
const std::string& last_error_string = copy.errors_.back();
208216
Local<String> exception_string;
209217
if (!String::NewFromUtf8(
210218
env->isolate(),
211-
copy.back().data(),
219+
last_error_string.data(),
212220
NewStringType::kNormal,
213-
copy.back().size()).ToLocal(&exception_string)) {
221+
last_error_string.size()).ToLocal(&exception_string)) {
214222
return MaybeLocal<Value>();
215223
}
216-
copy.pop_back();
224+
copy.errors_.pop_back();
217225
return copy.ToException(env, exception_string);
218226
}
219227

220228
Local<Value> exception_v = Exception::Error(exception_string);
221229
CHECK(!exception_v.IsEmpty());
222230

223-
if (!empty()) {
231+
if (!Empty()) {
224232
CHECK(exception_v->IsObject());
225233
Local<Object> exception = exception_v.As<Object>();
226234
Local<Value> stack;
227-
if (!ToV8Value(env->context(), *this).ToLocal(&stack) ||
235+
if (!ToV8Value(env->context(), errors_).ToLocal(&stack) ||
228236
exception->Set(env->context(), env->openssl_error_stack(), stack)
229237
.IsNothing()) {
230238
return MaybeLocal<Value>();
@@ -509,7 +517,7 @@ void ThrowCryptoError(Environment* env,
509517
Local<Object> obj;
510518
if (!String::NewFromUtf8(env->isolate(), message).ToLocal(&exception_string))
511519
return;
512-
CryptoErrorVector errors;
520+
CryptoErrorStore errors;
513521
errors.Capture();
514522
if (!errors.ToException(env, exception_string).ToLocal(&exception) ||
515523
!exception->ToObject(env->context()).ToLocal(&obj) ||
@@ -520,7 +528,7 @@ void ThrowCryptoError(Environment* env,
520528
}
521529

522530
#ifndef OPENSSL_NO_ENGINE
523-
EnginePointer LoadEngineById(const char* id, CryptoErrorVector* errors) {
531+
EnginePointer LoadEngineById(const char* id, CryptoErrorStore* errors) {
524532
MarkPopErrorOnReturn mark_pop_error_on_return;
525533

526534
EnginePointer engine(ENGINE_by_id(id));
@@ -539,14 +547,14 @@ EnginePointer LoadEngineById(const char* id, CryptoErrorVector* errors) {
539547
if (ERR_get_error() != 0) {
540548
errors->Capture();
541549
} else {
542-
errors->push_back(std::string("Engine \"") + id + "\" was not found");
550+
errors->Insert(NodeCryptoError::ENGINE_NOT_FOUND, id);
543551
}
544552
}
545553

546554
return engine;
547555
}
548556

549-
bool SetEngine(const char* id, uint32_t flags, CryptoErrorVector* errors) {
557+
bool SetEngine(const char* id, uint32_t flags, CryptoErrorStore* errors) {
550558
ClearErrorOnReturn clear_error_on_return;
551559
EnginePointer engine = LoadEngineById(id, errors);
552560
if (!engine)

‎src/crypto/crypto_util.h

Lines changed: 61 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -159,15 +159,64 @@ void Decode(const v8::FunctionCallbackInfo<v8::Value>& args,
159159
}
160160
}
161161

162+
enum class NodeCryptoError {
163+
CIPHER_JOB_FAILED,
164+
DERIVING_BITS_FAILED,
165+
ENGINE_NOT_FOUND,
166+
INVALID_KEY_TYPE,
167+
KEY_GENERATION_JOB_FAILED,
168+
OK
169+
};
170+
162171
// Utility struct used to harvest error information from openssl's error stack
163-
struct CryptoErrorVector : public std::vector<std::string> {
172+
struct CryptoErrorStore final : public MemoryRetainer {
173+
public:
164174
void Capture();
165175

176+
bool Empty() const;
177+
178+
template <typename... Args>
179+
void Insert(const NodeCryptoError error, Args&&... args);
180+
166181
v8::MaybeLocal<v8::Value> ToException(
167182
Environment* env,
168183
v8::Local<v8::String> exception_string = v8::Local<v8::String>()) const;
184+
185+
SET_NO_MEMORY_INFO()
186+
SET_MEMORY_INFO_NAME(CryptoErrorStore);
187+
SET_SELF_SIZE(CryptoErrorStore);
188+
189+
private:
190+
std::vector<std::string> errors_;
169191
};
170192

193+
template <typename... Args>
194+
void CryptoErrorStore::Insert(const NodeCryptoError error, Args&&... args) {
195+
const char* error_string = nullptr;
196+
switch (error) {
197+
case NodeCryptoError::CIPHER_JOB_FAILED:
198+
error_string = "Cipher job failed";
199+
break;
200+
case NodeCryptoError::DERIVING_BITS_FAILED:
201+
error_string = "Deriving bits failed";
202+
break;
203+
case NodeCryptoError::ENGINE_NOT_FOUND:
204+
error_string = "Engine \"%s\" was not found";
205+
break;
206+
case NodeCryptoError::INVALID_KEY_TYPE:
207+
error_string = "Invalid key type";
208+
break;
209+
case NodeCryptoError::KEY_GENERATION_JOB_FAILED:
210+
error_string = "Key generation failed";
211+
break;
212+
case NodeCryptoError::OK:
213+
error_string = "Ok";
214+
break;
215+
}
216+
errors_.emplace_back(SPrintF(error_string,
217+
std::forward<Args>(args)...));
218+
}
219+
171220
template <typename T>
172221
T* MallocOpenSSL(size_t count) {
173222
void* mem = OPENSSL_malloc(MultiplyWithOverflowCheck(count, sizeof(T)));
@@ -320,7 +369,7 @@ class CryptoJob : public AsyncWrap, public ThreadPoolWork {
320369

321370
CryptoJobMode mode() const { return mode_; }
322371

323-
CryptoErrorVector* errors() { return &errors_; }
372+
CryptoErrorStore* errors() { return &errors_; }
324373

325374
AdditionalParams* params() { return &params_; }
326375

@@ -364,7 +413,7 @@ class CryptoJob : public AsyncWrap, public ThreadPoolWork {
364413

365414
private:
366415
const CryptoJobMode mode_;
367-
CryptoErrorVector errors_;
416+
CryptoErrorStore errors_;
368417
AdditionalParams params_;
369418
};
370419

@@ -412,10 +461,10 @@ class DeriveBitsJob final : public CryptoJob<DeriveBitsTraits> {
412461
if (!DeriveBitsTraits::DeriveBits(
413462
AsyncWrap::env(),
414463
*CryptoJob<DeriveBitsTraits>::params(), &out_)) {
415-
CryptoErrorVector* errors = CryptoJob<DeriveBitsTraits>::errors();
464+
CryptoErrorStore* errors = CryptoJob<DeriveBitsTraits>::errors();
416465
errors->Capture();
417-
if (errors->empty())
418-
errors->push_back("Deriving bits failed");
466+
if (errors->Empty())
467+
errors->Insert(NodeCryptoError::DERIVING_BITS_FAILED);
419468
return;
420469
}
421470
success_ = true;
@@ -425,9 +474,9 @@ class DeriveBitsJob final : public CryptoJob<DeriveBitsTraits> {
425474
v8::Local<v8::Value>* err,
426475
v8::Local<v8::Value>* result) override {
427476
Environment* env = AsyncWrap::env();
428-
CryptoErrorVector* errors = CryptoJob<DeriveBitsTraits>::errors();
477+
CryptoErrorStore* errors = CryptoJob<DeriveBitsTraits>::errors();
429478
if (success_) {
430-
CHECK(errors->empty());
479+
CHECK(errors->Empty());
431480
*err = v8::Undefined(env->isolate());
432481
return DeriveBitsTraits::EncodeOutput(
433482
env,
@@ -436,9 +485,9 @@ class DeriveBitsJob final : public CryptoJob<DeriveBitsTraits> {
436485
result);
437486
}
438487

439-
if (errors->empty())
488+
if (errors->Empty())
440489
errors->Capture();
441-
CHECK(!errors->empty());
490+
CHECK(!errors->Empty());
442491
*result = v8::Undefined(env->isolate());
443492
return v8::Just(errors->ToException(env).ToLocal(err));
444493
}
@@ -505,12 +554,12 @@ struct EnginePointer {
505554
}
506555
};
507556

508-
EnginePointer LoadEngineById(const char* id, CryptoErrorVector* errors);
557+
EnginePointer LoadEngineById(const char* id, CryptoErrorStore* errors);
509558

510559
bool SetEngine(
511560
const char* id,
512561
uint32_t flags,
513-
CryptoErrorVector* errors = nullptr);
562+
CryptoErrorStore* errors = nullptr);
514563

515564
void SetEngine(const v8::FunctionCallbackInfo<v8::Value>& args);
516565
#endif // !OPENSSL_NO_ENGINE

0 commit comments

Comments
 (0)