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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

mbedtls: ramdom can use havege if enabled. #1227

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
3 participants
@p1ng0o
Contributor

p1ng0o commented Jan 25, 2017

I didn't succeed to write a "proper" CTR-DRBG random generator. So let's begin with havege 馃憤

Anyway, should I save mbedtls_havege_state somewhere ?

@mention-bot

This comment has been minimized.

mention-bot commented Jan 25, 2017

@p1ng0o, thanks for your PR! By analyzing the history of the files in this pull request, we identified @sasq64, @bagder and @jay to be potential reviewers.

@p1ng0o p1ng0o force-pushed the p1ng0o:master branch 2 times, most recently from b957d5e to cb34e4c Jan 25, 2017

@bagder bagder added the SSL/TLS label Jan 26, 2017

mbedtls_strerror(ret, errorbuf, sizeof(errorbuf));
#endif /* MBEDTLS_ERROR_C */
failf(data, "Failed - mbedTLS: ctr_drbg_seed returned (-0x%04X) %s\n",
-ret, errorbuf);

This comment has been minimized.

@bagder

bagder Jan 26, 2017

Member

shouldn't it also return an error here?

@p1ng0o p1ng0o force-pushed the p1ng0o:master branch from cb34e4c to 4c8daee Jan 26, 2017

@bagder

This comment has been minimized.

Member

bagder commented Jan 26, 2017

should I save mbedtls_havege_state somewhere

You tell me! Should it be saved? When the 'data' pointer is provided, it could be stored in that struct.

@@ -729,6 +729,56 @@ size_t Curl_mbedtls_version(char *buffer, size_t size)
(version>>16)&0xff, (version>>8)&0xff);
}

int Curl_mbedtls_random(struct Curl_easy *data, unsigned char *entropy,
size_t length)

This comment has been minimized.

@bagder

bagder Jan 26, 2017

Member

I think this is better declared to return CURLcode rather than int.

@p1ng0o

This comment has been minimized.

Contributor

p1ng0o commented Jan 27, 2017

The question was, do I privilege cpu time (struct on data), or mem (struct on cstack).
But regarding other vtls implementation of random, and for consistency, It should be better on cstack.

@p1ng0o p1ng0o force-pushed the p1ng0o:master branch from 4c8daee to bbd1f98 Jan 27, 2017

@bagder

This comment has been minimized.

Member

bagder commented Jan 27, 2017

Yeah, this function is not used a lot nor in any high performance situation so I think that's totally fine.

@bagder bagder closed this in a90a5bc Jan 29, 2017

@bagder

This comment has been minimized.

Member

bagder commented Jan 29, 2017

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment