Skip to content

fix(crypto): remove verifyError from DiffieHellman.prototype #18710

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Jarred-Sumner merged 3 commits into from
Apr 2, 2025
Merged

fix(crypto): remove verifyError from DiffieHellman.prototype #18710

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2a572ff
remove verifyError from prototype
dylan-conway Apr 1, 2025
bab312b
Merge branch 'main' into dylan/fix-diffiehellman-verifyError
dylan-conway Apr 1, 2025
2f51be2
maybe transition
dylan-conway Apr 2, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions src/bun.js/bindings/node/crypto/JSDiffieHellman.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,19 @@
#include <JavaScriptCore/LazyClassStructureInlines.h>
#include <JavaScriptCore/FunctionPrototype.h>
#include <JavaScriptCore/ObjectPrototype.h>
#include "JSDOMExceptionHandling.h"

namespace Bun {

JSC_DECLARE_HOST_FUNCTION(jsDiffieHellmanGetter_verifyError);

const JSC::ClassInfo JSDiffieHellman::s_info = { "DiffieHellman"_s, &Base::s_info, nullptr, nullptr, CREATE_METHOD_TABLE(JSDiffieHellman) };

void JSDiffieHellman::finishCreation(JSC::VM& vm, JSC::JSGlobalObject* globalObject)
{
Base::finishCreation(vm);
JSC_NATIVE_GETTER("verifyError"_s, jsDiffieHellmanGetter_verifyError, PropertyAttribute::ReadOnly | PropertyAttribute::Accessor);

m_sizeForGC = this->m_dh.size();
vm.heap.reportExtraMemoryAllocated(this, m_sizeForGC);
}
Expand Down Expand Up @@ -47,4 +52,26 @@ void setupDiffieHellmanClassStructure(JSC::LazyClassStructure::Initializer& init
init.setConstructor(constructor);
}

JSC_DEFINE_HOST_FUNCTION(jsDiffieHellmanGetter_verifyError, (JSC::JSGlobalObject * globalObject, JSC::CallFrame* callFrame))
{
JSC::VM& vm = globalObject->vm();
auto scope = DECLARE_THROW_SCOPE(vm);

JSValue thisValue = callFrame->thisValue();

JSDiffieHellman* thisObject = JSC::jsDynamicCast<JSDiffieHellman*>(thisValue);
if (UNLIKELY(!thisObject)) {
throwVMTypeError(globalObject, scope);
return {};
}

auto& dh = thisObject->getImpl();
auto result = dh.check();
if (result == ncrypto::DHPointer::CheckResult::CHECK_FAILED) {
return Bun::ERR::CRYPTO_OPERATION_FAILED(scope, globalObject, "Checking DH parameters failed"_s);
}

return JSC::JSValue::encode(JSC::jsNumber(static_cast<int>(result)));
}

} // namespace Bun
24 changes: 24 additions & 0 deletions src/bun.js/bindings/node/crypto/JSDiffieHellmanGroup.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,14 @@

namespace Bun {

JSC_DECLARE_HOST_FUNCTION(jsDiffieHellmanGroupGetter_verifyError);

const JSC::ClassInfo JSDiffieHellmanGroup::s_info = { "DiffieHellmanGroup"_s, &Base::s_info, nullptr, nullptr, CREATE_METHOD_TABLE(JSDiffieHellmanGroup) };

void JSDiffieHellmanGroup::finishCreation(JSC::VM& vm, JSC::JSGlobalObject* globalObject)
{
Base::finishCreation(vm);
JSC_NATIVE_GETTER("verifyError"_s, jsDiffieHellmanGroupGetter_verifyError, PropertyAttribute::ReadOnly | PropertyAttribute::Accessor);

m_sizeForGC = this->m_dh.size();
vm.heap.reportExtraMemoryAllocated(this, m_sizeForGC);
Expand Down Expand Up @@ -48,4 +51,25 @@ void setupDiffieHellmanGroupClassStructure(JSC::LazyClassStructure::Initializer&
init.setConstructor(constructor);
}

JSC_DEFINE_HOST_FUNCTION(jsDiffieHellmanGroupGetter_verifyError, (JSC::JSGlobalObject * globalObject, JSC::CallFrame* callFrame))
{
JSC::VM& vm = globalObject->vm();
auto scope = DECLARE_THROW_SCOPE(vm);

JSValue thisValue = callFrame->thisValue();

JSDiffieHellmanGroup* thisObject = jsDynamicCast<JSDiffieHellmanGroup*>(thisValue);
if (!thisObject) {
throwVMTypeError(globalObject, scope);
}

auto& dh = thisObject->getImpl();
auto result = dh.check();
if (result == ncrypto::DHPointer::CheckResult::CHECK_FAILED) {
return ERR::CRYPTO_OPERATION_FAILED(scope, globalObject, "Checking DH parameters failed"_s);
}

return JSValue::encode(JSC::jsNumber(static_cast<int>(result)));
}

} // namespace Bun
22 changes: 0 additions & 22 deletions src/bun.js/bindings/node/crypto/JSDiffieHellmanPrototype.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ JSC_DECLARE_HOST_FUNCTION(jsDiffieHellmanProtoFuncGetPublicKey);
JSC_DECLARE_HOST_FUNCTION(jsDiffieHellmanProtoFuncGetPrivateKey);
JSC_DECLARE_HOST_FUNCTION(jsDiffieHellmanProtoFuncSetPublicKey);
JSC_DECLARE_HOST_FUNCTION(jsDiffieHellmanProtoFuncSetPrivateKey);
JSC_DECLARE_CUSTOM_GETTER(jsDiffieHellmanGetter_verifyError);

const JSC::ClassInfo JSDiffieHellmanPrototype::s_info = { "DiffieHellman"_s, &Base::s_info, nullptr, nullptr, CREATE_METHOD_TABLE(JSDiffieHellmanPrototype) };

Expand All @@ -32,7 +31,6 @@ static const JSC::HashTableValue JSDiffieHellmanPrototypeTableValues[] = {
{ "getPrivateKey"_s, static_cast<unsigned>(JSC::PropertyAttribute::Function), JSC::NoIntrinsic, { JSC::HashTableValue::NativeFunctionType, jsDiffieHellmanProtoFuncGetPrivateKey, 0 } },
{ "setPublicKey"_s, static_cast<unsigned>(JSC::PropertyAttribute::Function), JSC::NoIntrinsic, { JSC::HashTableValue::NativeFunctionType, jsDiffieHellmanProtoFuncSetPublicKey, 1 } },
{ "setPrivateKey"_s, static_cast<unsigned>(JSC::PropertyAttribute::Function), JSC::NoIntrinsic, { JSC::HashTableValue::NativeFunctionType, jsDiffieHellmanProtoFuncSetPrivateKey, 1 } },
{ "verifyError"_s, static_cast<unsigned>(JSC::PropertyAttribute::ReadOnly | JSC::PropertyAttribute::CustomAccessor), JSC::NoIntrinsic, { JSC::HashTableValue::GetterSetterType, jsDiffieHellmanGetter_verifyError, 0 } },
};

void JSDiffieHellmanPrototype::finishCreation(JSC::VM& vm)
Expand Down Expand Up @@ -83,24 +81,4 @@ JSC_DEFINE_HOST_FUNCTION(jsDiffieHellmanProtoFuncSetPrivateKey, (JSC::JSGlobalOb
return jsDiffieHellmanProtoFuncSetPrivateKeyTemplate<JSDiffieHellman>(globalObject, callFrame);
}

JSC_DEFINE_CUSTOM_GETTER(jsDiffieHellmanGetter_verifyError, (JSC::JSGlobalObject * globalObject, JSC::EncodedJSValue thisValue, JSC::PropertyName))
{
JSC::VM& vm = globalObject->vm();
auto scope = DECLARE_THROW_SCOPE(vm);

auto* thisObject = JSC::jsDynamicCast<JSDiffieHellman*>(JSC::JSValue::decode(thisValue));
if (UNLIKELY(!thisObject)) {
throwThisTypeError(*globalObject, scope, "JSDiffieHellman"_s, "verifyError"_s);
return {};
}

auto& dh = thisObject->getImpl();
auto result = dh.check();
if (result == ncrypto::DHPointer::CheckResult::CHECK_FAILED) {
return Bun::ERR::CRYPTO_OPERATION_FAILED(scope, globalObject, "Checking DH parameters failed"_s);
}

return JSC::JSValue::encode(JSC::jsNumber(static_cast<int>(result)));
}

} // namespace Bun
16 changes: 16 additions & 0 deletions test/js/node/crypto/node-crypto.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -679,3 +679,19 @@ it("encoding should not throw in null, undefined or in valid encodings in create
expect(hmac.digest("hex")?.length).toBe(64);
}
});

it("verifyError should not be on the prototype of DiffieHellman and DiffieHellmanGroup", () => {
const dh = crypto.createDiffieHellman(512);
expect("verifyError" in crypto.DiffieHellman.prototype).toBeFalse();
expect("verifyError" in dh).toBeTrue();
expect(dh.verifyError).toBe(0);

const dhg = crypto.createDiffieHellmanGroup("modp5");
expect("verifyError" in crypto.DiffieHellmanGroup.prototype).toBeFalse();
expect("verifyError" in dhg).toBeTrue();

// boringssl seems to set DH_NOT_SUITABLE_GENERATOR for both
// DH_GENERATOR_2 and DH_GENERATOR_5 if not using
// DH_generate_parameters_ex
expect(dhg.verifyError).toBe(8);
});