Add tests for doing client auth with no certificates.
In TLS, you never skip the Certificate message. It may be empty, but its
presence is determined by CertificateRequest. (This is sensible.)
In SSL 3.0, the client omits the Certificate message. This means you need to
probe and may receive either Certificate or ClientKeyExchange (thankfully,
ClientKeyExchange is not optional, or we'd have to probe at ChangeCipherSpec).
We didn't have test coverage for this, despite some of this logic being a
little subtle asynchronously. Fix this.
Change-Id: I149490ae5506f02fa0136cb41f8fea381637bf45
Reviewed-on: https://boringssl-review.googlesource.com/7419
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
diff --git a/ssl/test/runner/alert.go b/ssl/test/runner/alert.go
index 541216e..7db6826 100644
--- a/ssl/test/runner/alert.go
+++ b/ssl/test/runner/alert.go
@@ -22,6 +22,7 @@
alertRecordOverflow alert = 22
alertDecompressionFailure alert = 30
alertHandshakeFailure alert = 40
+ alertNoCertficate alert = 41
alertBadCertificate alert = 42
alertUnsupportedCertificate alert = 43
alertCertificateRevoked alert = 44
diff --git a/ssl/test/runner/common.go b/ssl/test/runner/common.go
index 42f4a00..74c552e 100644
--- a/ssl/test/runner/common.go
+++ b/ssl/test/runner/common.go
@@ -473,6 +473,10 @@
// NewSessionTicket message despite promising to in ServerHello.
SkipNewSessionTicket bool
+ // SkipClientCertificate causes the client to skip the Certificate
+ // message.
+ SkipClientCertificate bool
+
// SkipChangeCipherSpec causes the implementation to skip
// sending the ChangeCipherSpec message (and adjusting cipher
// state accordingly for the Finished message).
diff --git a/ssl/test/runner/conn.go b/ssl/test/runner/conn.go
index 5a19c28..fe0b139 100644
--- a/ssl/test/runner/conn.go
+++ b/ssl/test/runner/conn.go
@@ -856,7 +856,7 @@
// L < c.out.Mutex.
func (c *Conn) sendAlert(err alert) error {
level := byte(alertLevelError)
- if err == alertNoRenegotiation || err == alertCloseNotify {
+ if err == alertNoRenegotiation || err == alertCloseNotify || err == alertNoCertficate {
level = alertLevelWarning
}
return c.SendAlert(level, err)
diff --git a/ssl/test/runner/handshake_client.go b/ssl/test/runner/handshake_client.go
index 1f52dce..67609fc 100644
--- a/ssl/test/runner/handshake_client.go
+++ b/ssl/test/runner/handshake_client.go
@@ -564,15 +564,20 @@
hs.writeServerHash(shd.marshal())
// If the server requested a certificate then we have to send a
- // Certificate message, even if it's empty because we don't have a
- // certificate to send.
+ // Certificate message in TLS, even if it's empty because we don't have
+ // a certificate to send. In SSL 3.0, skip the message and send a
+ // no_certificate warning alert.
if certRequested {
- certMsg := new(certificateMsg)
- if chainToSend != nil {
- certMsg.certificates = chainToSend.Certificate
+ if c.vers == VersionSSL30 && chainToSend == nil {
+ c.sendAlert(alertNoCertficate)
+ } else if !c.config.Bugs.SkipClientCertificate {
+ certMsg := new(certificateMsg)
+ if chainToSend != nil {
+ certMsg.certificates = chainToSend.Certificate
+ }
+ hs.writeClientHash(certMsg.marshal())
+ c.writeRecord(recordTypeHandshake, certMsg.marshal())
}
- hs.writeClientHash(certMsg.marshal())
- c.writeRecord(recordTypeHandshake, certMsg.marshal())
}
preMasterSecret, ckx, err := keyAgreement.generateClientKeyExchange(c.config, hs.hello, leaf)
diff --git a/ssl/test/runner/handshake_server.go b/ssl/test/runner/handshake_server.go
index 0232772..d2cac98 100644
--- a/ssl/test/runner/handshake_server.go
+++ b/ssl/test/runner/handshake_server.go
@@ -626,13 +626,22 @@
// certificate message, even if it's empty.
if config.ClientAuth >= RequestClientCert {
var certMsg *certificateMsg
- if certMsg, ok = msg.(*certificateMsg); !ok {
+ var certificates [][]byte
+ if certMsg, ok = msg.(*certificateMsg); ok {
+ if c.vers == VersionSSL30 && len(certMsg.certificates) == 0 {
+ return errors.New("tls: empty certificate message in SSL 3.0")
+ }
+
+ hs.writeClientHash(certMsg.marshal())
+ certificates = certMsg.certificates
+ } else if c.vers != VersionSSL30 {
+ // In TLS, the Certificate message is required. In SSL
+ // 3.0, the peer skips it when sending no certificates.
c.sendAlert(alertUnexpectedMessage)
return unexpectedMessageError(certMsg, msg)
}
- hs.writeClientHash(certMsg.marshal())
- if len(certMsg.certificates) == 0 {
+ if len(certificates) == 0 {
// The client didn't actually send a certificate
switch config.ClientAuth {
case RequireAnyClientCert, RequireAndVerifyClientCert:
@@ -641,14 +650,16 @@
}
}
- pub, err = hs.processCertsFromClient(certMsg.certificates)
+ pub, err = hs.processCertsFromClient(certificates)
if err != nil {
return err
}
- msg, err = c.readHandshake()
- if err != nil {
- return err
+ if ok {
+ msg, err = c.readHandshake()
+ if err != nil {
+ return err
+ }
}
}
diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go
index 4c46639..52729e3 100644
--- a/ssl/test/runner/runner.go
+++ b/ssl/test/runner/runner.go
@@ -2656,6 +2656,28 @@
})
}
}
+
+ testCases = append(testCases, testCase{
+ testType: serverTest,
+ name: "RequireAnyClientCertificate",
+ flags: []string{"-require-any-client-certificate"},
+ shouldFail: true,
+ expectedError: ":PEER_DID_NOT_RETURN_A_CERTIFICATE:",
+ })
+
+ testCases = append(testCases, testCase{
+ testType: serverTest,
+ name: "SkipClientCertificate",
+ config: Config{
+ Bugs: ProtocolBugs{
+ SkipClientCertificate: true,
+ },
+ },
+ // Setting SSL_VERIFY_PEER allows anonymous clients.
+ flags: []string{"-verify-peer"},
+ shouldFail: true,
+ expectedError: ":TLS_PEER_DID_NOT_RESPOND_WITH_CERTIFICATE_LIST:",
+ })
}
func addExtendedMasterSecretTests() {
@@ -2859,12 +2881,37 @@
// TLS client auth.
tests = append(tests, testCase{
testType: clientTest,
- name: "ClientAuth-NoCertificate",
+ name: "ClientAuth-NoCertificate-Client",
config: Config{
ClientAuth: RequestClientCert,
},
})
tests = append(tests, testCase{
+ testType: serverTest,
+ name: "ClientAuth-NoCertificate-Server",
+ // Setting SSL_VERIFY_PEER allows anonymous clients.
+ flags: []string{"-verify-peer"},
+ })
+ if protocol == tls {
+ tests = append(tests, testCase{
+ testType: clientTest,
+ name: "ClientAuth-NoCertificate-Client-SSL3",
+ config: Config{
+ MaxVersion: VersionSSL30,
+ ClientAuth: RequestClientCert,
+ },
+ })
+ tests = append(tests, testCase{
+ testType: serverTest,
+ name: "ClientAuth-NoCertificate-Server-SSL3",
+ config: Config{
+ MaxVersion: VersionSSL30,
+ },
+ // Setting SSL_VERIFY_PEER allows anonymous clients.
+ flags: []string{"-verify-peer"},
+ })
+ }
+ tests = append(tests, testCase{
testType: clientTest,
name: "ClientAuth-NoCertificate-OldCallback",
config: Config{
