-
Notifications
You must be signed in to change notification settings - Fork 5
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
fix: KeyAttestation#key failing when using libopenssl v1.0.2 #5
fix: KeyAttestation#key failing when using libopenssl v1.0.2 #5
Conversation
context "when is not valid" do | ||
before do | ||
expect(key_attestation).to receive(:valid?).and_return(false) | ||
end | ||
|
||
it 'returns nil' do | ||
expect(key_attestation.key).to be nil | ||
end | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wanted to avoid re-writing all the cases in which KeyAttestation
could be invalid, since we doing that above, in order to avoid duplicated code
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good 👍
f21d23e
to
efc4a77
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excellent 👏
context "when is not valid" do | ||
before do | ||
expect(key_attestation).to receive(:valid?).and_return(false) | ||
end | ||
|
||
it 'returns nil' do | ||
expect(key_attestation.key).to be nil | ||
end | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good 👍
6d240c3
to
07a6fe9
Compare
d37bb2c
to
750cae6
Compare
e3c9a9e
to
28c484e
Compare
- env: RB=ruby-head LIBSSL=1.0 | ||
- env: RB=ruby-head LIBSSL=1.1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After trying hundred times, I finally make it so that Travis ignores all the builds that have the environment variable ruby-head
.
It's important to note that Travis won't work if you don't define the entry in the allow_failures
matrix exactly as you define it above – see this stackoverflow post. For example, this wouldn't work for me:
- env:
- RB=ruby-head LIBSSL=1.0
- RB=ruby-head LIBSSL=1.1
...
matrix:
fast_finish: true
allow_failures:
- env: RB=ruby-head
In addition, Travis would not recognize your allowed failures on multiple environment variables entries if you define them as entries inside the same key, for example, this would also not work for me:
- env:
- RB=ruby-head LIBSSL=1.0
- RB=ruby-head LIBSSL=1.1
...
matrix:
fast_finish: true
allow_failures:
- env:
- RB=ruby-head LIBSSL=1.0
- RB=ruby-head LIBSSL=1.1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gotcha 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work! 💯
e645c13
to
4ce48c9
Compare
While working with OpenSSL 1.0.2, some test were failing with the error: 'OpenSSL::PKey::RSA missing key'. This error is caused because no exponent is passed to the method OpenSSL::PKey::RSA#set_key in 'lib/tpm/t_public.rb'. Given that we only support the default exponent as for now, we can just pass it to the method and it will work.
4ce48c9
to
868c6c0
Compare
Some libssl 1.0 jobs are failing on |
I re-run the build and now the job that was failing went green but the one with ruby 2.6.6, libopenssl 1.0 and openssl default failed with the same error – seems that we have a flaky one here. |
There seems to be a race condition between the |
Well... turns out that isn't why the test are being flaky 😅 . The test are being flaky because some of them randomly have their certificates not considered trustworthy, being more specific, the test are returning false in this line. I'm still searching for the reason though. |
It seems that when checking if the certificate is valid, comparing Time.now with the attribute not_before of a certificate (that is also initiated with Time.now) is sometimes returning an error due to the difference being too short – it could be related with the comparison of floats that are close to each other. I created a PR in an attempt to solve this issue. |
Thank you! |
What
There's is bug when using
libopenssl v1.0.2
where KeyAttestation#Key blows up with:Why
The error is produced in this line, after calling
OpenSSL::PKey::RSA#set_key
.If we turn on the debug mode for OpenSSL –
OpenSSL.debug = true
, we get:It seems that the error is caused because the we are not setting the field
e
, which is the exponent.If we take a look into the OpenSSL man page for the method RSA_set0_key, we can see that it states:
I still didn't figure out why it is failing only when using
OpenSSL v1.0.2
– and not when usingv1.1.1
.How
Given that we only support the default exponent as for now, we can just pass it to the method and it will work.
How come that we only support the default exponent?
As specified in TPMv2-Part2 section
12.2.3.5
.Steps taken
TBD
ruby-head
to fail