Skip to content

Changes needed for default TLS support in zephyr kernel#7731

Merged
JacobBarthelmeh merged 3 commits intowolfSSL:masterfrom
ColtonWilley:zephyr_tls_support
Jul 11, 2024
Merged

Changes needed for default TLS support in zephyr kernel#7731
JacobBarthelmeh merged 3 commits intowolfSSL:masterfrom
ColtonWilley:zephyr_tls_support

Conversation

@ColtonWilley
Copy link
Copy Markdown
Contributor

Description

Changes needed for default wolfSSL TLS support in zephyr kernel.

Checklist

  • added tests

@ColtonWilley
Copy link
Copy Markdown
Contributor Author

Retest this please

Copy link
Copy Markdown
Member

@dgarske dgarske left a comment

Choose a reason for hiding this comment

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

Please plan to document WOLFSSL_GET_CIPHER_BYTES in the documentation repo.

Comment thread src/ssl.c Outdated
}

#ifdef WOLFSSL_GET_CIPHER_BYTES
int wolfSSL_get_cipher_list_bytes(byte* buf, int *len)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is there other places in the code that use this code pattern we could change to calling this function? Then we could make this function always available and avoid a new WOLFSSL_GET_CIPHER_BYTES macro.

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.

I did not see any existing code patterns that could be changed to use this. In general it seems this is a somewhat less used way to interact with the ciphersuites, most of the other functions work with string representation of the names. This is just a useful format when converting it back to mbedtls style ciphersuite representation. My thought process here is that we havent needed this function until now so I didnt see a reason to inflate the default code size any more, even for a tiny function like this.

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.

I think this could be wolfSSL_get_cipher_list() then wolfSSL_get_cipher_suite_from_name() in the application? Or a wrapper around those two calls in wolfSSL. Would also rather avoid the WOLFSSL_GET_CIPHER_BYTES guard. If this function is added in, would like to just have it either always be available as a new public API or have it guarded by an existing macro.

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.

I am ok with no flag guard and available by default. @JacobBarthelmeh I am confused what you mean by "I think this could be wolfSSL_get_cipher_list() then wolfSSL_get_cipher_suite_from_name() in the application?". Just letting you know my use case is for converting our cipherlist to the mbedtls style list, which is an array of ints. While I probably could write this code to translate it to a string and back it would be way more work than actually needed seeing as they are already stored internally in a byte representation.

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.

Also feel it should be noted that we already have a guard for the setter, its guarded under WOLFSSL_SET_CIPHER_BYTES || OPENSSL_EXTRA

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.

OK, I have rewritten my zephyr code to use existing cipher list functions and modified this PR accordingly.

Comment thread src/ssl.c Outdated
}

#ifdef WOLFSSL_GET_CIPHER_BYTES
int wolfSSL_get_cipher_list_bytes(byte* buf, int *len)
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.

I think this could be wolfSSL_get_cipher_list() then wolfSSL_get_cipher_suite_from_name() in the application? Or a wrapper around those two calls in wolfSSL. Would also rather avoid the WOLFSSL_GET_CIPHER_BYTES guard. If this function is added in, would like to just have it either always be available as a new public API or have it guarded by an existing macro.

Comment thread zephyr/user_settings.h
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants