libtls: Support OCSP #47
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 30 commits
September 12, 2015 15:04
primality, do not unnecessarily convert the original decimal number to hex in the output. Hex numbers explicitly specified with -hex remain unchanged. ok beck@ deraadt@ jsing@ miod@
It was the only thing preventing -Werror from building on some systems due to the unchecked asprintf's.
This adds aes-128-gcm aes-256-gcm chacha20-poly1305 from Adam Langley's original patch for OpenSSL ok beck@ jsing@
We can now assume >= TLS v1.0 since SSL2_VERSION, SSL3_VERSION and DTLS1_BAD_VER support was removed. "reads ok" miod@
…irect and the symbols not in the C standard are weak
has been superseded by OPENSSL_CONF and discouraged from use for almost 16 years. "Definately ok" jsing@ "burn it" deraadt@ "Kill it with fire" miod@ "KILL IT WITH FIRE!!! BURN!!!!" beck@
… call that we will pass the result through tls_ssl_error() on failure. Otherwise we can end up reporting spurious errors due to their being unrelated errors already on the error stack. Spotted by Marko Kreen. ok beck@
functions. The original was written as a huge if/else if chain - split out the handling for each key exchange type. This allows us to reduce two levels of indentation, make the code far more readable and have single return paths so that we can simplify clean up. ok beck@
…lled in at handshake time. change accessors to return const char * to remove need for caller to free memory. ok jsing@
C standard are all weak.
Apply __{BEGIN,END}_HIDDEN_DECLS to gdtoa{,imp}.h, hiding the
arch-specific __strtorx, __ULtox_D2A, __strtorQ, __ULtoQ_D2A symbols.
…ipher over CHACHA20. Otherwise, prefer CHACHA20 with AES second. ok beck@ miod@
…unction, then decrement it and call a callback on exit from the function. As such, these functions should not return in the middle, otherwise in_handshake is never decremented and the callback never called. ok beck@ "with many sighs" miod@
cpu's specific hardware capabilities users of libcrypto might be interested in, as an integer value. This deprecates the existing OPENSSL_ia32cap() macro and the OPENSSL_ia32cap_loc() function (which returns the pointer so that you can mess with stuff you shouldn't mess with). Interpreting the value returned by OPENSSL_cpu_caps() is, of course, machine-dependent. Minor version bump for libcrypto. ok beck@ jsing@
C=FR, O=Certplus, CN=Class 2 Primary CA req by beck@, ok miod@ beck@
Currently, if you call ECDH_compute_key() it will silently truncate the resulting key if the output buffer is less than the key size. Instead, detect this condition and return an error. If the buffer provided is larger than the key length, zero the remainder. ok beck@ miod@ "+ shivers"
"jajaja" miod@
ok deraadt@ "hurray! finally!" miod@ "Yay!" sthen@
added 8 commits
November 25, 2015 15:49
straight replace: thanks both to schwarze and maja for feedback on how to rewrite parts; i've snuck in an rcs id->openbsd id change in dev_mkdb too;
ok jcs@ deraadt@ theo@
ok krw@
ok djm@ jsing@
|
Poke Poke... the ASN1_TIME changes whent in a little while ago.. this could be reworked |
Knuth-Fisher-Yates shuffle to make the random sequence of ports less biased. Based on the implementation in sys/netinet/ip_id.c. With helpful input from daniel@ and beck@ ok beck@ despite eye twitching
This enables ENGINE_get_digest to work again with SHA1. noted by NARUSE, Yui, @nurse from github
For situations where there can be deep and complex reason for libssl function failure that can be conveyed to user by simply passing libssl 'reason' field from error state. OCSP code will use it to show 'verify' failue, but it should be used in more places in current libtls code too.
Here is support for stapling for both client and server,
plus API to create and process OCSP network requests.
1) Stapling
* Client requests and verifies by default.
* Server attaches stapling response, if configured. Config API:
- int tls_config_set_ocsp_stapling_file(struct tls_config *cf, const char *fn);
- int tls_config_set_ocsp_stapling_mem(struct tls_config *cf, const uint8_t *blob, size_t len);
File is loaded on each request. Some sort of cronjob can be set up to
update it periodically. File and mem is not checked in any way, assumes
admin has set it up properly. Note that network requests *do* validate
server response.
2) Network request API
int tls_ocsp_check_peer_request(struct tls **ocsp_ctx_p,
struct tls *target, char **ocsp_url,
void **request_blob, size_t *request_size);
int tls_ocsp_refresh_stapling_request(struct tls **ocsp_ctx_p,
struct tls_config *config, char **ocsp_url,
void **request_blob, size_t *request_size);
int tls_ocsp_process_response(struct tls *ctx,
const void *response_blob, size_t size);
tls_ocsp_refresh_stapling_request() creates request for cert in config.
Returns TLS_NO_OCSP if cert has no ocsp urls.
tls_ocsp_check_peer() creates request for cert in peer cert. Retruns
TLS_NO_OCSP if cert has no OCSP urls set up.
Both create 'struct tls' to attach errors and keep state for response
processing. The ocsp_url and response_blob blob also are attached
to it so called does not need to free them.
tls_ocsp_process_response() accepts response that was sent by
OCSP responder, verifies and parses it. It will update config
if 'struct tls' was created by tls_ocsp_refresh_stapling_request().
3) Info API.
int tls_get_ocsp_info(struct tls *ctx, int *response_status,
int *cert_status, int *crl_reason,
time_t *this_update, time_t *next_update, time_t *revoction_time,
const char **result_text);
This function operates on both client-side 'struct tls' to check
stapled response from server, and on 'struct tls' created by
tls_ocsp_check_peer_request() / tls_ocsp_refresh_stapling_request()
where it returns info received from responder.
Situations:
* No OCSP cert, no stapling from server: returns TLS_NO_OCSP,
fills result_text with "no-ocsp"
* Have valid OCSP response with good signature - returns 0. All fields
are set. This does not mean cert is valid, it can still have
revoked status. Or responser may have rejected the request.
* OCSP response validation failed, some network error - returns -1.
*result_text has verification failure message.
If ctx is OCSP network context, it has error message. If client
connection to server, it has some later error.
result_text:
"no-ocsp" - TLS_NO_OCSP
"good", "unknown" - successful result
revocation reason
request rejection reason
"error" - processing failed
4) Open questions.
How should it coordinate with noverifycert()? Check and don't report,
don't check, or don't even ask? Should the OCSP info be visible
or not when noverifycert() is on?
The ->ocsp_result field usage is slightly weird but necessary:
it tracks whether there was any OCSP activity at all. ocsp_info
is set only on sucessful response.
|
Ok, here is resynced patch. I separated tls_set_error_libssl() to make it clearer. This patch still contains workaround for #45 as it's unfixed it libressl. |
|
Erg, I was reviewing PRs just now, and I'm pretty sure all of us completely missed your updates :( |
|
oooh. i missed that he had updated this too illl have a peek On Sunday, 31 January 2016, Brent Cook notifications@github.com wrote:
|
|
I'm in your PR marko.. playing with your stuff |
|
we can remove buggy-verify now |
|
Marko, I've made this work with -current OpenBSD, and posted a diff (with your test progtram) to tech@openbsd.org so we can start reviewing it.. there will probably be a One of the things I noticably removed from it before posting was the addition of tls_error_libssl()... I see where you're going with that, but it's kind of a separate issue |
|
I'm targeting this to go in post OpenBSD 6.0 release with some changes |
|
I think Bob is working on an updated version now, discussion will move to tech@openbsd.org. I'm going to close this PR as-is since it was damaged from the recent CVS tree updates. |
|
While this has been closed, the code is now in-tree and should show up in the next portable release. |
|
Thanks for merging it. I agree with leaving tls_error_libssl() out, it should come as whole tree-change for all situations where libssl's 'reason' contains additional details about failure that would be useful for user to see. But I noticed you have changed two TLS_OCSP_RESPONSE_ codes from values I picked up from RFC6960 section 4.2.1. The comment references section 2.3 but that does not contain any values. Is that an oversight or is there a reason behind that change? |
|
likely an oversight on my part. let me take a quick look
…On Tue, Dec 27, 2016 at 09:29 Marko Kreen ***@***.***> wrote:
Thanks for merging it.
I agree with leaving tls_error_libssl() out, it should come as whole
tree-change for all situations where libssl's 'reason' contains additional
details about failure that would be useful for user to see.
But I noticed you have changed two TLS_OCSP_RESPONSE_ codes from values I
picked up from RFC6960 section 4.2.1. The comment references section 2.3
but that does not contain any values.
Is that an oversight or is there a reason behind that change?
—
You are receiving this because you were assigned.
Reply to this email directly, view it on GitHub
<#47 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AHv2dYV5e9gYnbuyHlNCqF_yw7WDyrvLks5rMS4dgaJpZM4F8cvY>
.
|
|
Heh.. definately an overaggresive cleanup on either joel's or my part.. I
have the obvious fix being reviewed.. thanks
…On Thu, Dec 29, 2016 at 4:17 PM, Bob Beck ***@***.***> wrote:
likely an oversight on my part. let me take a quick look
On Tue, Dec 27, 2016 at 09:29 Marko Kreen ***@***.***>
wrote:
> Thanks for merging it.
>
>
> I agree with leaving tls_error_libssl() out, it should come as whole
> tree-change for all situations where libssl's 'reason' contains additional
> details about failure that would be useful for user to see.
>
>
> But I noticed you have changed two TLS_OCSP_RESPONSE_ codes from values I
> picked up from RFC6960 section 4.2.1. The comment references section 2.3
> but that does not contain any values.
>
>
> Is that an oversight or is there a reason behind that change?
>
>
>
>
> —
> You are receiving this because you were assigned.
> Reply to this email directly, view it on GitHub
> <#47 (comment)>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/AHv2dYV5e9gYnbuyHlNCqF_yw7WDyrvLks5rMS4dgaJpZM4F8cvY>
> .
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Here is support for stapling for both client and server,
plus API to launch OCSP network requests.
File is loaded on each request. Some sort of cronjob can be set up to
update it periodically. File and mem is not checked in any way, assumes
admin has set it up properly. Note that network requests do validate
server response.
tls_ocsp_refresh_stapling() loads response over network and attaches
to config. Returns TLS_NO_OCSP if cert has no ocsp urls.
tls_ocsp_check_peer() verifies peer cert over network. Returns
TLS_NO_OCSP if cert has no OCSP urls set up.
If async_fd_p is unset, they operate in sync mode. If set, a newly
created fd will be put there that can be used with any polling method.
And they return TLS_WANT_POLLIN / TLS_WANT_POLLOUT as needed. When poll
notifies about event, the call needs to be repeated.
In both async and sync mode they create new "struct tls" that can
be examined for errors and final OCSP details.
For client connection it returns info about stapled OCSP response.
For network requests it returns what OCSP responder sent.
Situations:
fills result_text with "no-ocsp"
are set. This does not mean cert is valid, it can still have
revoked status. Or responser may have rejected the request.
*result_text has verification failure message.
If ctx is OCSP network context, it has error message. If client
connection to server, it has some later error.
result_text:
"no-ocsp" - TLS_NO_OCSP
"good", "unknown", "revoked: ..." - successful result
"..." - error about failure.
How should it coordinate with noverifycert(): check and don't report
don't check, or don't even ask? Should the OCSP info be visible
or not when noverifycert() is on?
The ->ocsp_result field usage is slightly weird but necessary:
it tracks whether there was any OCSP activity at all. ocsp_info
is set only on sucessful response. Also, OCSP errors during
stapling check will be lost, it will track them.
tls_ocsp_refresh_stapling() makes it actually usable.
to test the code - both server refresh and actual verification -
against normal public servers. So it's worth having on purely
testability grounds.
connection. Or server that checks client cert. Instead OCSP
responses should be cached, and request is made when cache
expires. So the tls_ocsp_check_peer() should not be integrated
into basic tls_connect(). libtls could give