Skip to content
This repository was archived by the owner on Dec 1, 2025. It is now read-only.

add OCSP status worker #336

Merged
jvehent merged 11 commits intomozilla:masterfrom
adamdecaf:ocspStatus
May 8, 2018
Merged

add OCSP status worker #336
jvehent merged 11 commits intomozilla:masterfrom
adamdecaf:ocspStatus

Conversation

@adamdecaf
Copy link
Copy Markdown
Contributor

This builds on #286 and takes the work done there to finish the worker.

Issue: #199

--- Analyzers ---
* CAA records: not found
* CRL: Not Revoked
* Mozilla evaluation: non compliant
  - for modern level: remove ciphersuites AES128-GCM-SHA256, AES128-SHA256, DHE-RSA-AES128-GCM-SHA256, DHE-RSA-AES128-SHA256, AES128-SHA, ECDHE-RSA-AES128-SHA, DHE-RSA-AES128-SHA, AES256-GCM-SHA384, AES256-SHA256, DHE-RSA-AES256-GCM-SHA384, DHE-RSA-AES256-SHA256, AES256-SHA, ECDHE-RSA-AES256-SHA, DHE-RSA-AES256-SHA, DES-CBC3-SHA, EDH-RSA-DES-CBC3-SHA
  - for modern level: consider adding ciphers ECDHE-ECDSA-AES256-GCM-SHA384, ECDHE-ECDSA-CHACHA20-POLY1305, ECDHE-RSA-CHACHA20-POLY1305, ECDHE-ECDSA-AES128-GCM-SHA256, ECDHE-ECDSA-AES256-SHA384, ECDHE-ECDSA-AES128-SHA256
  - for modern level: remove protocols SSLv3, TLSv1, TLSv1.1
  - for modern level: enable Perfect Forward Secrecy with a curve of at least 256bits, don't use DHE
  - for modern level: use a certificate of type ecdsa, not RSA
  - oldest clients: 
* Grade: A (90/100)
* OCSP Status: 
 - Not revoked
* SSLLabs Client Support: showing oldest known clients
  - Firefox 10.0.12 ESR, Chrome 27, Edge 12, IE 6, Safari 5, Opera 12.15, Android 2.3.7, OpenSSL 0.9.8y, Java 6u45
* Symantec distrust: not impacted

}
if resp.Status == ocsp.Revoked {
status.RevokedAt = resp.RevokedAt
}
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The revoked_at field is still stored here, but it looks to be detected by .IsZero(), which doesn't really make sense as a revocation time anyway.

The status field is always checked in the printer anyway.

tls_observatory=# select * from analysis where worker_name='ocspStatus' and scan_id=5; 
 id | scan_id | worker_name | success |                       output                        
----+---------+-------------+---------+-----------------------------------------------------
 25 |       5 | ocspStatus  | t       | {"status": 0, "revoked_at": "0001-01-01T00:00:00Z"}

@adamdecaf
Copy link
Copy Markdown
Contributor Author

It looks like there's some (stalled) work on a badssl.com example for revoked/expired OCSP. I don't see an example on their main site.

chromium/badssl.com#54

@ghost
Copy link
Copy Markdown

ghost commented Apr 15, 2018

@adamdecaf Thanks for finishing this! I was trying to find time to complete this, but I've been really busy lately.

Copy link
Copy Markdown
Contributor

@jvehent jvehent left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works locally and code looks good, just one nit on the printer output.

results = append(results, fmt.Sprintf(" - Revoked at %s\n", result.RevokedAt.Format(time.RFC3339)))
default:
results = append(results, fmt.Sprintf(" - Unknown status code %d\n", result.Status))
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets put everything on a single line like we did for the CAA and CRL workers. The output should be fmt.Sprintf("* OCSP: <status>"). Also, don't include a newline, the printer does that automatically.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jvehent Done!

@jvehent jvehent merged commit 8f2d8d7 into mozilla:master May 8, 2018
@adamdecaf adamdecaf deleted the ocspStatus branch May 8, 2018 18:12
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants