haskell-tls / hs-certificate

Certificate and Key Reader/Writer in haskell
60 stars 57 forks source link

validateDefault/validate implicit assume certificate chain order #85

Open wangbj opened 7 years ago

wangbj commented 7 years ago

Say if A -> B -> C (-> implies issue) It assumes the chain is in reverse order: C <> B <> A. This could be dangerous because the API doesn't mention it has to be in this order, so user could keep the order as A <> B <> C:

1) make a cache by adding A (root) only; 2) call validateDefault with above cache and chain (A <> B <> C)

Both A/B will pass, however, C will not be checked at all because of below code:

       doCheckChain level current chain = do
            r <- doCheckCertificate (getCertificate current)
            -- check if we have a trusted certificate in the store belonging to this issuer.
            return r |> (case findCertificate (certIssuerDN . getCertificate $ current) store of
                Just trustedSignedCert      -> return (checkSignature current trustedSignedCert)         -- ^ will not check the rest of the certificates!
                Nothing | isSelfSigned cert -> return [SelfSigned] |> return (checkSignature current current)
                        | null chain        -> return [UnknownCA]
                        | otherwise         -> 

And the overall result is chain valid, even when C is invalid (or not issued by B at all).

Ideally, this API should always check all the certificate in the chain, without assuming any order.

ocheron commented 7 years ago

Order is end-entity certificate first, just like what is mandated in TLS spec. Yes, this should be explicitely documented.

In your scenario you validate the root A itself. (B, C) are extra certificates, not needed.

Unfortunately I don't see how we could identify better the end-entity certificate, especially in non-strict mode. Let's imagine you also forget to put B in the certificate chain. From (A <> C) the code is unable to find that C is the certificate validation is requested for.

In practice key usage and hostname checks will make the validation fail anyway, right?