Comments (9)
ignisf
commented on July 25, 2024
1
Here's how another gem does it:
from sshkey.
MerlijnWajer
commented on July 25, 2024
I looked at the OpenSSL code and man page, consider this:
BN_num_bits() returns the number of significant bits in a BIGNUM, following the
same principle as BN_num_bits_word().
BN_num_bytes() is a macro.
BN_num_bytes() returns the size of a BIGNUM in bytes.
And this is what the macro is:
./include/openssl/bn.h:# define BN_num_bytes(a) ((BN_num_bits(a)+7)/8)
So - I am pretty sure the num_bytes == 32 is wrong, since it requires that the bignum has to be of size 32 bytes precisely. Note that the macro also just works on int, so it will round down in the above calculation.
from sshkey.
bensie
commented on July 25, 2024
@MerlijnWajer Do you have a suggested fix?
from sshkey.
MerlijnWajer
commented on July 25, 2024
You can remove the num_bytes == 32 check. Or move the check to before the bignum parsing. In the end, if you read 32 bytes, why bother checking if they are 'valid' bignum in the first place? Any values for the bytes should be valid, I think. (I am mostly ignorant of the inner working of OpenSSL bignum).
So:
- Check earlier, before parsing to bignum (just check if you read 32 bytes)
- Or: remove the == 32 check. (checking for == 31 or == 32 seems not that clean to me)
from sshkey.
MerlijnWajer
commented on July 25, 2024
@bensie - please let me know what you would prefer. I can make a PR with my suggested fix, or we can look at an alternative. I cannot use my GnuPG ssh auth key on Gitlab because of this issue, and I'd like to get it fixed ASAP.
from sshkey.
bensie
commented on July 25, 2024
@MerlijnWajer A PR would be most welcome! I'd pick moving the check earlier to ensure 32 bytes were read. I agree that checking for both 31 and 32 is pretty gross. Thanks for the legwork on this!
from sshkey.
MerlijnWajer
commented on July 25, 2024
I am not a ruby dev, but how does this look?
From 07da091636e0f43532597c32c2b3eacccbf1c7ef Mon Sep 17 00:00:00 2001
From: Merlijn Wajer <[email protected]>
Date: Sat, 9 Feb 2019 12:59:27 +0100
Subject: [PATCH] lib/sshkey: accept valid ed25519 keys with leading zero byte
---
lib/sshkey.rb | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/lib/sshkey.rb b/lib/sshkey.rb
index fdd1958..4f1a512 100644
--- a/lib/sshkey.rb
+++ b/lib/sshkey.rb
@@ -67,7 +67,7 @@ class SSHKey
when "ssh-rsa", "ssh-dss"
sections.size == SSH_CONVERSION[SSH_TYPES[ssh_type]].size
when "ssh-ed25519"
- sections.size == 1 && sections[0].num_bytes == 32 # https://tools.ietf.org/id/draft-bjh21-ssh-ed25519-00.html#rfc.section.4
+ sections.size == 1 # https://tools.ietf.org/id/draft-bjh21-ssh-ed25519-00.html#rfc.section.4
when "ecdsa-sha2-nistp256", "ecdsa-sha2-nistp384", "ecdsa-sha2-nistp521"
sections.size == 2 # https://tools.ietf.org/html/rfc5656#section-3.1
else
@@ -171,6 +171,12 @@ class SSHKey
raise PublicKeyError, "validation error"
end
+ if ssh_type == "ssh-ed25519"
+ unless decoded.length == 32
+ raise PublicKeyError, "validation error, ed25519 key length not OK"
+ end
+ end
+
data = []
until decoded.empty?
front = decoded.slice!(0,4)
--
2.16.4
I'll run it past a friend who does know ruby, and then submit a PR if this change looks OK to you.
from sshkey.
MerlijnWajer
commented on July 25, 2024
New version, confirmed to work:
From: Merlijn Wajer <[email protected]>
Date: Sat, 9 Feb 2019 12:59:27 +0100
Subject: [PATCH] lib/sshkey: accept valid ed25519 keys with leading zero byte
---
lib/sshkey.rb | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/lib/sshkey.rb b/lib/sshkey.rb
index fdd1958..352e755 100644
--- a/lib/sshkey.rb
+++ b/lib/sshkey.rb
@@ -67,7 +67,7 @@ class SSHKey
when "ssh-rsa", "ssh-dss"
sections.size == SSH_CONVERSION[SSH_TYPES[ssh_type]].size
when "ssh-ed25519"
- sections.size == 1 && sections[0].num_bytes == 32 # https://tools.ietf.org/id/draft-bjh21-ssh-ed25519-00.html#rfc.section.4
+ sections.size == 1 # https://tools.ietf.org/id/draft-bjh21-ssh-ed25519-00.html#rfc.section.4
when "ecdsa-sha2-nistp256", "ecdsa-sha2-nistp384", "ecdsa-sha2-nistp521"
sections.size == 2 # https://tools.ietf.org/html/rfc5656#section-3.1
else
@@ -171,16 +171,26 @@ class SSHKey
raise PublicKeyError, "validation error"
end
+ byte_count = 0
data = []
until decoded.empty?
front = decoded.slice!(0,4)
size = front.unpack("N").first
segment = decoded.slice!(0, size)
+ byte_count += segment.length
unless front.length == 4 && segment.length == size
raise PublicKeyError, "byte array too short"
end
data << OpenSSL::BN.new(segment, 2)
end
+
+
+ if ssh_type == "ssh-ed25519"
+ unless byte_count == 32
+ raise PublicKeyError, "validation error, ed25519 key length not OK"
+ end
+ end
+
return data
end
--
2.16.4
from sshkey.
bensie
commented on July 25, 2024
Closed by #37
from sshkey.
Related Issues (16)
- Validation for multiline keys HOT 7
- ECDSA keys support? HOT 4
- Generate always adding passphrase HOT 5
- Validating ECDSA keys HOT 6
- Add support for ssh certificates HOT 1
- Error in README: 'No such file or directory @ rb_sysopen - ~/.ssh/id_rsa.pub (Errno::ENOENT)'
- Add support for PPK PuTTY keys HOT 1
- Split ssh public key and comment HOT 3
- Convert SSH to RSA HOT 1
- ssh_public_key_bits returns wrong result for ecdsa keys HOT 2
- Ruby 2.4 + support HOT 2
- SSH public keys not validated/parsed if contain comments
- Pass-phrases HOT 2
- Does not work with new algorithms, like ED25519 HOT 2
- Directives give error. Rake tests pass but not directives manually. HOT 2
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from sshkey.