Skip to content
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.

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

webauthn + refactor #23

Merged
merged 40 commits into from
May 25, 2022
Merged

webauthn + refactor #23

merged 40 commits into from
May 25, 2022

Conversation

tt0mmy
Copy link
Contributor

@tt0mmy tt0mmy commented May 10, 2022

@tt0mmy tt0mmy requested a review from hansl May 10, 2022 21:37
@tt0mmy tt0mmy changed the title Tt/webauthn webauthn May 10, 2022
@tt0mmy tt0mmy changed the title webauthn webauthn + refactor May 11, 2022
@tt0mmy tt0mmy marked this pull request as ready for review May 11, 2022 05:12
}

getCoseKey(): CoseKey {
const c = new Map()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

?

Copy link
Contributor Author

@tt0mmy tt0mmy May 12, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this.getCommon was removed from CoseKey, moved into getCoseKey for each Identity.
code snip below is from current main branch and for ANONYMOUS, we still create the common structure

// this.getProtectedHeader (ANONYMOUS) -> new CoseKey(ANONYMOUS) -> getCommon(ANONYMOUS)

    const protectedHeader = this.getProtectedHeader(
      keys ? keys.publicKey : ANONYMOUS
    );
    
    private static getProtectedHeader(publicKey: Key): CborMap {
      const coseKey = new CoseKey(publicKey);
      const protectedHeader = new Map();
      protectedHeader.set(1, -8); // alg: "Ed25519"
      protectedHeader.set(4, coseKey.keyId); // kid: kid
      protectedHeader.set("keyset", coseKey.toCborData());
      return protectedHeader;
    }


export class CoseKey {
  key: CborMap;
  keyId: CborData;
  private common: CborMap;

  constructor(publicKey: Key) {
   // publicKey would be ANONYMOUS here
    this.common = this.getCommon(publicKey);
    this.keyId = this.getKeyId();
    this.key = this.getKey();
  }

  private getCommon(publicKey: Key) {
    const common = new Map();
    common.set(1, 1); // kty: OKP
    common.set(3, -8); // alg: EdDSA
    common.set(-1, 6); // crv: Ed25519
    common.set(4, [2]); // key_ops: [verify]
    common.set(-2, publicKey); // x: publicKey
    return common;
  }

@tt0mmy tt0mmy requested a review from hansl May 13, 2022 16:54
@tt0mmy
Copy link
Contributor Author

tt0mmy commented May 13, 2022

Weird thing about the anonymous identity having a public key...

public key has been removed from anon identity.

@tt0mmy
Copy link
Contributor Author

tt0mmy commented May 21, 2022

@hansl , now ready for review

return Address.anonymous()
}

toJson() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
toJson() {
toJSON() {


export abstract class Identity implements Signer, Verifier {
abstract getAddress(): Promise<Address>
abstract toJson(): unknown
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
abstract toJson(): unknown
abstract toJSON(): unknown

publicKey: ArrayBuffer
protected privateKey: ArrayBuffer

constructor(publicKey: ArrayBuffer, privateKey: ArrayBuffer) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could be protected or private

return new WebAuthnIdentity(cosePublicKey, publicKeyCredential.rawId)
}

async sign(): Promise<ArrayBuffer> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
async sign(): Promise<ArrayBuffer> {
async sign(): Promise<ArrayBuffer> {
// This value ends up in the signature COSE field, but we don't
// use it for WebAuthn verification, so we simply set it to
// EMPTY.

}

async verify(_: ArrayBuffer): Promise<boolean> {
return false
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add comment here please.


pubKeyCredParams: [
{
// ES256 -7 ECDSA w/ SHA-256
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a comment explaining why we only use ES256.

@@ -0,0 +1,16 @@
import { Message } from ".."

export class Attributes {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a TODO and an issue on this repo for your thoughts on how this can be made better?

const sha512 = require("js-sha512")

const CHALLENGE_BUFFER = new TextEncoder().encode("lifted")
const ONE_MINUTE = 60000
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move this into a const.ts file or something similar.

Comment on lines 29 to 54
async function fetchAsyncStatus(
n: Network,
asyncToken: ArrayBuffer,
): Promise<Message> {
const start = new Date().getTime()
let waitTime = ONE_SECOND
let isDurationReached = false
let res = new Message(new Map())

while (!isDurationReached) {
res = (await n.call("async.status", new Map([[0, asyncToken]]))) as Message
const payload = res.getPayload()
if (payload) {
if (payload.has(0)) {
const asyncResult = payload.get(0)
if (asyncResult === 3 && payload.has(1)) {
const msg = cbor.decode(payload.get(1)).value
return new Message(msg)
}
}
}
const now = new Date().getTime()
isDurationReached = now - start >= ONE_MINUTE
await sleep(waitTime)
waitTime *= 1.5
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
async function fetchAsyncStatus(
n: Network,
asyncToken: ArrayBuffer,
): Promise<Message> {
const start = new Date().getTime()
let waitTime = ONE_SECOND
let isDurationReached = false
let res = new Message(new Map())
while (!isDurationReached) {
res = (await n.call("async.status", new Map([[0, asyncToken]]))) as Message
const payload = res.getPayload()
if (payload) {
if (payload.has(0)) {
const asyncResult = payload.get(0)
if (asyncResult === 3 && payload.has(1)) {
const msg = cbor.decode(payload.get(1)).value
return new Message(msg)
}
}
}
const now = new Date().getTime()
isDurationReached = now - start >= ONE_MINUTE
await sleep(waitTime)
waitTime *= 1.5
}
enum AsyncStatusResult {
Unknown = 0,
Queued = 1,
Processing = 2,
Done = 3,
Expired = 4,
}
type AsyncStatusPayload = { result: AsyncStatusResult.Unknown }
| { result: AsyncStatusResult.Queued }
| { result: AsyncStatusResult.Processing }
| { result: AsyncStatusResult.Expired }
| {
result: AsyncStatusResult.Done,
payload: ArrayBuffer,
}
function parseAsyncStatusPayload(cbor: Map<number, unknown>): AsyncStatusPayload {
const index = cbor.get(0);
if (typeof index != "number") {
throw new Error("Invalid async result")
}
if (!(index in AsyncStatusResult)) {
throw Error("Invalid async result")
}
const result = index as AsyncStatusResult
let payload = undefined;
if (result === AsyncStatusResult.Done) {
payload = cbor.get(1) as ArrayBuffer;
if (!payload || !Buffer.isBuffer(payload)) {
throw Error("Invalid async result")
}
}
return { result, payload } as AsyncStatusPayload
}
async function pollAsyncStatus(
n: Network,
asyncToken: ArrayBuffer,
options: {
timeoutInMsec: number,
waitTimeInMsec: number,
} = {}
): Promise<Message> {
const timeoutInMsec = options.timeoutInMsec ?? ONE_MINUTE;
const waitTimeInMsec = options.waitTimeInMsec ?? ONE_SECOND;
const end = new Date() + timeoutInMsec
while (true) {
const res = (await n.call("async.status", new Map([[0, asyncToken]]))) as Message
const result = parseAsyncStatusPayload(res.getPayload())
switch (result.result) {
case AsyncStatusResult.Done:
return new Message(cbor.decode(result.payload).value)
case AsyncStatusResult.Expired:
throw new Error("Async Expired before getting a result")
}
if (Date.now() >= end) {
return res;
}
await sleep(waitTime)
waitTime *= 1.5
}

@hansl
Copy link
Contributor

hansl commented May 25, 2022

BTW you should request a follow up review using the little icon on the right:
image

@tt0mmy tt0mmy requested a review from hansl May 25, 2022 16:17
@tt0mmy tt0mmy merged commit f38012e into liftedinit:main May 25, 2022
@tt0mmy tt0mmy deleted the tt/webauthn branch May 25, 2022 16:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants