Respond with TLS alert "bad_certificate" if certificate parsing fails

[mj-vivavis]

Improve the error handling in case a invalid (syntactically incorrect) certificate is received in a TLS handshake from the server.

Previously, the exception thrown during ASN.1 parsing was not caught and therefore caused a TLS alert "internal_error". This PR handles the exception to respond with a RFC-compliant TLS alert "bad_certificate".

[peterdettman]

The basic idea is good, but the patch is a bit simplistic:

• we should deal also with processing of the client certificate
• shouldn't swallow existing TlsFatalAlert (which is an IOException subclass)
• probably not all errors from Certificate.parse should be bad_certificate; I guess decode_error should be used for some cases and there may be others.

[mj-vivavis]

Thanks for your feedback.

we should deal also with processing of the client certificate

Agreed, I refactored my patch to have a common method which is used both for handling of received client and server certificates

shouldn't swallow existing TlsFatalAlert (which is an IOException subclass)

Fixed.

probably not all errors from Certificate.parse should be bad_certificate; I guess decode_error should be used for some cases and there may be others.

JcaTlsCrypto.createCertificate() detects if the certificate is not of type X.509 and responds with "unsupported_certificate". https://github.com/bcgit/bc-java/blob/0520ebcd3291d6d3176ea25c571f1c7e0a963847/tls/src/main/java/org/bouncycastle/tls/crypto/impl/jcajce/JcaTlsCrypto.java#L162-L171 If none or more than one ASN.1 object has been provided, TlsUtils.readASN1Object() throws a "decode_error" (the method is originally called by JcaTlsCertificate.parseCertificate()) https://github.com/bcgit/bc-java/blob/0520ebcd3291d6d3176ea25c571f1c7e0a963847/tls/src/main/java/org/bouncycastle/tls/TlsUtils.java#L1080-L1093

For everything else, "bad_certificate" is IMHO the right response at this place. Rationale:

• We extracted the portion of the handshake message, which is supposed to contain the certificate. No range check failed until now, and no other inconsistency or invalid value in the handshake message or its overall structure has been detected, which would have justified a "decode_error"
• We failed to process the certificate, which is what "bad_certificate" is supposed to signal
  • Any general IOException due to out-of-bounds access of the ByteArrayStream etc. indicates that the certificates internal structure is broken/invalid (or as a special case, the sender only sent a portion of it)
  • Any ASN1Exception indicates, as far as I can tell, explicitly a broken certificate