-
Notifications
You must be signed in to change notification settings - Fork 9
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
COCOS-192 - Attested TLS #279
base: main
Are you sure you want to change the base?
Conversation
internal/server/grpc/grpc.go
Outdated
if err != nil { | ||
return fmt.Errorf("failed to listen on port %s: %w", s.Address, err) | ||
} | ||
// listener, err := net.Listen("tcp", s.Address) |
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.
Dead code?
manager/agentEventsLogs.go
Outdated
@@ -17,7 +17,7 @@ import ( | |||
) | |||
|
|||
const ( | |||
ManagerVsockPort = 9997 | |||
ManagerVsockPort = 9998 // CHANGED BY ME |
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.
Take care about these comments
pkg/tls_extensions/atls_extensions.h
Outdated
#define CLIENT_RANDOM_SIZE 32 | ||
#define TLS_CLIENT_CTX 0 | ||
#define TLS_SERVER_CTX 1 | ||
#define SSL_ERROR_WANT_READ_WRITE 2 |
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.
#define SSL_ERROR_WANT_READ_WRITE 2 | |
#define SSL_ERROR_WANT_WRITE 2 | |
#define SSL_ERROR_WANT_READ 2 |
pkg/tls_extensions/atls_extensions.h
Outdated
#define TLS_SERVER_CTX 1 | ||
#define SSL_ERROR_WANT_READ_WRITE 2 | ||
|
||
typedef struct tls_server_c |
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.
typedef struct tls_server_c | |
typedef struct tls_server_connection |
pkg/tls_extensions/atls_extensions.h
Outdated
struct sockaddr_storage addr; | ||
} tls_server_connection; | ||
|
||
typedef struct tls_c |
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.
typedef struct tls_c | |
typedef struct tls_connection |
pkg/tls_extensions/atls_extensions.c
Outdated
const unsigned char *out, | ||
void *add_arg) | ||
{ | ||
// fprintf(stderr, "evidence_request_ext_free_cb called\n"); |
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.
// fprintf(stderr, "evidence_request_ext_free_cb called\n"); | |
free((void *)out); | |
// fprintf(stderr, "evidence_request_ext_free_cb called\n"); |
pkg/tls_extensions/atls_extensions.c
Outdated
const unsigned char *out, | ||
void *add_arg) | ||
{ | ||
// fprintf(stderr, "attestation_certificate_ext_free_cb from server called\n"); |
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.
// fprintf(stderr, "attestation_certificate_ext_free_cb from server called\n"); | |
free((void *)out); | |
// fprintf(stderr, "attestation_certificate_ext_free_cb from server called\n"); |
pkg/tls_extensions/atls_extensions.c
Outdated
{ | ||
unsigned char* client_random_buffer = malloc(CLIENT_RANDOM_SIZE); | ||
unsigned char* client_random_print_buffer = malloc(CLIENT_RANDOM_SIZE * 2 + 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.
if (client_random_buffer == NULL || client_random_print_buffer == NULL) { | |
free(client_random_buffer); | |
free(client_random_print_buffer); | |
*al = SSL_AD_INTERNAL_ERROR; | |
return 0; | |
} |
pkg/tls_extensions/atls_extensions.c
Outdated
{ | ||
unsigned char *hash = malloc(SHA256_DIGEST_LENGTH); | ||
// fprintf(stderr, "attestation_certificate_ext_add_cb called\n"); | ||
|
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.
if (hash == NULL) { | |
*al = SSL_AD_INTERNAL_ERROR; | |
return 0; | |
} |
pkg/tls_extensions/atls_extensions.c
Outdated
/* | ||
Attestation Certificate extension | ||
- Contains the attestation report | ||
- The attestation report contains the hash of the nonce and the Publik Key of the x.509 Agent 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.
- The attestation report contains the hash of the nonce and the Publik Key of the x.509 Agent certificate | |
- The attestation report contains the hash of the nonce and the Public Key of the x.509 Agent certificate |
sprint_string_hex(hex_buffer, hash, 32); | ||
// fprintf(stderr, "Receiving sha256 of public key from server: %s\n", hex_buffer); | ||
} | ||
|
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.
free(hex_buffer); |
void *parse_arg) | ||
{ | ||
char* hex_buffer = malloc(inlen*2 + 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.
if (hex_buffer == NULL) { | |
*al = SSL_AD_INTERNAL_ERROR; | |
return 0; | |
} |
pkg/tls_extensions/atlsLIstener.go
Outdated
|
||
sec, usec := timeToTimeout(t) | ||
if C.set_socket_timeout(c.tlsConn, C.int(sec), C.int(usec)) < 0 { | ||
return fmt.Errorf("could not set deadline") |
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.
return fmt.Errorf("could not set deadline") | |
return fmt.Errorf("could not set deadline, socket timeout failed") |
pkg/tls_extensions/atlsLIstener.go
Outdated
func (c *CustomTLSConn) SetReadDeadline(t time.Time) error { | ||
fmt.Fprintf(os.Stderr, "SetReadDeadline - SetDeadline\n") | ||
if c.SetDeadline(t) != nil { | ||
return fmt.Errorf("could not set write deadline") |
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.
return fmt.Errorf("could not set write deadline") | |
return fmt.Errorf("could not set read deadline") |
return 0, 0 | ||
} | ||
|
||
d := time.Until(t) |
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.
d := time.Until(t) | |
d := time.Until(t) | |
if d<=0 { | |
return 0,0 | |
} |
pkg/tls_extensions/atls_listener.c
Outdated
free(conn); | ||
close(conn->socket_fd); |
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.
free(conn); | |
close(conn->socket_fd); | |
close(conn->socket_fd); | |
free(conn); |
pkg/tls_extensions/atls_listener.c
Outdated
free(conn); | ||
SSL_free(conn->ssl); |
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.
free(conn); | |
SSL_free(conn->ssl); | |
SSL_free(conn->ssl); | |
free(conn); |
return nil | ||
} | ||
|
||
func FetchAttestation(reportDataSlice []byte) []byte { |
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.
propagate error
func VerifyAttestationReportTLS(attestationBytes []byte, reportData []byte) int { | ||
logger.Init("", false, false, io.Discard) | ||
|
||
AttConfigurationSEVSNP.SNPPolicy.ReportData = reportData[:] |
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.
SNPPolicy could be nil, this could panic
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.
try combining with exisiting code in cli, to avoid repetition
RootOfTrust *check.RootOfTrust `json:"root_of_trust,omitempty"` | ||
} | ||
|
||
func VerifyAttestationReportTLS(attestationBytes []byte, reportData []byte) int { |
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.
the idiomatic golang way is to return an error
) | ||
|
||
var ( | ||
AttConfigurationSEVSNP = AttestationConfiguration{} |
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.
avoid package level variable, this is not thread safe
defer c.fdReadMutex.Unlock() | ||
|
||
if c.tlsConn == nil { | ||
return 0, fmt.Errorf("tls connection is 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.
return 0, fmt.Errorf("tls connection is NULL") | |
return 0, fmt.Errorf("tls connection is nil") |
also declare errors using vars
func (l *ATLSServerListener) Accept() (net.Conn, error) { | ||
conn := C.tls_server_accept(l.tlsListener) | ||
if conn == nil { | ||
return &ATLSConn{tlsConn: nil}, nil |
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.
if conn is nil, return an error
defer c.fdWriteMutex.Unlock() | ||
|
||
if c.tlsConn == nil { | ||
return 0, fmt.Errorf("tls connection is 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.
return 0, fmt.Errorf("tls connection is NULL") | |
return 0, fmt.Errorf("tls connection is nil") |
} | ||
|
||
if (ext_data != NULL) { | ||
if (RAND_bytes(ext_data->er.data, CLIENT_RANDOM_SIZE) < 0) { |
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.
probably != 1 rather than <0
return 0; | ||
} | ||
|
||
attestation_report = triggerFetchAttestationCallback(ext_data->fetch_attestation_handler, hash); |
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.
probably requires some nil check before assignment
What type of PR is this?
This feature is an enhancement for attested TLS. The idea is to have the attestation fetched and forwarded as a TLS extension. This way, we can guarantee the freshness of the evidence.
What does this do?
Adds custom TLS extensions that transfer the nonce and the attestation report between the client (CLI) and server (Agent).
Which issue(s) does this PR fix/relate to?
Have you included tests for your changes?
No, the process is tested as before. There is no need for new tests.
Did you document any new/modified feature?
No, the documentation will be added upon completion.
Notes
This PR is still a draft.