Skip to content

Commit bc18da5

Browse files
author
Tim Taubert
committed
Bug 1437616 - Use proper WebAuthn result types defined in the .pidl r=jcj
Reviewers: jcj Reviewed By: jcj Bug #: 1437616 Differential Revision: https://phabricator.services.mozilla.com/D582
1 parent dce0960 commit bc18da5

13 files changed

+54
-89
lines changed

‎dom/u2f/U2F.cpp‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -462,7 +462,7 @@ U2F::Register(const nsAString& aAppId,
462462

463463
void
464464
U2F::FinishMakeCredential(const uint64_t& aTransactionId,
465-
nsTArray<uint8_t>& aRegBuffer)
465+
const WebAuthnMakeCredentialResult& aResult)
466466
{
467467
MOZ_ASSERT(NS_IsMainThread());
468468

@@ -483,7 +483,7 @@ U2F::FinishMakeCredential(const uint64_t& aTransactionId,
483483
}
484484

485485
CryptoBuffer regBuf;
486-
if (NS_WARN_IF(!regBuf.Assign(aRegBuffer))) {
486+
if (NS_WARN_IF(!regBuf.Assign(aResult.RegBuffer()))) {
487487
RejectTransaction(NS_ERROR_ABORT);
488488
return;
489489
}
@@ -606,8 +606,7 @@ U2F::Sign(const nsAString& aAppId,
606606

607607
void
608608
U2F::FinishGetAssertion(const uint64_t& aTransactionId,
609-
nsTArray<uint8_t>& aCredentialId,
610-
nsTArray<uint8_t>& aSigBuffer)
609+
const WebAuthnGetAssertionResult& aResult)
611610
{
612611
MOZ_ASSERT(NS_IsMainThread());
613612

@@ -628,13 +627,13 @@ U2F::FinishGetAssertion(const uint64_t& aTransactionId,
628627
}
629628

630629
CryptoBuffer credBuf;
631-
if (NS_WARN_IF(!credBuf.Assign(aCredentialId))) {
630+
if (NS_WARN_IF(!credBuf.Assign(aResult.CredentialID()))) {
632631
RejectTransaction(NS_ERROR_ABORT);
633632
return;
634633
}
635634

636635
CryptoBuffer sigBuf;
637-
if (NS_WARN_IF(!sigBuf.Assign(aSigBuffer))) {
636+
if (NS_WARN_IF(!sigBuf.Assign(aResult.SigBuffer()))) {
638637
RejectTransaction(NS_ERROR_ABORT);
639638
return;
640639
}

‎dom/u2f/U2F.h‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
#include "mozilla/dom/Nullable.h"
1414
#include "mozilla/dom/U2FBinding.h"
1515
#include "mozilla/dom/WebAuthnManagerBase.h"
16+
#include "mozilla/dom/PWebAuthnTransaction.h"
1617
#include "mozilla/ErrorResult.h"
1718
#include "mozilla/MozPromise.h"
1819
#include "nsProxyRelease.h"
@@ -122,12 +123,11 @@ class U2F final : public WebAuthnManagerBase
122123

123124
void
124125
FinishMakeCredential(const uint64_t& aTransactionId,
125-
nsTArray<uint8_t>& aRegBuffer) override;
126+
const WebAuthnMakeCredentialResult& aResult) override;
126127

127128
void
128129
FinishGetAssertion(const uint64_t& aTransactionId,
129-
nsTArray<uint8_t>& aCredentialId,
130-
nsTArray<uint8_t>& aSigBuffer) override;
130+
const WebAuthnGetAssertionResult& aResult) override;
131131

132132
void
133133
RequestAborted(const uint64_t& aTransactionId,

‎dom/webauthn/PWebAuthnTransaction.ipdl‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,10 @@ struct WebAuthnMakeCredentialInfo {
4343
WebAuthnAuthenticatorSelection AuthenticatorSelection;
4444
};
4545

46+
struct WebAuthnMakeCredentialResult {
47+
uint8_t[] RegBuffer;
48+
};
49+
4650
struct WebAuthnGetAssertionInfo {
4751
uint8_t[] RpIdHash;
4852
uint8_t[] ClientDataHash;
@@ -52,6 +56,11 @@ struct WebAuthnGetAssertionInfo {
5256
WebAuthnExtension[] Extensions;
5357
};
5458

59+
struct WebAuthnGetAssertionResult {
60+
uint8_t[] CredentialID;
61+
uint8_t[] SigBuffer;
62+
};
63+
5564
async protocol PWebAuthnTransaction {
5665
manager PBackground;
5766

@@ -63,8 +72,8 @@ async protocol PWebAuthnTransaction {
6372

6473
child:
6574
async __delete__();
66-
async ConfirmRegister(uint64_t aTransactionId, uint8_t[] RegBuffer);
67-
async ConfirmSign(uint64_t aTransactionId, uint8_t[] CredentialID, uint8_t[] ReplyBuffer);
75+
async ConfirmRegister(uint64_t aTransactionId, WebAuthnMakeCredentialResult aResult);
76+
async ConfirmSign(uint64_t aTransactionId, WebAuthnGetAssertionResult aResult);
6877
async Abort(uint64_t aTransactionId, nsresult Error);
6978
};
7079

‎dom/webauthn/U2FHIDTokenManager.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -214,7 +214,7 @@ U2FHIDTokenManager::HandleRegisterResult(UniquePtr<U2FResult>&& aResult)
214214
return;
215215
}
216216

217-
U2FRegisterResult result(Move(registration));
217+
WebAuthnMakeCredentialResult result(registration);
218218
mRegisterPromise.Resolve(Move(result), __func__);
219219
}
220220

@@ -241,7 +241,7 @@ U2FHIDTokenManager::HandleSignResult(UniquePtr<U2FResult>&& aResult)
241241
return;
242242
}
243243

244-
U2FSignResult result(Move(keyHandle), Move(signature));
244+
WebAuthnGetAssertionResult result(keyHandle, signature);
245245
mSignPromise.Resolve(Move(result), __func__);
246246
}
247247

‎dom/webauthn/U2FSoftTokenManager.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -689,7 +689,7 @@ U2FSoftTokenManager::Register(const nsTArray<WebAuthnScopedCredential>& aCredent
689689
registrationBuf.AppendSECItem(attestCert.get()->derCert);
690690
registrationBuf.AppendSECItem(signatureItem);
691691

692-
U2FRegisterResult result((nsTArray<uint8_t>(registrationBuf)));
692+
WebAuthnMakeCredentialResult result((nsTArray<uint8_t>(registrationBuf)));
693693
return U2FRegisterPromise::CreateAndResolve(Move(result), __func__);
694694
}
695695

@@ -832,7 +832,7 @@ U2FSoftTokenManager::Sign(const nsTArray<WebAuthnScopedCredential>& aCredentials
832832
signatureBuf.AppendSECItem(counterItem);
833833
signatureBuf.AppendSECItem(signatureItem);
834834

835-
U2FSignResult result(Move(keyHandle), nsTArray<uint8_t>(signatureBuf));
835+
WebAuthnGetAssertionResult result(keyHandle, nsTArray<uint8_t>(signatureBuf));
836836
return U2FSignPromise::CreateAndResolve(Move(result), __func__);
837837
}
838838

‎dom/webauthn/U2FTokenManager.cpp‎

Lines changed: 6 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -251,7 +251,7 @@ U2FTokenManager::Register(PWebAuthnTransactionParent* aTransactionParent,
251251
aTransactionInfo.ClientDataHash(),
252252
aTransactionInfo.TimeoutMS())
253253
->Then(GetCurrentThreadSerialEventTarget(), __func__,
254-
[tid, startTime](U2FRegisterResult&& aResult) {
254+
[tid, startTime](WebAuthnMakeCredentialResult&& aResult) {
255255
U2FTokenManager* mgr = U2FTokenManager::Get();
256256
mgr->MaybeConfirmRegister(tid, aResult);
257257
Telemetry::ScalarAdd(
@@ -274,15 +274,12 @@ U2FTokenManager::Register(PWebAuthnTransactionParent* aTransactionParent,
274274

275275
void
276276
U2FTokenManager::MaybeConfirmRegister(const uint64_t& aTransactionId,
277-
U2FRegisterResult& aResult)
277+
const WebAuthnMakeCredentialResult& aResult)
278278
{
279279
MOZ_ASSERT(mLastTransactionId == aTransactionId);
280280
mRegisterPromise.Complete();
281281

282-
nsTArray<uint8_t> registration;
283-
aResult.ConsumeRegistration(registration);
284-
285-
Unused << mTransactionParent->SendConfirmRegister(aTransactionId, registration);
282+
Unused << mTransactionParent->SendConfirmRegister(aTransactionId, aResult);
286283
ClearTransaction();
287284
}
288285

@@ -325,7 +322,7 @@ U2FTokenManager::Sign(PWebAuthnTransactionParent* aTransactionParent,
325322
aTransactionInfo.RequireUserVerification(),
326323
aTransactionInfo.TimeoutMS())
327324
->Then(GetCurrentThreadSerialEventTarget(), __func__,
328-
[tid, startTime](U2FSignResult&& aResult) {
325+
[tid, startTime](WebAuthnGetAssertionResult&& aResult) {
329326
U2FTokenManager* mgr = U2FTokenManager::Get();
330327
mgr->MaybeConfirmSign(tid, aResult);
331328
Telemetry::ScalarAdd(
@@ -348,17 +345,12 @@ U2FTokenManager::Sign(PWebAuthnTransactionParent* aTransactionParent,
348345

349346
void
350347
U2FTokenManager::MaybeConfirmSign(const uint64_t& aTransactionId,
351-
U2FSignResult& aResult)
348+
const WebAuthnGetAssertionResult& aResult)
352349
{
353350
MOZ_ASSERT(mLastTransactionId == aTransactionId);
354351
mSignPromise.Complete();
355352

356-
nsTArray<uint8_t> keyHandle;
357-
aResult.ConsumeKeyHandle(keyHandle);
358-
nsTArray<uint8_t> signature;
359-
aResult.ConsumeSignature(signature);
360-
361-
Unused << mTransactionParent->SendConfirmSign(aTransactionId, keyHandle, signature);
353+
Unused << mTransactionParent->SendConfirmSign(aTransactionId, aResult);
362354
ClearTransaction();
363355
}
364356

‎dom/webauthn/U2FTokenManager.h‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
#define mozilla_dom_U2FTokenManager_h
99

1010
#include "mozilla/dom/U2FTokenTransport.h"
11+
#include "mozilla/dom/PWebAuthnTransaction.h"
1112

1213
/*
1314
* Parent process manager for U2F and WebAuthn API transactions. Handles process
@@ -46,9 +47,11 @@ class U2FTokenManager final
4647
RefPtr<U2FTokenTransport> GetTokenManagerImpl();
4748
void AbortTransaction(const uint64_t& aTransactionId, const nsresult& aError);
4849
void ClearTransaction();
49-
void MaybeConfirmRegister(const uint64_t& aTransactionId, U2FRegisterResult& aResult);
50+
void MaybeConfirmRegister(const uint64_t& aTransactionId,
51+
const WebAuthnMakeCredentialResult& aResult);
5052
void MaybeAbortRegister(const uint64_t& aTransactionId, const nsresult& aError);
51-
void MaybeConfirmSign(const uint64_t& aTransactionId, U2FSignResult& aResult);
53+
void MaybeConfirmSign(const uint64_t& aTransactionId,
54+
const WebAuthnGetAssertionResult& aResult);
5255
void MaybeAbortSign(const uint64_t& aTransactionId, const nsresult& aError);
5356
// Using a raw pointer here, as the lifetime of the IPC object is managed by
5457
// the PBackground protocol code. This means we cannot be left holding an

‎dom/webauthn/U2FTokenTransport.h‎

Lines changed: 2 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -18,43 +18,8 @@
1818
namespace mozilla {
1919
namespace dom {
2020

21-
class U2FRegisterResult {
22-
public:
23-
explicit U2FRegisterResult(nsTArray<uint8_t>&& aRegistration)
24-
: mRegistration(aRegistration)
25-
{ }
26-
27-
void ConsumeRegistration(nsTArray<uint8_t>& aBuffer) {
28-
aBuffer = Move(mRegistration);
29-
}
30-
31-
private:
32-
nsTArray<uint8_t> mRegistration;
33-
};
34-
35-
class U2FSignResult {
36-
public:
37-
explicit U2FSignResult(nsTArray<uint8_t>&& aKeyHandle,
38-
nsTArray<uint8_t>&& aSignature)
39-
: mKeyHandle(aKeyHandle)
40-
, mSignature(aSignature)
41-
{ }
42-
43-
void ConsumeKeyHandle(nsTArray<uint8_t>& aBuffer) {
44-
aBuffer = Move(mKeyHandle);
45-
}
46-
47-
void ConsumeSignature(nsTArray<uint8_t>& aBuffer) {
48-
aBuffer = Move(mSignature);
49-
}
50-
51-
private:
52-
nsTArray<uint8_t> mKeyHandle;
53-
nsTArray<uint8_t> mSignature;
54-
};
55-
56-
typedef MozPromise<U2FRegisterResult, nsresult, true> U2FRegisterPromise;
57-
typedef MozPromise<U2FSignResult, nsresult, true> U2FSignPromise;
21+
typedef MozPromise<WebAuthnMakeCredentialResult, nsresult, true> U2FRegisterPromise;
22+
typedef MozPromise<WebAuthnGetAssertionResult, nsresult, true> U2FSignPromise;
5823

5924
class U2FTokenTransport
6025
{

‎dom/webauthn/WebAuthnManager.cpp‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -650,7 +650,7 @@ WebAuthnManager::Store(const Credential& aCredential)
650650

651651
void
652652
WebAuthnManager::FinishMakeCredential(const uint64_t& aTransactionId,
653-
nsTArray<uint8_t>& aRegBuffer)
653+
const WebAuthnMakeCredentialResult& aResult)
654654
{
655655
MOZ_ASSERT(NS_IsMainThread());
656656

@@ -660,7 +660,8 @@ WebAuthnManager::FinishMakeCredential(const uint64_t& aTransactionId,
660660
}
661661

662662
CryptoBuffer regData;
663-
if (NS_WARN_IF(!regData.Assign(aRegBuffer.Elements(), aRegBuffer.Length()))) {
663+
if (NS_WARN_IF(!regData.Assign(aResult.RegBuffer().Elements(),
664+
aResult.RegBuffer().Length()))) {
664665
RejectTransaction(NS_ERROR_OUT_OF_MEMORY);
665666
return;
666667
}
@@ -783,8 +784,7 @@ WebAuthnManager::FinishMakeCredential(const uint64_t& aTransactionId,
783784

784785
void
785786
WebAuthnManager::FinishGetAssertion(const uint64_t& aTransactionId,
786-
nsTArray<uint8_t>& aCredentialId,
787-
nsTArray<uint8_t>& aSigBuffer)
787+
const WebAuthnGetAssertionResult& aResult)
788788
{
789789
MOZ_ASSERT(NS_IsMainThread());
790790

@@ -794,8 +794,8 @@ WebAuthnManager::FinishGetAssertion(const uint64_t& aTransactionId,
794794
}
795795

796796
CryptoBuffer tokenSignatureData;
797-
if (NS_WARN_IF(!tokenSignatureData.Assign(aSigBuffer.Elements(),
798-
aSigBuffer.Length()))) {
797+
if (NS_WARN_IF(!tokenSignatureData.Assign(aResult.SigBuffer().Elements(),
798+
aResult.SigBuffer().Length()))) {
799799
RejectTransaction(NS_ERROR_OUT_OF_MEMORY);
800800
return;
801801
}
@@ -833,7 +833,7 @@ WebAuthnManager::FinishGetAssertion(const uint64_t& aTransactionId,
833833
}
834834

835835
CryptoBuffer credentialBuf;
836-
if (!credentialBuf.Assign(aCredentialId)) {
836+
if (!credentialBuf.Assign(aResult.CredentialID())) {
837837
RejectTransaction(NS_ERROR_OUT_OF_MEMORY);
838838
return;
839839
}

‎dom/webauthn/WebAuthnManager.h‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -123,12 +123,11 @@ class WebAuthnManager final : public WebAuthnManagerBase
123123

124124
void
125125
FinishMakeCredential(const uint64_t& aTransactionId,
126-
nsTArray<uint8_t>& aRegBuffer) override;
126+
const WebAuthnMakeCredentialResult& aResult) override;
127127

128128
void
129129
FinishGetAssertion(const uint64_t& aTransactionId,
130-
nsTArray<uint8_t>& aCredentialId,
131-
nsTArray<uint8_t>& aSigBuffer) override;
130+
const WebAuthnGetAssertionResult& aResult) override;
132131

133132
void
134133
RequestAborted(const uint64_t& aTransactionId,

0 commit comments

Comments
 (0)