add_existing_user_info has no effect

[jaroban]

If I understand things correctly, add_existing_user_info never adds any user info because the Get Assertion request only contains credential IDs, and no user info. The function is_matching_rk will match the rpIds but id_size will be non-zero for the resident key and zero for the parsed GA request credential.

[LockRing]

As far as I know, GA->creds points getAssertionState.creds which is an array. https://github.com/solokeys/solo/blob/8b91ec7c538d0d071842e0b86ef94266936ab1d7/fido2/ctap.h#L367 https://github.com/solokeys/solo/blob/8b91ec7c538d0d071842e0b86ef94266936ab1d7/fido2/ctap.h#L188-L192 In the function add_existing_user_info, it casts &cred->credential to CTAP_residentKey . https://github.com/solokeys/solo/blob/8b91ec7c538d0d071842e0b86ef94266936ab1d7/fido2/ctap.c#L1098 CTAP_residentKey also has CredentialId and CTAP_userEntity like struct Credential but there are more members rpId and rpIdSize. https://github.com/solokeys/solo/blob/8b91ec7c538d0d071842e0b86ef94266936ab1d7/fido2/ctap.h#L175-L186 Isn't it invalid casting struct Credential to CTAP_residentKey * and access to rpId or rpIdSize? As GA->creds is an array of struct Credential and CTAP_residentKey is bigger than struct Credential, it results access next element of the array. Is there anything what I missed?

And I couldn't find any code to set rpId and rpIdSize before add_existing_user_info. I think add_existing_user_info cannot add user info as you said. There's a code that copies rk to credential, but it's not related to add_existing_user_info and I wonder if this is valid as I said above. https://github.com/solokeys/solo/blob/8b91ec7c538d0d071842e0b86ef94266936ab1d7/fido2/ctap.c#L1190

[jaroban]

You're right... I think you should open a separate issue regarding that memmove / overflow / type cleanup. The cast you mention relies on Credential and CTAP_residentKey starting with the same members: id, user. This would be better done using composition like so:

typedef struct {
    CredentialId id;
    CTAP_userEntity user;
} __attribute__((packed)) Credential;

typedef struct {
    Credential credential;

    // Maximum amount of "extra" space in resident key.
    uint8_t rpId[48];
    uint8_t rpIdSize;
} __attribute__((packed)) CTAP_residentKey;

And then the types in the functions could be changed to the correct type...

I changed the title since the point of the function is to get user info from a resident key... so I think what should be done is add_existing_user_info should use a different matching function, something like:

static int is_matching_rk2(CTAP_residentKey *rk_cred, struct Credential *allow_list_cred)
{
    return memcmp(rk_cred->id.rpIdHash, allow_list_cred->id.rpIdHash, 32) == 0 &&
           memcmp(&rk_cred->id.entropy, &allow_list_cred->id.entropy, CREDENTIAL_NONCE_SIZE) == 0 &&
           memcmp(&rk_cred->id.tag, &allow_list_cred->id.tag, CREDENTIAL_TAG_SIZE) == 0;
}