refactor(auth): prune p256 dependency down to der crate

[alvarowolfx]

Towards #5584

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.77%. Comparing base (ac4115f) to head (6e3d2e6).

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #5617   +/-   ##
=======================================
  Coverage   97.77%   97.77%           
=======================================
  Files         221      221           
  Lines       51107    51107           
=======================================
+ Hits        49969    49970    +1     
+ Misses       1138     1137    -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[gemini-code-assist]

Code Review

This pull request replaces the p256 dependency with the der crate for GDCH service account credentials, introducing manual conversion of ASN.1 DER-encoded ECDSA signatures to the 64-byte JOSE format. Feedback focuses on ensuring strict DER adherence by checking for trailing data during sequence decoding and improving the robustness of the integer padding logic by using dynamic lengths and explicit zero-initialization.

[gemini-code-assist]

The closure passed to reader.sequence decodes two integers but does not verify that the sequence is then exhausted. To ensure strict adherence to the ASN.1 DER format for ECDSA signatures (SEQUENCE { r INTEGER, s INTEGER }), you should check that the nested reader is finished.

    let (r_ref, s_ref) = reader
        .sequence(|nested| {
            let r = nested.decode::<UintRef<'_>>()?;
            let s = nested.decode::<UintRef<'_>>()?;
            if !nested.is_finished() {
                return Err(nested.error(der::ErrorKind::TrailingData));
            }
            Ok((r, s))
        })

References

1. Demand Explosive Correctness: Never swallow errors or ignore Result types. Fail loudly and explicitly when appropriate. ^(link)

[gemini-code-assist]

The function copy_jose_integer uses hardcoded values 32 and 33. It is more robust and idiomatic to use dest.len() and dest.len() + 1 instead. Additionally, the function relies on the caller to zero-initialize the destination buffer for correct padding. It is safer to explicitly zero the padding area within the function.

fn copy_jose_integer(dest: &mut [u8], bytes: &[u8]) -> Result<()> {
    let dest_len = dest.len();
    if bytes.len() > dest_len + 1 {
        return Err(errors::non_retryable_from_str(format!(
            "invalid GDCH ECDSA signature integer length: {} bytes",
            bytes.len()
        )));
    } else if bytes.len() == dest_len + 1 {
        if bytes[0] != 0 {
            return Err(errors::non_retryable_from_str(
                format!("invalid {}-byte GDCH ECDSA signature integer: leading byte is not zero", bytes.len()),
            ));
        }
        dest.copy_from_slice(&bytes[1..]);
    } else {
        let offset = dest_len - bytes.len();
        dest[..offset].fill(0);
        dest[offset..].copy_from_slice(bytes);
    }
    Ok(())
}

References

1. Reject 'Code Poetry': Dismantle complex abstractions used for simple tasks. Prefer simplicity over cleverness. ^(link)