Enhance tstclnt and ssltap to support OCSP stapling

RESOLVED FIXED in 3.15

Status

NSS
Tools
P2
enhancement
RESOLVED FIXED
7 years ago
5 years ago

People

(Reporter: kaie, Assigned: kaie)

Tracking

Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(4 obsolete attachments)

(Assignee)

Description

7 years ago
Created attachment 572857 [details] [diff] [review]
Patch v1

We must enhance tstclnt and ssltap to support OCSP stapling.

Today, when using ssltap to inspect an SSL connection with OCSP stapling, ssltap will report an error. We should fix that, and in addition, ssltap should report some details about the OCSP information (we can copy existing code from ocspclnt.c).

In addition, we need a client tool to request information from the server (no information will be sent by the server unless the client requests it).

I've modified tstclnt.c and added -C (certificate status request).

Updated

6 years ago
Assignee: nobody → kaie
Severity: normal → enhancement
Priority: -- → P2
Target Milestone: 3.13.3 → 3.13.4

Updated

6 years ago
Target Milestone: 3.13.4 → 3.13.5
(Assignee)

Comment 1

6 years ago
Created attachment 618870 [details] [diff] [review]
Patch v2

In attachment 616669 [details] [diff] [review] (in bug 360420), we got a patch from chromium that contains changes to tstclnt.

The only functional change was a status message that prints a statement if cert status data was received in the handshake.

I enhanced the tstclnt patch contained in this bug to product equivalent output, compatible with patch v16 from bug 360420.
Attachment #572857 - Attachment is obsolete: true
(Assignee)

Updated

6 years ago
Target Milestone: 3.13.5 → 3.14
(Assignee)

Comment 2

6 years ago
Created attachment 618872 [details] [diff] [review]
patch v2 with -u20 -p
(Assignee)

Comment 3

6 years ago
How could OCSP stapling get tested automatically?

- we must disable outgoing connections to OCSP servers
- we must disable outgoing connections to fetch CRLs
- we must connect to an SSL server with OCSP stapling enabled
- we must verify the server certificate,
  using a verification function that requires 
  that fresh revocation data is available

I don't know of a way to perform this test using the classic verification engine.

I propose we enhance the tstclnt utility to perform these steps when requested
with a parameter, and it must use the CERT_PKIXVerifyCert API.
(In reply to Kai Engert (:kaie) from comment #3)
> How could OCSP stapling get tested automatically?
> 
> - we must disable outgoing connections to OCSP servers
> - we must disable outgoing connections to fetch CRLs
> - we must connect to an SSL server with OCSP stapling enabled
> - we must verify the server certificate,
>   using a verification function that requires 
>   that fresh revocation data is available

Instead of modifying tstclnt and selfserv, we can use the loopback testing framework in bug 702322. I started it with the intention of using it for NPN and OCSP stapling. With an extension of that framework, we can do a test like the following:

Prerequsitites:
1. Add a hook to the the server side of libssl that adds the OCSP stapling extension, without implementing the server side of OCSP stapling in libssl itself (except for a couple of lines to add support for the hook, if I didn't already write that).
2. Add a hook to the server side of libssl that adds the CertificateStatus message to the server side of libssl, without modifying libssl itself (again, except for a couple of lines to add support for the hook). This hook would take an encoded OCSP response as a parameter.

Test 1: Ensure that SSL_RevealCertStatus works within the certificate auth hook.

1. Generate a blob of data to simulate an OCSP response; it doesn't actually have to be a valid OCSP response.
2. Enable OCSP stapling on the client side.
3. Enable the hooks from above; for hook #2, pass in the generated OCSP response.
4. Set the client-side connection's certificate auth hook to a function that calls SSL_RevealCertStatus and verifies that it returns exactly one item that is exactly equal to the OCSP response, and then returns true.
5. Use the convenience functions in the framework to drive the handshake to completion.

Test 2: Ensure that the auth cerificate hook is called before the client auth hook is called when a certificate status is provided:

1, 2, 3: same as above
4. Configure the server side to request a client certificate
5. Set the client-side connection's certificate auth hook to a function that calls SSL_RevealCertStatus and verifies that it returns exactly one item that is exactly equal to the OCSP response, sets a flag indicating that the cert verification occured, and then returns true.
6. Add a client auth data hook to the client side of the connection, that verifies hat the flag was set.

Test 3: Ensure that the auth cerificate hook is called before the client auth hook is called when no certificate status is provided:

* Same as the previous test, except have hook #2 skip sending the CertificateStatus message.

In PSM (in a PSM bug, not this one), we will need additional tests:
1. Ensure that we process REVOKED responses correctly
2. Ensure that we process GOOD responses correctly
3. Ensure that we process unsigned responses (e.g. 5 - Try Later) correctly
4. Ensure that we process malformed responses correctly
...

The PSM-level tests are more difficult because we actually care about what the blob of data contains, and we may actually have to do certificate validation in some of those cases.

I suppose that we should have the same tests as the PSM-level tests, for the built-in SSL_AuthCertificate function. But, IMO, that is asking too much. If I were to want to test the built-in functionality, I would rather write tests that test the interaction between CERT_CacheOCSPResponseFromSideChannel and CERT_[PKIX]VerifyCert directly. IMO, that should also be done in a separate bug.
(Assignee)

Comment 5

6 years ago
Brainstorming: A new mode for tstclnt could work like this:

-F                   Ping server, verify cert using CERT_PKIXVerifyCert,
                     disable dynamic fetching of OCSP and CRL,
                     require fresh OCSP revocation info is available,
                     (this test is useful with -C). Exit with code:
                     0: have fresh and valid revocation data, status good
                     1: cert bad, revocation data not checked
                     2: old or invalid revocation data
                     3: have fresh and valid revocation data, status revoked

-C                   Enable the cert_status extension (OCSP stapling).


The implementation could run several separate verifications.

Verify with revocation checking disabled.
On failure return 1.

if result if "good", then continue testing.

Verify with CERT_REV_M_FAIL_ON_MISSING_FRESH_INFO.
If result is good, return 0.

On failure continue testing, find out why it failed.

Verify with CERT_REV_M_IGNORE_MISSING_FRESH_INFO

If result is "good", then our previous test failed,
because we don't have fresh revocation info, return 2.

If result is still bad, we do have revocation info,
and it says "revoked" or something equivalent, return 3.
(Assignee)

Comment 6

6 years ago
I don't like the proposal in comment 4.
I don't want simulated OCSP responses, and I don't want to introduce simulation hooks.

I prefer to test the real code path with real OCSP response data that will go through the OCSP response signature and freshness checking.
(Assignee)

Comment 7

6 years ago
If you want to continue the discussion around self-contained OCSP stapling testing, please move such work and discussion to a separate bug. IMHO such work must involve local generation of OCSP responses.

In this bug I want to find a simple and minimal solution that will test against known available servers. We had discussed in the NSS conference call that such minimal testing would be sufficient for testing the initial ingegration of the OCSP stapling code.
(Assignee)

Updated

6 years ago
Depends on: 554369
(Assignee)

Comment 8

6 years ago
I'll change my patch to use -P to enable the cert status extension,
because that's the chrome patch attached in bug 360420 currently uses,
let's avoid confusion.
(Assignee)

Comment 9

6 years ago
Created attachment 619099 [details] [diff] [review]
Patch v3

Using this patch, plus 360420 plus 554369,
I got positive test results for the following scenarios:

#define EXIT_CODE_SIDECHANNELTEST_GOOD 0
#define EXIT_CODE_SIDECHANNELTEST_BADCERT 1
#define EXIT_CODE_SIDECHANNELTEST_NODATA 2
#define EXIT_CODE_SIDECHANNELTEST_REVOKED 3


Sites that support OCSP stapling and use a valid certificate:

$ tstclnt -2 -3 -P -v -F -M 1 -O -h kuix.de -p 5143 -d /tmp/nssdb/
Received 1 Cert Status items (OCSP stapled data)
tstclnt: exiting with return code 0

$ tstclnt -2 -3 -P -v -F -M 1 -O -h kuix.de -p 5146 -d /tmp/nssdb/
Received 1 Cert Status items (OCSP stapled data)
tstclnt: exiting with return code 0

$ tstclnt -2 -3 -P -v -F -M 1 -O -h login.live.com -p 443 -d /tmp/nssdb/
Received 1 Cert Status items (OCSP stapled data)
tstclnt: exiting with return code 0


Sites that support OCSP stapling and use a revoked certificate:

$ tstclnt -2 -3 -P -v -F -M 1 -O -h kuix.de -p 5144 -d /tmp/nssdb/
Received 1 Cert Status items (OCSP stapled data)
tstclnt: exiting with return code 3

$ tstclnt -2 -3 -P -v -F -M 1 -O -h kuix.de -p 5147 -d /tmp/nssdb/
Received 1 Cert Status items (OCSP stapled data)
tstclnt: exiting with return code 3


Site that doesn't support OCSP stapling and uses a valid certificate:

$ tstclnt -2 -3 -P -v -F -M 1 -O -h kuix.de -p 443 -d /tmp/nssdb/
tstclnt: exiting with return code 2


Site that uses a certificate not trusted by NSS by default:

$ tstclnt -2 -3 -P -v -F -M 1 -O -h www.cacert.org -p 443 -d /tmp/nssdb/
tstclnt: exiting with return code 1
Attachment #618870 - Attachment is obsolete: true
Attachment #618872 - Attachment is obsolete: true
(Assignee)

Updated

6 years ago
Blocks: 749714
(Assignee)

Updated

6 years ago
Assignee: kaie → nobody

Updated

6 years ago
Target Milestone: 3.14 → 3.14.1
(Assignee)

Updated

6 years ago
Target Milestone: 3.14.1 → 3.14.2

Updated

5 years ago
Assignee: nobody → kaie

Comment 10

5 years ago
Kai: Assigning to you based on your Patch #3. Is it needing review?
(Assignee)

Comment 11

5 years ago
Comment on attachment 619099 [details] [diff] [review]
Patch v3

I've folded tstclnt/ssltap enhancements into the patch attached to and havinvg received r+ in bug 360420. I will mark this bug as resolved/duplicate, once bug 360420 is done.
Attachment #619099 - Attachment is obsolete: true
(Assignee)

Updated

5 years ago
Blocks: 841792
(Assignee)

Comment 12

5 years ago
checked in, see bug 360420 comment 161
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: 3.14.2 → 3.14.4
(Assignee)

Updated

5 years ago
Target Milestone: 3.14.4 → 3.15
You need to log in before you can comment on or make changes to this bug.