Skip to content
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.

Sign up for GitHub

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

Fix for setting wrong version in CSRs. #8136

Merged
merged 4 commits into from
Nov 15, 2024
Merged

Fix for setting wrong version in CSRs. #8136

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2254ec8
Fix for setting wrong version in CSRs.
anhu Oct 31, 2024
d959d9d
cast 1 to long
anhu Nov 1, 2024
0508151
Quick fix
anhu Nov 6, 2024
b1ccbbc
Addressing review comments from dgarske
anhu Nov 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 23 additions & 2 deletions src/x509.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7067,8 +7067,10 @@ int wolfSSL_X509_REQ_print(WOLFSSL_BIO* bio, WOLFSSL_X509* x509)
return WOLFSSL_FAILURE;
}

/* print version of cert */
if (X509PrintVersion(bio, wolfSSL_X509_version(x509), 8)
/* print version of cert. Note that we increment by 1 because for REQs,
* the value stored in x509->version is the actual value of the field; not
* the version. */
if (X509PrintVersion(bio, (int)wolfSSL_X509_REQ_get_version(x509) + 1, 8)
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this really the right way/place to adjust the version? The wolfSSL_X509_get_version and wolfSSL_X509_set_version do it differently.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I think the reason this is correct is because this is specific for REQs; not certificates.

Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn’t the +1 be inside the get function? Or is this the same way openssl does it?

Copy link
Member Author

Choose a reason for hiding this comment

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

If the +1 was in the wolfSSL_X509_REQ_get_version() then it would return the wrong value. This behaviour matches OpenSSL.

!= WOLFSSL_SUCCESS) {
return WOLFSSL_FAILURE;
}
Expand Down Expand Up @@ -14840,6 +14842,25 @@ void wolfSSL_X509_REQ_free(WOLFSSL_X509* req)
wolfSSL_X509_free(req);
}

int wolfSSL_X509_REQ_set_version(WOLFSSL_X509 *x, long version)
Copy link
Contributor

Choose a reason for hiding this comment

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

See wolfSSL_X509_set_version -> x509->version = (int) v + 1;

{
WOLFSSL_ENTER("wolfSSL_X509_REQ_set_version");
if ((x == NULL) || (version < 0) || (version >= INT_MAX)) {
return WOLFSSL_FAILURE;
}
x->version = (int)version;
return WOLFSSL_SUCCESS;
}

long wolfSSL_X509_REQ_get_version(const WOLFSSL_X509 *req)
{
WOLFSSL_ENTER("wolfSSL_X509_REQ_get_version");
if (req == NULL) {
return 0; /* invalid arg */
}
return (long)req->version;
}

int wolfSSL_X509_REQ_sign(WOLFSSL_X509 *req, WOLFSSL_EVP_PKEY *pkey,
const WOLFSSL_EVP_MD *md)
{
Expand Down
3 changes: 2 additions & 1 deletion wolfssl/openssl/ssl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -509,7 +509,8 @@ typedef STACK_OF(ACCESS_DESCRIPTION) AUTHORITY_INFO_ACCESS;
#define X509_set1_notBefore wolfSSL_X509_set1_notBefore
#define X509_set_serialNumber wolfSSL_X509_set_serialNumber
#define X509_set_version wolfSSL_X509_set_version
#define X509_REQ_set_version wolfSSL_X509_set_version
#define X509_REQ_set_version wolfSSL_X509_REQ_set_version
#define X509_REQ_get_version wolfSSL_X509_REQ_get_version
#define X509_sign wolfSSL_X509_sign
#define X509_sign_ctx wolfSSL_X509_sign_ctx
#define X509_print wolfSSL_X509_print
Expand Down
2 changes: 2 additions & 0 deletions wolfssl/ssl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4815,6 +4815,8 @@ WOLFSSL_API int wolfSSL_PEM_write_bio_X509(WOLFSSL_BIO *bp, WOLFSSL_X509 *x);
WOLFSSL_API int wolfSSL_i2d_X509_REQ(WOLFSSL_X509* req, unsigned char** out);
WOLFSSL_API WOLFSSL_X509* wolfSSL_X509_REQ_new(void);
WOLFSSL_API void wolfSSL_X509_REQ_free(WOLFSSL_X509* req);
WOLFSSL_API long wolfSSL_X509_REQ_get_version(const WOLFSSL_X509 *req);
WOLFSSL_API int wolfSSL_X509_REQ_set_version(WOLFSSL_X509 *x, long version);
WOLFSSL_API int wolfSSL_X509_REQ_sign(WOLFSSL_X509 *req, WOLFSSL_EVP_PKEY *pkey,
const WOLFSSL_EVP_MD *md);
WOLFSSL_API int wolfSSL_X509_REQ_sign_ctx(WOLFSSL_X509 *req,
Expand Down
Loading