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.

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

In OpenSSL 3, ASN1_item_verify*() can return 2 on error. #24575

Closed
botovq opened this issue Jun 6, 2024 · 1 comment
Closed

In OpenSSL 3, ASN1_item_verify*() can return 2 on error. #24575

botovq opened this issue Jun 6, 2024 · 1 comment
Labels
branch: 3.0 Merge to openssl-3.0 branch branch: 3.1 Merge to openssl-3.1 branch: 3.2 Merge to openssl-3.2 branch: 3.3 Merge to openssl-3.3 severity: important Important bugs affecting a released version triaged: bug The issue/pr is/fixes a bug

Comments

@botovq
Copy link
Contributor

botovq commented Jun 6, 2024

Introduced in #10942 as part of extracting ASN1_item_verify_ctx() from ASN1_item_verify() when a ret = -1; was deleted after the goto err here:

ret = pkey->ameth->item_verify(ctx, it, data, alg, signature, pkey);
/*
* Return values meaning:
* <=0: error.
* 1: method does everything.
* 2: carry on as normal, method has called EVP_DigestVerifyInit()
*/
if (ret <= 0)
ERR_raise(ERR_LIB_ASN1, ERR_R_EVP_LIB);
if (ret <= 1)
goto err;
} else {

If item_verify() returns 2 (which it usually will for RSA-PSS for example) the control flow would skip here and return 2 if ASN1_item_i2d() errors:

inl = ASN1_item_i2d(data, &buf_in, it);
if (inl <= 0) {
ERR_raise(ERR_LIB_ASN1, ERR_R_INTERNAL_ERROR);
goto err;
}
if (buf_in == NULL) {
ERR_raise(ERR_LIB_ASN1, ERR_R_ASN1_LIB);
goto err;
}

@botovq botovq added the issue: bug report The issue was opened to report a bug label Jun 6, 2024
@t8m t8m added triaged: bug The issue/pr is/fixes a bug severity: important Important bugs affecting a released version branch: 3.0 Merge to openssl-3.0 branch branch: 3.1 Merge to openssl-3.1 branch: 3.2 Merge to openssl-3.2 branch: 3.3 Merge to openssl-3.3 and removed issue: bug report The issue was opened to report a bug labels Jun 6, 2024
t8m added a commit to t8m/openssl that referenced this issue Jun 6, 2024
@t8m
Copy link
Member

t8m commented Jun 6, 2024

Fix in #24576

t8m added a commit to t8m/openssl that referenced this issue Jun 17, 2024
This is a test for openssl#24575
Original idea by Theo Buehler.
t8m added a commit to t8m/openssl that referenced this issue Jun 17, 2024
This is a test for openssl#24575
Original idea by Theo Buehler.
openssl-machine pushed a commit that referenced this issue Jun 21, 2024
This is a test for #24575
Original idea by Theo Buehler.

Reviewed-by: Tom Cosgrove <[email protected]>
Reviewed-by: Neil Horman <[email protected]>
Reviewed-by: Dmitry Belyavskiy <[email protected]>
(Merged from #24576)
openssl-machine pushed a commit that referenced this issue Jun 21, 2024
Fixes #24575

Reviewed-by: Tom Cosgrove <[email protected]>
Reviewed-by: Neil Horman <[email protected]>
Reviewed-by: Dmitry Belyavskiy <[email protected]>
(Merged from #24576)

(cherry picked from commit 8d380f8)
openssl-machine pushed a commit that referenced this issue Jun 21, 2024
This is a test for #24575
Original idea by Theo Buehler.

Reviewed-by: Tom Cosgrove <[email protected]>
Reviewed-by: Neil Horman <[email protected]>
Reviewed-by: Dmitry Belyavskiy <[email protected]>
(Merged from #24576)

(cherry picked from commit 2f0b497)
openssl-machine pushed a commit that referenced this issue Jun 21, 2024
Fixes #24575

Reviewed-by: Tom Cosgrove <[email protected]>
Reviewed-by: Neil Horman <[email protected]>
Reviewed-by: Dmitry Belyavskiy <[email protected]>
(Merged from #24576)

(cherry picked from commit 8d380f8)
openssl-machine pushed a commit that referenced this issue Jun 21, 2024
This is a test for #24575
Original idea by Theo Buehler.

Reviewed-by: Tom Cosgrove <[email protected]>
Reviewed-by: Neil Horman <[email protected]>
Reviewed-by: Dmitry Belyavskiy <[email protected]>
(Merged from #24576)

(cherry picked from commit 2f0b497)
openssl-machine pushed a commit that referenced this issue Jun 21, 2024
Fixes #24575

Reviewed-by: Tom Cosgrove <[email protected]>
Reviewed-by: Neil Horman <[email protected]>
Reviewed-by: Dmitry Belyavskiy <[email protected]>
(Merged from #24576)

(cherry picked from commit 8d380f8)
openssl-machine pushed a commit that referenced this issue Jun 21, 2024
Fixes #24575

Reviewed-by: Tom Cosgrove <[email protected]>
Reviewed-by: Neil Horman <[email protected]>
Reviewed-by: Dmitry Belyavskiy <[email protected]>
(Merged from #24576)

(cherry picked from commit 8d380f8)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
branch: 3.0 Merge to openssl-3.0 branch branch: 3.1 Merge to openssl-3.1 branch: 3.2 Merge to openssl-3.2 branch: 3.3 Merge to openssl-3.3 severity: important Important bugs affecting a released version triaged: bug The issue/pr is/fixes a bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants