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

The signature problem on Integrating with Lit #173

Closed
0xja-eth opened this issue Nov 15, 2023 · 9 comments · Fixed by #176
Closed

The signature problem on Integrating with Lit #173

0xja-eth opened this issue Nov 15, 2023 · 9 comments · Fixed by #176

Comments

@0xja-eth
Copy link

0xja-eth commented Nov 15, 2023

Description

I'm trying to approve for Lit Protocol and Ceramic with just one signature. I've managed to do so, but there might be some issues with my solution. I'll describe my problems here and discuss if there's a better approach, or whether it's possible to optimize the code of Lit or Ceramic to better address these issues.

Technical Information

My approach is as follows: first, I call Ceramic's authorization, with the code as below:

const accountId = await getAccountId(ethProvider, address)
      
const authMethod = await getAuthMethod(ethProvider, accountId)
console.log("[AuthCeramic] Authorize", authMethod)

session = await DIDSession.authorize(authMethod, {resources: ["ceramic://*"]})

After the user signs, they get a session as a login credential.
In Lit Protocol, similar to session, there's a data structure called AuthSig as a login credential. The structure of AuthSig is as follows:

interface AuthSig {
    sig: any; // Data after signing
    derivedVia: string; // Generally "web3.eth.personal.sign"
    signedMessage: string; // Data before signing
    address: string; // Signing address
}

Actually, session and AuthSig are somewhat interchangeable to a certain extent. Below is my conversion code:

async function getAuthSig(session, chainId = TargetChainId) {
  const {p, s} = session.cacao;
  const address = p.iss.split(":").pop();

  const message = new SiweMessage({
    domain: p.domain, address, 
    statement: p.statement,
    uri: p.aud,
    version: p.version,
    nonce: p.nonce,
    issuedAt: p.iat,
    expirationTime: p.exp,
    chainId: chainId.toString(),
    resources: p.resources,
  })

  return {
    address, derivedVia: "web3.eth.personal.sign",
    sig: s.s, signedMessage: message.toMessage(),
  } as AuthSig
}

Therefore, we can convert the session obtained after authorizing Ceramic into AuthSig and set it in LocalStorage, to authorize Lit:

const LitAuthSigKey = "lit-auth-signature"
const LitProviderKey = "lit-web3-provider"
const LitDefaultProvider = "metamask"

if (authSig) { // Set the default authSig in advance to avoid reconnecting and re-signing
  setLocalStorage(LitProviderKey, LitDefaultProvider)
  setLocalStorage(LitAuthSigKey, authSig)
}
authSig = await LitJsSdk.checkAndSignAuthMessage({
  chain: Chain
});

// Do some encryption/decryptions ....
const res = await LitJsSdk.decryptToString({
  authSig, chain: Chain, ...
}, lit)

This is my approach.

Solution Problem

There's a problem with the above solution, which is the issue of case sensitivity in the address in the signature information.

In the signature information of Ceramic, the address is converted to lowercase, here's an example:

localhost wants you to sign in with your Ethereum account:
0x66d369e52b1a7ee16.......f84025

Give this application access to some of your data on Ceramic

URI: did:key:z6Mkofcp......pnsDJ26j
Version: 1
Chain ID: 1
Nonce: 1CjSZZjMv2
Issued At: 2023-11-15T08:13:19.690Z
Expiration Time: 2023-11-22T08:13:19.690Z
Resources:
- ceramic://*

However, when we convert this signature into AuthSig and authenticate it in Lit, it says that the address does not conform to the EIP-55 format:
a1fbbbbb99c57d8ab7f2f9a94c12187
And this is the request data:
image

Therefore, I have to insert the following statement in the first section of the code to ensure that the address in the signature information is in EIP-55 format:

const accountId = await getAccountId(ethProvider, address)
accountId.address = address // Prevent the lowercase address

Only then can I successfully authorize Lit.
But after converting to EIP-55, the generated PKH-DID looks like this: did:pkh:eip155:1:0x66d369E52b1A7ee16D65e6a052745561C1f84025, which contains case sensitivity, and that's the problem:

  • If I keep the address lowercase, the generated DID is normal and conforms to the rules, but Lit cannot be successfully authorized.
  • If I retain the case sensitivity, I can successfully authorize Lit, but the generated DID will also retain the case sensitivity, but it seems to be able to be used normally in Ceramic.

Therefore, I wonder if there is a better solution, or whether it's possible to make some modifications to Ceramic or Lit to solve this problem?

@stbrody
Copy link
Contributor

stbrody commented Nov 15, 2023

@oed @ukstv any ideas here?

@0xja-eth
Copy link
Author

I have also opened the same issue in Lit: LIT-Protocol/js-sdk#258

@oed
Copy link
Member

oed commented Nov 16, 2023

I'm actually surprised that your proposed solution works. It seems like in the getAuthMethod code, the accountId that get's passed in eventually gets normalized.

Regardless, it seems like we could be able to solve the problem of the mixed case address, by adding a .toLowerCase() later in the code.

@0xja-eth
Copy link
Author

0xja-eth commented Nov 16, 2023

I'm actually surprised that your proposed solution works. It seems like in the getAuthMethod code, the accountId that get's passed in eventually gets normalized.

Regardless, it seems like we could be able to solve the problem of the mixed case address, by adding a .toLowerCase() later in the code.

Wait, I forgot something. In fact, in my code, I overrode the createCACAO function to ensure the mixed-case address. Here is my code:

// region Override

export function makeSiweMessage(opts, address, chainId = TargetChainId) {
  const now = new Date();
  const oneWeekLater = new Date(now.getTime() + 7 * 24 * 60 * 60 * 1000);
  return new SiweMessage({
    domain: opts.domain,
    address: address,
    statement: opts.statement ?? 'Give this application access to some of your data on Ceramic',
    uri: opts.uri,
    version: VERSION,
    nonce: opts.nonce ?? randomString(10),
    issuedAt: now.toISOString(),
    expirationTime: opts.expirationTime ?? oneWeekLater.toISOString(),
    chainId: chainId.toString(),
    resources: opts.resources
  });
}

async function createCACAO(opts, ethProvider, account) {
  const siweMessage = makeSiweMessage(opts, account.address, account.chainId.reference)
  siweMessage.signature = await safeSend(
    ethProvider, 'personal_sign', [
      encodeHexStr(siweMessage.signMessage()),
      account.address
    ]);
  console.log('[CACAO] siweMessage', siweMessage, siweMessage.signMessage())
  return fromSiweMessage(siweMessage);
}
function encodeHexStr(str) {
  return `0x${bytesToHex(utf8ToBytes(str))}`;
}
async function getAuthMethod(ethProvider, account) {
  if (typeof window === 'undefined') throw new Error('Web Auth method requires browser environment');
  const domain = window.location.hostname;
  return async (opts)=>{
    opts.domain = domain;
    console.log('[CACAO] opts', opts, ethProvider, account)
    return createCACAO(opts, ethProvider, account);
  };
}

// endregion

I call getAuthMethod defined by myself.
If I don't do that, the address will be lowercased and cannot verify by Lit Protocol.

@oed
Copy link
Member

oed commented Nov 16, 2023

Got it. Will work on a PR to fix this 👍

@oed
Copy link
Member

oed commented Nov 16, 2023

After looking close at this, it seems like the best solution to the problem is probably a change to LIT protocol not to be case sensitive about the address.

@0xja-eth
Copy link
Author

After looking close at this, it seems like the best solution to the problem is probably a change to LIT protocol not to be case sensitive about the address.

yeh, I think so, so I also opened the same issue in Lit, I'll talk to Lit team. Thank you!

@glitch003
Copy link

Update here: @oed and I are trying to figure out the best path forward. EIP-55 is recommended in the SIWE standard (and Lit just uses the provided SIWE packages that enforce this standard), so this isn't a Lit specific requirement. It's a hard problem because of all the existing data that was stored with non EIP-55 SIWEs, so Ceramic can't just switch to EIP-55. On the other hand, Lit wants to conform to the standard if possible, so that our SIWEs are compatible with the libs that enforce the standard.

I don't think we have an immediate solution but are thinking about options.

@skylar745
Copy link

Interested into solving this too.
FYI, I did run into the same issue a couple of months back when Lit changed their auth process to require checksum addresses. I found a message on their discord describing the change from lowercase to checksum. not sure about the reason but probably necessary to them somehow.

@oed oed mentioned this issue Dec 13, 2023
@oed oed closed this as completed in #176 Dec 19, 2023
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 a pull request may close this issue.

5 participants