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

key: extract error on read fail #9

Merged
merged 2 commits into from
Aug 1, 2023

Conversation

DifferentialOrange
Copy link
Member

@DifferentialOrange DifferentialOrange commented Jul 27, 2023

Before this patch, read fail had generated a Go error to return it to a user, even though read errors (for example, caused by bad input or wrong password) were placed in error queue. Since goroutines are not binded to a thread and error queue is per thread, sometimes it had resulted in valid connections failing with error. This patch fixes the issue.

@DifferentialOrange DifferentialOrange force-pushed the DifferentialOrange/flaky-bad-decrypt branch 2 times, most recently from 8f572b8 to 1e5db74 Compare August 1, 2023 10:31
@DifferentialOrange DifferentialOrange changed the base branch from v0.0.7-24-g0fadeb4-tarantool to master August 1, 2023 10:32
@DifferentialOrange DifferentialOrange changed the title test: provide bad decrypt error reproducer key: extract error on read fail Aug 1, 2023
Before this patch, read fail had generated a Go error to return it to a
user, even though read errors (for example, caused by bad input or wrong
password) were placed in error queue. Since goroutines are not binded to
a thread and error queue is per thread, sometimes it had resulted in
valid connections failing with error. This patch fixes the issue.
@DifferentialOrange DifferentialOrange force-pushed the DifferentialOrange/flaky-bad-decrypt branch from 1e5db74 to bfc58d2 Compare August 1, 2023 10:40
@DifferentialOrange DifferentialOrange marked this pull request as ready for review August 1, 2023 10:40
@DifferentialOrange
Copy link
Member Author

It seems that Windows tests are flaky and I don't see any reasons that my updates are the reason.

Comment on lines +306 to 311
runtime.LockOSThread()
defer runtime.UnlockOSThread()
key := C.PEM_read_bio_PrivateKey(bio, nil, nil, nil)
if key == nil {
return nil, errors.New("failed reading private key")
return nil, fmt.Errorf("failed reading private key: %w", errorFromErrorQueue())
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

We could extract it into a private function. Up to you.

Copy link
Collaborator

@oleg-jukovec oleg-jukovec left a comment

Choose a reason for hiding this comment

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

Thank you for the patch!

@DifferentialOrange DifferentialOrange merged commit b452431 into master Aug 1, 2023
20 checks passed
DifferentialOrange added a commit to tarantool/go-tarantool that referenced this pull request Aug 1, 2023
Support `ssl_password` and `ssl_password_file` options in SslOpts.
Tarantool EE supports SSL passwords and password files since 2.11.0 [1].
Since it is possible to use corresponding non-encrypted key, cert and CA
on server, tests works fine even for Tarantool EE 2.10.0.

Same as in Tarantool, we try `SslOpts.Password`, then each line in
`SslOpts.PasswordFile`. If all of the above fail, we re-raise errors.

If the key is encrypted and password is not provided,
`openssl.LoadPrivateKeyFromPEM(keyBytes)` asks to enter PEM pass phrase
interactively. On the other hand,
`openssl.LoadPrivateKeyFromPEMWithPassword(keyBytes, password)` works
fine for non-encrypted key with any password, including empty string.
If the key is encrypted, we fast fail with password error instead of
requesting the pass phrase interactively.

The patch also bumps go-openssl since latest patch fixes flaky
tests [2].

The patch is based on a similar patch for tarantool-python [3].

1. tarantool/tarantool-ee#22
2. tarantool/go-openssl#9
3. tarantool/tarantool-python#274
DifferentialOrange added a commit to tarantool/go-tarantool that referenced this pull request Aug 1, 2023
Support `ssl_password` and `ssl_password_file` options in SslOpts.
Tarantool EE supports SSL passwords and password files since 2.11.0 [1].
Since it is possible to use corresponding non-encrypted key, cert and CA
on server, tests works fine even for Tarantool EE 2.10.0.

Same as in Tarantool, we try `SslOpts.Password`, then each line in
`SslOpts.PasswordFile`. If all of the above fail, we re-raise errors.

If the key is encrypted and password is not provided,
`openssl.LoadPrivateKeyFromPEM(keyBytes)` asks to enter PEM pass phrase
interactively. On the other hand,
`openssl.LoadPrivateKeyFromPEMWithPassword(keyBytes, password)` works
fine for non-encrypted key with any password, including empty string.
If the key is encrypted, we fast fail with password error instead of
requesting the pass phrase interactively.

The patch also bumps go-openssl since latest patch fixes flaky
tests [2].

The patch is based on a similar patch for tarantool-python [3].

1. tarantool/tarantool-ee#22
2. tarantool/go-openssl#9
3. tarantool/tarantool-python#274
DifferentialOrange added a commit to tarantool/go-tarantool that referenced this pull request Aug 1, 2023
Support `ssl_password` and `ssl_password_file` options in SslOpts.
Tarantool EE supports SSL passwords and password files since 2.11.0 [1].
Since it is possible to use corresponding non-encrypted key, cert and CA
on server, tests works fine even for Tarantool EE 2.10.0.

Same as in Tarantool, we try `SslOpts.Password`, then each line in
`SslOpts.PasswordFile`. If all of the above fail, we re-raise errors.

If the key is encrypted and password is not provided,
`openssl.LoadPrivateKeyFromPEM(keyBytes)` asks to enter PEM pass phrase
interactively. On the other hand,
`openssl.LoadPrivateKeyFromPEMWithPassword(keyBytes, password)` works
fine for non-encrypted key with any password, including empty string.
If the key is encrypted, we fast fail with password error instead of
requesting the pass phrase interactively.

The patch also bumps go-openssl since latest patch fixes flaky
tests [2].

The patch is based on a similar patch for tarantool-python [3].

1. tarantool/tarantool-ee#22
2. tarantool/go-openssl#9
3. tarantool/tarantool-python#274
DifferentialOrange added a commit to tarantool/go-tarantool that referenced this pull request Aug 3, 2023
Support `ssl_password` and `ssl_password_file` options in SslOpts.
Tarantool EE supports SSL passwords and password files since 2.11.0 [1].
Since it is possible to use corresponding non-encrypted key, cert and CA
on server, tests works fine even for Tarantool EE 2.10.0.

Same as in Tarantool, we try `SslOpts.Password`, then each line in
`SslOpts.PasswordFile`. If all of the above fail, we re-raise errors.

If the key is encrypted and password is not provided,
`openssl.LoadPrivateKeyFromPEM(keyBytes)` asks to enter PEM pass phrase
interactively. On the other hand,
`openssl.LoadPrivateKeyFromPEMWithPassword(keyBytes, password)` works
fine for non-encrypted key with any password, including empty string.
If the key is encrypted, we fast fail with password error instead of
requesting the pass phrase interactively.

The patch also bumps go-openssl since latest patch fixes flaky
tests [2].

The patch is based on a similar patch for tarantool-python [3].

1. tarantool/tarantool-ee#22
2. tarantool/go-openssl#9
3. tarantool/tarantool-python#274
DifferentialOrange added a commit to tarantool/go-tarantool that referenced this pull request Aug 3, 2023
Support `ssl_password` and `ssl_password_file` options in SslOpts.
Tarantool EE supports SSL passwords and password files since 2.11.0 [1].
Since it is possible to use corresponding non-encrypted key, cert and CA
on server, tests works fine even for Tarantool EE 2.10.0.

Same as in Tarantool, we try `SslOpts.Password`, then each line in
`SslOpts.PasswordFile`. If all of the above fail, we re-raise errors.

If the key is encrypted and password is not provided,
`openssl.LoadPrivateKeyFromPEM(keyBytes)` asks to enter PEM pass phrase
interactively. On the other hand,
`openssl.LoadPrivateKeyFromPEMWithPassword(keyBytes, password)` works
fine for non-encrypted key with any password, including empty string.
If the key is encrypted, we fast fail with password error instead of
requesting the pass phrase interactively.

The patch also bumps go-openssl since latest patch fixes flaky
tests [2].

The patch is based on a similar patch for tarantool-python [3].

1. tarantool/tarantool-ee#22
2. tarantool/go-openssl#9
3. tarantool/tarantool-python#274
DifferentialOrange added a commit to tarantool/go-tarantool that referenced this pull request Aug 3, 2023
Support `ssl_password` and `ssl_password_file` options in SslOpts.
Tarantool EE supports SSL passwords and password files since 2.11.0 [1].
Since it is possible to use corresponding non-encrypted key, cert and CA
on server, tests works fine even for Tarantool EE 2.10.0.

Same as in Tarantool, we try `SslOpts.Password`, then each line in
`SslOpts.PasswordFile`. If all of the above fail, we re-raise errors.

If the key is encrypted and password is not provided,
`openssl.LoadPrivateKeyFromPEM(keyBytes)` asks to enter PEM pass phrase
interactively. On the other hand,
`openssl.LoadPrivateKeyFromPEMWithPassword(keyBytes, password)` works
fine for non-encrypted key with any password, including empty string.
If the key is encrypted, we fast fail with password error instead of
requesting the pass phrase interactively.

The patch also bumps go-openssl since latest patch fixes flaky
tests [2].

The patch is based on a similar patch for tarantool-python [3].

1. tarantool/tarantool-ee#22
2. tarantool/go-openssl#9
3. tarantool/tarantool-python#274
DifferentialOrange added a commit to tarantool/go-tarantool that referenced this pull request Aug 3, 2023
Support `ssl_password` and `ssl_password_file` options in SslOpts.
Tarantool EE supports SSL passwords and password files since 2.11.0 [1].
Since it is possible to use corresponding non-encrypted key, cert and CA
on server, tests works fine even for Tarantool EE 2.10.0.

Same as in Tarantool, we try `SslOpts.Password`, then each line in
`SslOpts.PasswordFile`. If all of the above fail, we re-raise errors.

If the key is encrypted and password is not provided,
`openssl.LoadPrivateKeyFromPEM(keyBytes)` asks to enter PEM pass phrase
interactively. On the other hand,
`openssl.LoadPrivateKeyFromPEMWithPassword(keyBytes, password)` works
fine for non-encrypted key with any password, including empty string.
If the key is encrypted, we fast fail with password error instead of
requesting the pass phrase interactively.

The patch also bumps go-openssl since latest patch fixes flaky
tests [2].

The patch is based on a similar patch for tarantool-python [3].

1. tarantool/tarantool-ee#22
2. tarantool/go-openssl#9
3. tarantool/tarantool-python#274
oleg-jukovec pushed a commit to tarantool/go-tarantool that referenced this pull request Aug 3, 2023
Support `ssl_password` and `ssl_password_file` options in SslOpts.
Tarantool EE supports SSL passwords and password files since 2.11.0 [1].
Since it is possible to use corresponding non-encrypted key, cert and CA
on server, tests works fine even for Tarantool EE 2.10.0.

Same as in Tarantool, we try `SslOpts.Password`, then each line in
`SslOpts.PasswordFile`. If all of the above fail, we re-raise errors.

If the key is encrypted and password is not provided,
`openssl.LoadPrivateKeyFromPEM(keyBytes)` asks to enter PEM pass phrase
interactively. On the other hand,
`openssl.LoadPrivateKeyFromPEMWithPassword(keyBytes, password)` works
fine for non-encrypted key with any password, including empty string.
If the key is encrypted, we fast fail with password error instead of
requesting the pass phrase interactively.

The patch also bumps go-openssl since latest patch fixes flaky
tests [2].

The patch is based on a similar patch for tarantool-python [3].

1. tarantool/tarantool-ee#22
2. tarantool/go-openssl#9
3. tarantool/tarantool-python#274
oleg-jukovec added a commit that referenced this pull request Feb 8, 2024
Overview

    The first release with a number of fixes. Since `libp2p/openssl`
    is not supported any more we need to support our version for usage
    in the Golang connector `tarantool/go-tarantool`.

New features

    DialContext function (#10).

Bugfixes

     Build by Golang 1.13 (#6).

     Build with OpenSSL < 1.1.1 (#7).

     Build on macOS as a static library (#8).

     Build on macOS with Apple M1 (#8).

     Random errors in the code caused by an invalid OpenSSL error
     handling in LoadPrivateKeyFromPEM,
     LoadPrivateKeyFromPEMWithPassword,
     LoadPrivateKeyFromDER and LoadPublicKeyFromPEM (#9).
@oleg-jukovec oleg-jukovec mentioned this pull request Feb 8, 2024
oleg-jukovec added a commit that referenced this pull request Feb 8, 2024
Overview

    The first release with a number of fixes. Since `libp2p/openssl`
    is not supported any more we need to support our version for usage
    in the Golang connector `tarantool/go-tarantool`.

    See releases of `libp2p/openssl`[1] for previous changes history.

New features

    DialContext function (#10).

Bugfixes

     Build by Golang 1.13 (#6).

     Build with OpenSSL < 1.1.1 (#7).

     Build on macOS as a static library (#8).

     Build on macOS with Apple M1 (#8).

     Random errors in the code caused by an invalid OpenSSL error
     handling in LoadPrivateKeyFromPEM,
     LoadPrivateKeyFromPEMWithPassword,
     LoadPrivateKeyFromDER and LoadPublicKeyFromPEM (#9).

1. https://github.com/libp2p/go-openssl/releases
oleg-jukovec added a commit that referenced this pull request Feb 8, 2024
Overview

    The first release with a number of fixes. Since `libp2p/openssl`
    is not supported any more we need to support our version for usage
    in the Golang connector `tarantool/go-tarantool`.

    See releases of `libp2p/openssl`[1] for previous changes history.

New features

    DialContext function (#10).

Bugfixes

     Build by Golang 1.13 (#6).

     Build with OpenSSL < 1.1.1 (#7).

     Build on macOS as a static library (#8).

     Build on macOS with Apple M1 (#8).

     Random errors in the code caused by an invalid OpenSSL error
     handling in LoadPrivateKeyFromPEM,
     LoadPrivateKeyFromPEMWithPassword,
     LoadPrivateKeyFromDER and LoadPublicKeyFromPEM (#9).

1. https://github.com/libp2p/go-openssl/releases
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.

None yet

3 participants