Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(D)TLS certificate_verify signature never checked #70

Closed
areiter opened this issue Apr 14, 2014 · 2 comments
Closed

(D)TLS certificate_verify signature never checked #70

areiter opened this issue Apr 14, 2014 · 2 comments

Comments

@areiter
Copy link

areiter commented Apr 14, 2014

TLSServerProtocol and DTLSServerProtocol process the certificate_verify message. They verify the signature generated by the client and should, in my understanding, throw an error if the signature is not correct.

Currently the return value of the verifyRawSignature call is not evaluate at all:

// Verify the CertificateVerify message contains a correct signature.
 try
 {
    // TODO For TLS 1.2, this needs to be the hash specified in the DigitallySigned
    byte[] certificateVerifyHash = getCurrentPRFHash(getContext(), prepareFinishHash, null);

     org.bouncycastle.asn1.x509.Certificate x509Cert = this.peerCertificate.getCertificateAt(0);
     SubjectPublicKeyInfo keyInfo = x509Cert.getSubjectPublicKeyInfo();
     AsymmetricKeyParameter publicKey = PublicKeyFactory.createKey(keyInfo);

     TlsSigner tlsSigner = TlsUtils.createTlsSigner(this.clientCertificateType);
     tlsSigner.init(getContext());
     tlsSigner.verifyRawSignature(clientCertificateVerify.getAlgorithm(),
         clientCertificateVerify.getSignature(), publicKey, certificateVerifyHash);
 }
 catch (Exception e)
 {
     throw new TlsFatalAlert(AlertDescription.decrypt_error);
 }
@bcgit
Copy link
Collaborator

bcgit commented Apr 14, 2014

Thanks for this. You're correct in that the return value should be checked as well. There's some further work in this area to deal with TLS 1.2 and DTLS so we're going to try and get that done as well before issuing a new release.

@peterdettman
Copy link
Collaborator

This is now fixed, and test coverage has been added to vet various client-authentication scenarios (see TlsTestSuite.java). A new beta 151b12 is now available (http://downloads.bouncycastle.org/betas/) including this and other fixes.

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

No branches or pull requests

2 participants