COCOS-192 - Attested TLS #279

danko-miladinovic · 2024-10-08T15:58:40Z

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?

Resolves Feature: Enhance attested TLS #192

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.

drasko · 2024-10-08T16:02:34Z

internal/server/grpc/grpc.go

-	if err != nil {
-		return fmt.Errorf("failed to listen on port %s: %w", s.Address, err)
-	}
+	// listener, err := net.Listen("tcp", s.Address)


drasko · 2024-10-08T16:02:52Z

manager/agentEventsLogs.go

@@ -17,7 +17,7 @@ import (
 )

 const (
-	ManagerVsockPort     = 9997
+	ManagerVsockPort     = 9998 // CHANGED BY ME


Take care about these comments

SammyOina · 2024-10-14T07:59:37Z

pkg/tls_extensions/atls_extensions.h

+#define CLIENT_RANDOM_SIZE 32
+#define TLS_CLIENT_CTX 0
+#define TLS_SERVER_CTX 1
+#define SSL_ERROR_WANT_READ_WRITE 2


Suggested change

#define SSL_ERROR_WANT_READ_WRITE 2

#define SSL_ERROR_WANT_WRITE 2

#define SSL_ERROR_WANT_READ 2

SammyOina · 2024-10-14T08:00:57Z

pkg/tls_extensions/atls_extensions.h

+#define TLS_SERVER_CTX 1
+#define SSL_ERROR_WANT_READ_WRITE 2
+
+typedef struct tls_server_c


Suggested change

typedef struct tls_server_c

typedef struct tls_server_connection

SammyOina · 2024-10-14T08:08:58Z

pkg/tls_extensions/atls_extensions.h

+    struct sockaddr_storage addr;
+} tls_server_connection;
+
+typedef struct tls_c


Suggested change

typedef struct tls_c

typedef struct tls_connection

SammyOina · 2024-10-14T08:11:48Z

pkg/tls_extensions/atls_extensions.c

+                                    const unsigned char *out,
+                                    void *add_arg)
+{
+    // fprintf(stderr, "evidence_request_ext_free_cb called\n");


Suggested change

// fprintf(stderr, "evidence_request_ext_free_cb called\n");

free((void *)out);

// fprintf(stderr, "evidence_request_ext_free_cb called\n");

SammyOina · 2024-10-14T08:12:53Z

pkg/tls_extensions/atls_extensions.c

+                                    const unsigned char *out,
+                                    void *add_arg)
+{
+    // fprintf(stderr, "attestation_certificate_ext_free_cb from server called\n");


Suggested change

// 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");

SammyOina · 2024-10-15T07:19:40Z

pkg/tls_extensions/atls_extensions.c

+        {
+            unsigned char* client_random_buffer = malloc(CLIENT_RANDOM_SIZE);
+            unsigned char* client_random_print_buffer = malloc(CLIENT_RANDOM_SIZE * 2 + 1);
+


Suggested change

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;

}

SammyOina · 2024-10-15T07:20:30Z

pkg/tls_extensions/atls_extensions.c

+{
+    unsigned char *hash = malloc(SHA256_DIGEST_LENGTH);
+    // fprintf(stderr, "attestation_certificate_ext_add_cb called\n");
+


Suggested change

if (hash == NULL) {

*al = SSL_AD_INTERNAL_ERROR;

return 0;

}

SammyOina · 2024-10-15T07:21:03Z

pkg/tls_extensions/atls_extensions.c

+/*
+    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


Suggested change

- 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

SammyOina · 2024-10-15T07:24:23Z

pkg/tls_extensions/atls_extensions.c

+        sprint_string_hex(hex_buffer, hash, 32);
+        // fprintf(stderr, "Receiving sha256 of public key from server: %s\n", hex_buffer);
+    }
+


Suggested change

free(hex_buffer);

SammyOina · 2024-10-15T07:26:10Z

pkg/tls_extensions/atls_extensions.c

+                                          void *parse_arg)
+{
+    char* hex_buffer = malloc(inlen*2 + 1); 
+


Suggested change

if (hex_buffer == NULL) {

*al = SSL_AD_INTERNAL_ERROR;

return 0;

}

SammyOina · 2024-10-15T07:32:37Z

pkg/tls_extensions/atlsLIstener.go

+
+	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")


Suggested change

return fmt.Errorf("could not set deadline")

return fmt.Errorf("could not set deadline, socket timeout failed")

SammyOina · 2024-10-15T07:34:41Z

pkg/tls_extensions/atlsLIstener.go

+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")


Suggested change

return fmt.Errorf("could not set write deadline")

return fmt.Errorf("could not set read deadline")

SammyOina · 2024-10-15T07:35:53Z

pkg/tls_extensions/atlsLIstener.go

+		return 0, 0
+	}
+
+	d := time.Until(t)


Suggested change

d := time.Until(t)

d := time.Until(t)

if d<=0 {

return 0,0

}

SammyOina · 2024-10-15T07:47:22Z

pkg/tls_extensions/atls_listener.c

+                free(conn);
+                close(conn->socket_fd);


Suggested change

free(conn);

close(conn->socket_fd);

close(conn->socket_fd);

free(conn);

SammyOina · 2024-10-15T07:48:03Z

pkg/tls_extensions/atls_listener.c

+        free(conn);
+        SSL_free(conn->ssl);


Suggested change

free(conn);

SSL_free(conn->ssl);

SSL_free(conn->ssl);

free(conn);

SammyOina · 2024-10-23T11:40:01Z

pkg/attestation/attestation.go

+	return nil
+}
+
+func FetchAttestation(reportDataSlice []byte) []byte {


propagate error

SammyOina · 2024-10-23T11:41:22Z

pkg/attestation/attestation.go

+func VerifyAttestationReportTLS(attestationBytes []byte, reportData []byte) int {
+	logger.Init("", false, false, io.Discard)
+
+	AttConfigurationSEVSNP.SNPPolicy.ReportData = reportData[:]


SNPPolicy could be nil, this could panic

SammyOina · 2024-10-23T11:42:07Z

pkg/attestation/attestation.go

try combining with exisiting code in cli, to avoid repetition

SammyOina · 2024-10-23T11:43:02Z

pkg/attestation/attestation.go

+	RootOfTrust *check.RootOfTrust `json:"root_of_trust,omitempty"`
+}
+
+func VerifyAttestationReportTLS(attestationBytes []byte, reportData []byte) int {


the idiomatic golang way is to return an error

SammyOina · 2024-10-23T11:45:54Z

pkg/attestation/attestation.go

+)
+
+var (
+	AttConfigurationSEVSNP = AttestationConfiguration{}


avoid package level variable, this is not thread safe

SammyOina · 2024-10-23T11:48:19Z

pkg/tls_extensions/atlsLIstener.go

+	defer c.fdReadMutex.Unlock()
+
+	if c.tlsConn == nil {
+		return 0, fmt.Errorf("tls connection is NULL")


Suggested change

return 0, fmt.Errorf("tls connection is NULL")

return 0, fmt.Errorf("tls connection is nil")

also declare errors using vars

SammyOina · 2024-10-23T11:50:29Z

pkg/tls_extensions/atlsLIstener.go

+func (l *ATLSServerListener) Accept() (net.Conn, error) {
+	conn := C.tls_server_accept(l.tlsListener)
+	if conn == nil {
+		return &ATLSConn{tlsConn: nil}, nil


if conn is nil, return an error

SammyOina · 2024-10-23T11:51:01Z

pkg/tls_extensions/atlsLIstener.go

+	defer c.fdWriteMutex.Unlock()
+
+	if c.tlsConn == nil {
+		return 0, fmt.Errorf("tls connection is NULL")


Suggested change

return 0, fmt.Errorf("tls connection is NULL")

return 0, fmt.Errorf("tls connection is nil")

SammyOina · 2024-10-23T11:53:17Z

pkg/tls_extensions/atls_extensions.c

+        }
+
+        if (ext_data != NULL) {
+            if (RAND_bytes(ext_data->er.data, CLIENT_RANDOM_SIZE) < 0) {


probably != 1 rather than <0

SammyOina · 2024-10-23T11:54:24Z

pkg/tls_extensions/atls_extensions.c

+                return 0;
+            }
+
+            attestation_report = triggerFetchAttestationCallback(ext_data->fetch_attestation_handler, hash);


probably requires some nil check before assignment

add draft tls extension

4c4892f

drasko requested changes Oct 8, 2024

View reviewed changes

danko-miladinovic added 2 commits October 11, 2024 15:34

add client support for ipv6

a46890e

remove vscode

ae31ba1

SammyOina requested changes Oct 14, 2024

View reviewed changes

SammyOina requested changes Oct 15, 2024

View reviewed changes

danko-miladinovic added 2 commits October 18, 2024 15:44

add evidence request server payload

394993c

clean up the code

4dc1c8e

dborovcanin marked this pull request as ready for review October 23, 2024 07:30

SammyOina requested changes Oct 23, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

COCOS-192 - Attested TLS #279

COCOS-192 - Attested TLS #279

danko-miladinovic commented Oct 8, 2024

drasko Oct 8, 2024

drasko Oct 8, 2024

SammyOina Oct 14, 2024

SammyOina Oct 14, 2024

SammyOina Oct 14, 2024

SammyOina Oct 14, 2024

SammyOina Oct 14, 2024

SammyOina Oct 15, 2024

SammyOina Oct 15, 2024

SammyOina Oct 15, 2024

SammyOina Oct 15, 2024

SammyOina Oct 15, 2024

SammyOina Oct 15, 2024

SammyOina Oct 15, 2024

SammyOina Oct 15, 2024

SammyOina Oct 15, 2024

SammyOina Oct 15, 2024

SammyOina Oct 23, 2024

SammyOina Oct 23, 2024

SammyOina Oct 23, 2024

SammyOina Oct 23, 2024

SammyOina Oct 23, 2024

SammyOina Oct 23, 2024

SammyOina Oct 23, 2024

SammyOina Oct 23, 2024

SammyOina Oct 23, 2024

SammyOina Oct 23, 2024

	#define SSL_ERROR_WANT_READ_WRITE 2
	#define SSL_ERROR_WANT_WRITE 2
	#define SSL_ERROR_WANT_READ 2

	typedef struct tls_server_c
	typedef struct tls_server_connection

	// fprintf(stderr, "evidence_request_ext_free_cb called\n");
	free((void *)out);
	// fprintf(stderr, "evidence_request_ext_free_cb called\n");

	// 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");

+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;
+}

	- 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

	return fmt.Errorf("could not set deadline")
	return fmt.Errorf("could not set deadline, socket timeout failed")

	return fmt.Errorf("could not set write deadline")
	return fmt.Errorf("could not set read deadline")

	return 0, fmt.Errorf("tls connection is NULL")
	return 0, fmt.Errorf("tls connection is nil")

COCOS-192 - Attested TLS #279

Are you sure you want to change the base?

COCOS-192 - Attested TLS #279

Conversation

danko-miladinovic commented Oct 8, 2024

What type of PR is this?

What does this do?

Which issue(s) does this PR fix/relate to?

Have you included tests for your changes?

Did you document any new/modified feature?

Notes

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment