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

Jump to bottom

OpenSSL 3: x25519 a decode from and then encode to a pem file corrupts the key if fips base provider is used #21493

Closed
junaruga opened this issue Jul 19, 2023 · 2 comments
Closed

OpenSSL 3: x25519 a decode from and then encode to a pem file corrupts the key if fips base provider is used #21493

junaruga opened this issue Jul 19, 2023 · 2 comments
Assignees
@t8m
Labels
branch: master Merge to master branch severity: regression The issue/pr is a regression from previous released version triaged: bug The issue/pr is/fixes a bug

Comments

@junaruga
Copy link

junaruga commented Jul 19, 2023 •
edited
Loading

This issue ticket is for the x25519 crypto version of another issue with ed25519 crypto: #20758 (comment). As the issue with the ed25519 has a difficulty to reproduce with OpenSSL master branch, and includes another issue. So, I created this issue ticket not to mix the 2 issues. And as we can reproduce this issue with the x25519 crypto file on the master branch (OpenSSL 3.2.0.dev), it's more convenient to debug and fix it.

I checked this issue on the current latest master branch 8c34367e434c6b9555f21cc4fc77a18d6ef84a85. The x25519 crypto is approved in the FIPS case in the master branch as a reference.

$ grep PROV_NAMES_X25519 providers/fips/fipsprov.c
    { PROV_NAMES_X25519, FIPS_DEFAULT_PROPERTIES, ossl_x25519_keyexch_functions },
    { PROV_NAMES_X25519, FIPS_DEFAULT_PROPERTIES, ossl_x25519_keymgmt_functions,

Summary

When reading a x25519 crypto public key pem file, then encoding and decoding it in FIPS case with "base" and "fips" providers, the decoded text is different from the original one in only FIPS case.

Reproducing steps

I prepared the reproducing program whose repository name was previously "report-openssl-fips-ed25519" and used in the #20758.

Prepare the x25519 public key pem file.

$ cat x25519_pub.pem
-----BEGIN PUBLIC KEY-----
MCowBQYDK2VuAyEA3p7bfXt9wbTTW2HC7OQ1Nz DQ8hbeGdNrfx FG IK08=
-----END PUBLIC KEY-----

Compile the reproducing program with the targeting OpenSSL.

$ gcc \
  -I /home/jaruga/.local/openssl-3.2.0.dev-fips-debug-8c34367e43/include \
  -L /home/jaruga/.local/openssl-3.2.0.dev-fips-debug-8c34367e43/lib \
  -o reproducer reproducer.c -lcrypto

Non-FIPS case

The decoded text is the same with the original PEM file. This is ok.

$ OPENSSL_CONF_INCLUDE=/home/jaruga/.local/openssl-3.2.0.dev-fips-debug-8c34367e43/ssl \
  OPENSSL_MODULES=/home/jaruga/.local/openssl-3.2.0.dev-fips-debug-8c34367e43/lib/ossl-modules \
  LD_LIBRARY_PATH=/home/jaruga/.local/openssl-3.2.0.dev-fips-debug-8c34367e43/lib \
  ./reproducer x25519_pub.pem
[DEBUG] Loaded providers:
  default
[DEBUG] FIPS mode enabled: 0
[DEBUG] data_size: 113
[DEBUG] OSSL_DECODER_from_bio PEM 2 failed.
[DEBUG] Got a pkey! 0x5707a0
[DEBUG] It's held by the provider default
[DEBUG] ossl_membio2str buf->data:
-----BEGIN PUBLIC KEY-----
MCowBQYDK2VuAyEA3p7bfXt9wbTTW2HC7OQ1Nz DQ8hbeGdNrfx FG IK08=
-----END PUBLIC KEY-----

FIPS case

The decode text is different from the original PEM file. This is not ok.

$ OPENSSL_CONF=$(pwd)/openssl_fips.cnf \
  OPENSSL_CONF_INCLUDE=/home/jaruga/.local/openssl-3.2.0.dev-fips-debug-8c34367e43/ssl \
  OPENSSL_MODULES=/home/jaruga/.local/openssl-3.2.0.dev-fips-debug-8c34367e43/lib/ossl-modules \
  LD_LIBRARY_PATH=/home/jaruga/.local/openssl-3.2.0.dev-fips-debug-8c34367e43/lib \
  ./reproducer x25519_pub.pem
[DEBUG] Loaded providers:
  fips
  base
[DEBUG] FIPS mode enabled: 1
[DEBUG] data_size: 113
[DEBUG] OSSL_DECODER_from_bio PEM 2 failed.
[DEBUG] Got a pkey! 0xd7fde0
[DEBUG] It's held by the provider fips
[DEBUG] ossl_membio2str buf->data:
-----BEGIN PUBLIC KEY-----
MCowBQYDK2VuAyEAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=
-----END PUBLIC KEY----
@junaruga junaruga added the issue: bug report The issue was opened to report a bug label Jul 19, 2023
This was referenced Jul 19, 2023
Closed
Open
@paulidale paulidale added branch: master Merge to master branch triaged: bug The issue/pr is/fixes a bug severity: regression The issue/pr is a regression from previous released version and removed issue: bug report The issue was opened to report a bug labels Jul 19, 2023
@t8m t8m mentioned this issue Jul 21, 2023
Closed
t8m added a commit to t8m/openssl that referenced this issue Jul 21, 2023
@t8m
When exporting/importing decoded keys do not use 0 as selection
dc242ef 
When decoding 0 as the selection means to decode anything
you get.

However when exporting and then importing the key data 0 as
selection is not meaningful.
So we set it to OSSL_KEYMGMT_SELECT_ALL to make the export/import
function export/import everything that we have decoded.

Fixes openssl#21493
@t8m
Copy link
Member

t8m commented Jul 21, 2023

@junaruga please try the fixes in #21519

@t8m t8m self-assigned this Jul 21, 2023
@junaruga
Copy link
Author

junaruga commented Jul 21, 2023 •
edited
Loading

Wow, thank you for the PR! I will try the fixes, and will let you know the result here!

openssl-machine pushed a commit that referenced this issue Aug 4, 2023
@t8m @tmshort
When exporting/importing decoded keys do not use 0 as selection
137ba05 
When decoding 0 as the selection means to decode anything
you get.

However when exporting and then importing the key data 0 as
selection is not meaningful.
So we set it to OSSL_KEYMGMT_SELECT_ALL to make the export/import
function export/import everything that we have decoded.

Fixes #21493

Reviewed-by: Matt Caswell <[email protected]>
Reviewed-by: Paul Dale <[email protected]>
Reviewed-by: Todd Short <[email protected]>
(Merged from #21519)

(cherry picked from commit 2acb0d3)
openssl-machine pushed a commit that referenced this issue Aug 4, 2023
@t8m @tmshort
When exporting/importing decoded keys do not use 0 as selection
52080c8 
When decoding 0 as the selection means to decode anything
you get.

However when exporting and then importing the key data 0 as
selection is not meaningful.
So we set it to OSSL_KEYMGMT_SELECT_ALL to make the export/import
function export/import everything that we have decoded.

Fixes #21493

Reviewed-by: Matt Caswell <[email protected]>
Reviewed-by: Paul Dale <[email protected]>
Reviewed-by: Todd Short <[email protected]>
(Merged from #21519)

(cherry picked from commit 2acb0d3)
(cherry picked from commit 137ba05)
This was referenced Aug 8, 2023
Merged
Merged
xl32 pushed a commit to xl32/openssl that referenced this issue Sep 29, 2023
@t8m @xl32
When exporting/importing decoded keys do not use 0 as selection
72ec2b1 
When decoding 0 as the selection means to decode anything
you get.

However when exporting and then importing the key data 0 as
selection is not meaningful.
So we set it to OSSL_KEYMGMT_SELECT_ALL to make the export/import
function export/import everything that we have decoded.

Fixes openssl#21493

Reviewed-by: Matt Caswell <[email protected]>
Reviewed-by: Paul Dale <[email protected]>
Reviewed-by: Todd Short <[email protected]>
(Merged from openssl#21519)

(cherry picked from commit 2acb0d3)
(cherry picked from commit 137ba05)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@t8m t8m

Labels
branch: master Merge to master branch severity: regression The issue/pr is a regression from previous released version triaged: bug The issue/pr is/fixes a bug
Projects
Project Board
Archived in project
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Decoder key export fixes t8m/openssl
3 participants
@junaruga @t8m @paulidale