Skip to content

fix(sign): match cloud signature to requested digest - #3

Merged
Le-Syl21 merged 3 commits into
Le-Syl21:mainfrom
Loggableim:agent/fix-signature-digest-match
Jul 22, 2026
Merged

fix(sign): match cloud signature to requested digest#3
Le-Syl21 merged 3 commits into
Le-Syl21:mainfrom
Loggableim:agent/fix-signature-digest-match

Conversation

@Loggableim

Copy link
Copy Markdown
Contributor

What changed

  • Select the cloud signature by the exact digest requested from Certum.
  • Reject a response that lacks that digest instead of taking the first map entry.
  • Add regression tests for multi-entry responses and missing requested digests.

Scope

This PR intentionally contains only the response-selection fix. Timestamp transport policy, Content-Type handling, PE bounds checks, and output-write hardening are excluded.

@Le-Syl21

Copy link
Copy Markdown
Owner

Thanks — the selection bug is real: values().next() can pick a signature that doesn't match the digest we asked for, so keying on the requested digest is the right call.

One thing before this goes in. Map::get is an exact, case-sensitive match, and we build the key with hex::encode, which is lowercase. Even if Certum happens to echo lowercase today, that isn't something we can rely on: what the protocol gives us — in the docs and by the nature of the value — is hex. The case is not part of that contract, so it can change without Certum breaking anything on their side, and observing it once wouldn't make it a guarantee. The failure mode is every signing run turning into no signature for requested digest.

Since the case carries no information for a hex string, I'd rather not depend on it:

.and_then(|m| {
    m.iter()
        .find(|(k, _)| k.eq_ignore_ascii_case(digest_hex))
        .map(|(_, v)| v)
})

Same guarantee, no allocation, and nothing to break on a formatting detail that was never promised. Worth adding a test with a realistic 64-char digest in both cases too — the current ones use "wanted"/"other", which don't exercise that path.

@Le-Syl21 Le-Syl21 mentioned this pull request Jul 22, 2026
@Le-Syl21
Le-Syl21 marked this pull request as ready for review July 22, 2026 12:41
@Le-Syl21
Le-Syl21 merged commit a2b7ddd into Le-Syl21:main Jul 22, 2026
9 checks passed
Le-Syl21 added a commit that referenced this pull request Jul 22, 2026
CI : ajout de --workspace à clippy et cargo test. Sans lui, le CI ne
lançait qu'UN test (celui du binaire) et n'a jamais exécuté les 16 tests
de ssign-core — y compris ceux ajoutés par #3 et #4. Clippy ne lintait
pas non plus ssign-core. --all-targets sur clippy pour couvrir les tests.

Secrets : la graine TOTP et le code sont désormais portés par
Zeroizing<String> et effacés à leur libération ; les copies laissées par
clap sont effacées dès que le secret est repris. Limite documentée dans
le code : cela ne couvre pas ce qui précède main (argv et environ restent
lisibles dans /proc), d'où le conseil de préférer les variables
d'environnement.

Contenu de la release, via #3 et #4 :
- signature cloud sélectionnée sur le digest exact demandé, comparaison
  insensible à la casse (le contrat de l'API porte sur du hex, pas sur
  une casse) ;
- table de certificats PE bornée, sans rejeter un offset résiduel quand
  la table est vide ;
- validation structurelle CMS du token d'horodatage avant intégration ;
- écriture atomique des sorties : fichier temporaire O_EXCL dans le même
  répertoire, sync, permissions préservées, remplacement atomique, et
  refus d'écraser un .orig existant.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0138mtPxwfXPetBypyjp6KwU
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.

2 participants