-
Notifications
You must be signed in to change notification settings - Fork 22
Implement TLS helper #953
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
base: main
Are you sure you want to change the base?
Implement TLS helper #953
Conversation
This adds a ggl-tls-helper binary that supports standard connections. Proxy and TPM support is not yet supported.
} | ||
|
||
BIO *bio = BIO_new_socket(tcp_fd, BIO_NOCLOSE); | ||
if (bio == NULL) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we use GGL_CLEANUP here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On the success path, the SSL takes ownership of it; in error paths we exit. So not much need to do so other than satisfying address sanitizer on error paths in debug builds.
ERR_print_errors_cb(openssl_err_cb, NULL); | ||
return GGL_ERR_CONFIG; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happens in absence of the key and certificate?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We just don't do mTLS if no key and cert is passed.
fds[0].fd = -1; | ||
} else if (ret != GGL_ERR_OK) { | ||
GGL_LOGE("Proxy socket read error."); | ||
return 1; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we be closing the TLS socket at this point?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Exiting closes all our fds.
(void) shutdown(tcp_fd, SHUT_WR); | ||
fds[0].fd = -1; | ||
} else if (ret != GGL_ERR_OK) { | ||
GGL_LOGE("Proxy socket read error."); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overarching comment - we are using Proxy here interchangeably with the HTTP/HTTPs proxy - it is a bit confusing,
GGL_LOGE("Openssl shutdown on network socket failed."); | ||
ERR_print_errors_cb(openssl_err_cb, NULL); | ||
} | ||
(void) shutdown(tcp_fd, SHUT_WR); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: maybe use fds[1]
?
I mean maybe pick one - we are using fds[]
and tcp_fd
/proxy_fd
interchangeably.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fds[1] is the handle in the poll which we use for checking the poll and disabling the slot by setting it to -1. tcp_fd will remain valid even after we clear the poll slot.
The TLS helper enables sandboxing TLS functionality into a separate process, enabling easily switching out TLS implementations, and allowing us to be resilient to resource leaks in the TLS handling code.