-
Notifications
You must be signed in to change notification settings - Fork 262
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
OS Customisation: Regex to match ssh public keys #959
base: qml
Are you sure you want to change the base?
Conversation
Fixes raspberrypi#956 Previously we would just trust that users would include a correct SSH public key. Unfortunately, this isn't a viable stategy. So, let's introduce a regex to perform some light validation of the ssh-key, to avoid having boot failures.
3318487
to
c2faa5d
Compare
wrapMode: TextEdit.WrapAnywhere | ||
Layout.fillWidth: true | ||
Layout.minimumWidth: 350 | ||
Layout.leftMargin: 40 | ||
selectByMouse: true | ||
validator: RegularExpressionValidator { regularExpression: /^ssh-(ed25519|rsa|dss|ecdsa) AAAA(?:[A-Za-z0-9+\/]{4})*(?:[A-Za-z0-9+\/]{2}==|[A-Za-z0-9+\/]{3}=|[A-Za-z0-9+\/]{4})( [^@]+@[^@]+)?/ } |
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.
If the aim of this PR is to fix #956 by preventing '
or "
from appearing in the public key field (which breaks firstrun.sh
), perhaps the use of [^@]+
is a bit overly broad? 🤔
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.
If the aim of this PR is to fix #956 by preventing
'
or"
from appearing in the public key field (which breaksfirstrun.sh
), perhaps the use of[^@]+
is a bit overly broad? 🤔
You're correct - this would prevent one appearing at the start of the ssh-key, but not at the end.
@@ -404,15 +404,14 @@ Window { | |||
// textFormat: Text.PlainText | |||
Layout.leftMargin: 40 | |||
} | |||
TextArea { | |||
TextField { |
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.
Does the switch from TextArea
to TextField
mean that advanced users will no longer be able to specify multiple public SSH keys in this field?
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.
Yes - this breaks that case in favour of introducing a real-time validator. TextArea has no such equivalent mechanism.
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.
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.
Good call, @maxnet, and one that clearly identifies where the TextArea choice came from.
The semi-obvious mitigation is to use a Repeater or similar to create dynamic UI elements that we then validate independently, and manually compose into the customisation line.
Fixes #956
Previously we would just trust that users would include a correct SSH public key. Unfortunately, this isn't a viable stategy.
So, let's introduce a regex to perform some light validation of the ssh-key, to avoid having boot failures.