ssh-key: extract a Comment type

Extracts a type which models the concerns of comments, namely that they
can be encoded as an RFC4251 `string` type which can hold arbitrary
binary data when stored in the binary serialization of a private key.

This replaces the various accessor methods on `PublicKey` and
`PrivateKey` for handling the various conversions.

Author: tarcieri

ssh-key/src/comment.rs

@@ -0,0 +1,141 @@
+//! SSH key comment support.
+
+use alloc::{borrow::ToOwned, boxed::Box, string::String, vec::Vec};
+use core::{
+    convert::Infallible,
+    fmt,
+    str::{self, FromStr},
+};
+use encoding::{Decode, Encode, Error, Reader, Writer};
+
+/// SSH key comment (e.g. email address of owner)
+///
+/// Comments may be found in both the binary serialization of  [`PrivateKey`] as well as the text
+/// serialization of [`PublicKey`].
+///
+/// The binary serialization of [`PrivateKey`] stores the comment encoded as an [RFC4251]
+/// `string` type which can contain arbitrary binary data and does not necessarily represent valid
+/// UTF-8. To support round trip encoding of such comments.
+///
+/// To support round-trip encoding of such comments, this type also supports arbitrary binary data.
+///
+/// [RFC4251]: https://datatracker.ietf.org/doc/html/rfc4251#section-5
+/// [`PrivateKey`]: crate::PrivateKey
+/// [`PublicKey`]: crate::PublicKey
+#[derive(Clone, Debug, Default, Eq, Hash, Ord, PartialEq, PartialOrd)]
+pub struct Comment(Box<[u8]>);
+
+impl AsRef<[u8]> for Comment {
+    fn as_ref(&self) -> &[u8] {
+        self.as_bytes()
+    }
+}
+
+impl AsRef<str> for Comment {
+    fn as_ref(&self) -> &str {
+        self.as_str_lossy()
+    }
+}
+
+impl Decode for Comment {
+    type Error = Error;
+
+    fn decode(reader: &mut impl Reader) -> encoding::Result<Self> {
+        Vec::<u8>::decode(reader).map(Into::into)
+    }
+}
+
+impl Encode for Comment {
+    fn encoded_len(&self) -> Result<usize, Error> {
+        self.0.encoded_len()
+    }
+
+    fn encode(&self, writer: &mut impl Writer) -> Result<(), Error> {
+        self.0.encode(writer)
+    }
+}
+
+impl FromStr for Comment {
+    type Err = Infallible;
+
+    fn from_str(s: &str) -> Result<Comment, Infallible> {
+        Ok(s.into())
+    }
+}
+
+impl From<&str> for Comment {
+    fn from(s: &str) -> Comment {
+        Self::from(s.to_owned())
+    }
+}
+
+impl From<String> for Comment {
+    fn from(s: String) -> Self {
+        s.into_bytes().into()
+    }
+}
+
+impl From<Vec<u8>> for Comment {
+    fn from(vec: Vec<u8>) -> Self {
+        Self(vec.into_boxed_slice())
+    }
+}
+
+impl From<Comment> for Vec<u8> {
+    fn from(comment: Comment) -> Vec<u8> {
+        comment.0.into()
+    }
+}
+
+impl TryFrom<Comment> for String {
+    type Error = Error;
+
+    fn try_from(comment: Comment) -> Result<String, Error> {
+        comment.as_str().map(ToOwned::to_owned)
+    }
+}
+
+impl fmt::Display for Comment {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        f.write_str(self.as_str_lossy())
+    }
+}
+
+impl Comment {
+    /// Interpret the comment as raw binary data.
+    pub fn as_bytes(&self) -> &[u8] {
+        &self.0
+    }
+
+    /// Interpret the comment as a UTF-8 string.
+    pub fn as_str(&self) -> Result<&str, Error> {
+        Ok(str::from_utf8(&self.0)?)
+    }
+
+    /// Interpret the comment as a UTF-8 string.
+    ///
+    /// This is the maximal prefix of the
+    #[cfg(feature = "alloc")]
+    pub fn as_str_lossy(&self) -> &str {
+        for i in (1..=self.len()).rev() {
+            if let Ok(s) = str::from_utf8(&self.0[..i]) {
+                return s;
+            }
+        }
+
+        ""
+    }
+
+    /// Is the comment empty?
+    pub fn is_empty(&self) -> bool {
+        self.0.is_empty()
+    }
+
+    /// Get the length of this comment in bytes.
+    pub fn len(&self) -> usize {
+        self.0.len()
+    }
+}
+
+#[cfg(test)]
+mod tests {}

ssh-key/src/lib.rs

@@ -51,7 +51,7 @@
 //!
 //! // Key attributes
 //! assert_eq!(public_key.algorithm(), ssh_key::Algorithm::Ed25519);
-//! assert_eq!(public_key.comment(), "[email protected]");
+//! assert_eq!(public_key.comment().as_bytes(), b"[email protected]");
 //!
 //! // Key data: in this example an Ed25519 key
 //! if let Some(ed25519_public_key) = public_key.key_data().ed25519() {
@@ -100,7 +100,7 @@
 //!
 //! // Key attributes
 //! assert_eq!(private_key.algorithm(), ssh_key::Algorithm::Ed25519);
-//! assert_eq!(private_key.comment(), "[email protected]");
+//! assert_eq!(private_key.comment().as_bytes(), b"[email protected]");
 //!
 //! // Key data: in this example an Ed25519 key
 //! if let Some(ed25519_keypair) = private_key.key_data().ed25519() {
@@ -156,6 +156,8 @@ mod error;
 mod fingerprint;
 mod kdf;
 
+#[cfg(feature = "alloc")]
+mod comment;
 #[cfg(feature = "std")]
 mod dot_ssh;
 #[cfg(feature = "ppk")]
@@ -183,6 +185,7 @@ pub use {
     crate::{
         algorithm::AlgorithmName,
         certificate::Certificate,
+        comment::Comment,
         known_hosts::KnownHosts,
         signature::{Signature, SigningKey},
         sshsig::SshSig,

ssh-key/src/private.rs

@@ -124,7 +124,7 @@ pub use self::{
 
 #[cfg(feature = "alloc")]
 pub use crate::{
-    SshSig,
+    Comment, SshSig,
     private::{
         dsa::{DsaKeypair, DsaPrivateKey},
         opaque::{OpaqueKeypair, OpaqueKeypairBytes, OpaquePrivateKeyBytes},
@@ -212,7 +212,7 @@ impl PrivateKey {
     ///
     /// On `no_std` platforms, use `PrivateKey::from(key_data)` instead.
     #[cfg(feature = "alloc")]
-    pub fn new(key_data: KeypairData, comment: impl Into<Vec<u8>>) -> Result<Self> {
+    pub fn new(key_data: KeypairData, comment: impl Into<Comment>) -> Result<Self> {
         if key_data.is_encrypted() {
             return Err(Error::Encrypted);
         }
@@ -465,43 +465,8 @@ impl PrivateKey {
 
     /// Comment on the key (e.g. email address).
     #[cfg(feature = "alloc")]
-    #[deprecated(
-        since = "0.7.0",
-        note = "please use `comment_bytes`, `comment_str`, or `comment_str_lossy` instead"
-    )]
-    pub fn comment(&self) -> &str {
-        self.comment_str_lossy()
-    }
-
-    /// Comment on the key (e.g. email address).
-    #[cfg(not(feature = "alloc"))]
-    pub fn comment_bytes(&self) -> &[u8] {
-        b""
-    }
-
-    /// Comment on the key (e.g. email address).
-    ///
-    /// Since comments can contain arbitrary binary data when decoded from a
-    /// private key, this returns the raw bytes of the comment.
-    #[cfg(feature = "alloc")]
-    pub fn comment_bytes(&self) -> &[u8] {
-        self.public_key.comment_bytes()
-    }
-
-    /// Comment on the key (e.g. email address).
-    ///
-    /// This returns a UTF-8 interpretation of the comment when valid.
-    #[cfg(feature = "alloc")]
-    pub fn comment_str(&self) -> core::result::Result<&str, str::Utf8Error> {
-        self.public_key.comment_str()
-    }
-
-    /// Comment on the key (e.g. email address).
-    ///
-    /// This returns as much data as can be interpreted as valid UTF-8.
-    #[cfg(feature = "alloc")]
-    pub fn comment_str_lossy(&self) -> &str {
-        self.public_key.comment_str_lossy()
+    pub fn comment(&self) -> &Comment {
+        self.public_key.comment()
     }
 
     /// Cipher algorithm (a.k.a. `ciphername`).
@@ -575,7 +540,7 @@ impl PrivateKey {
 
     /// Set the comment on the key.
     #[cfg(feature = "alloc")]
-    pub fn set_comment(&mut self, comment: impl Into<Vec<u8>>) {
+    pub fn set_comment(&mut self, comment: impl Into<Comment>) {
         self.public_key.set_comment(comment);
     }
 
@@ -681,7 +646,7 @@ impl PrivateKey {
         checkint.encode(writer)?;
         checkint.encode(writer)?;
         self.key_data.encode(writer)?;
-        self.comment_bytes().encode(writer)?;
+        self.comment().encode(writer)?;
         writer.write(&PADDING_BYTES[..padding_len])?;
         Ok(())
     }
@@ -704,7 +669,7 @@ impl PrivateKey {
         [
             8, // 2 x uint32 checkints,
             self.key_data.encoded_len()?,
-            self.comment_bytes().encoded_len()?,
+            self.comment().encoded_len()?,
         ]
         .checked_sum()
     }

ssh-key/src/public.rs

@@ -35,7 +35,7 @@ use encoding::{Base64Reader, Decode, Reader};
 
 #[cfg(feature = "alloc")]
 use {
-    crate::SshSig,
+    crate::{Comment, SshSig},
     alloc::{
         borrow::ToOwned,
         string::{String, ToString},
@@ -67,7 +67,7 @@ use crate::PrivateKey;
 ///
 /// 1. Algorithm identifier (in this example `ssh-ed25519`)
 /// 2. Key data encoded as Base64
-/// 3. Comment (optional): arbitrary label describing a key. Usually an email address
+/// 3. [`Comment`] (optional): arbitrary label describing a key. Usually an email address
 ///
 /// The [`PublicKey::from_openssh`] and [`PublicKey::to_openssh`] methods can be
 /// used to decode/encode public keys, or alternatively, the [`FromStr`] and
@@ -99,15 +99,15 @@ pub struct PublicKey {
     /// binary data, so `Vec<u8>` is used to store the comment to ensure keys
     /// containing such comments successfully round-trip.
     #[cfg(feature = "alloc")]
-    pub(crate) comment: Vec<u8>,
+    pub(crate) comment: Comment,
 }
 
 impl PublicKey {
     /// Create a new public key with the given comment.
     ///
     /// On `no_std` platforms, use `PublicKey::from(key_data)` instead.
     #[cfg(feature = "alloc")]
-    pub fn new(key_data: KeyData, comment: impl Into<Vec<u8>>) -> Self {
+    pub fn new(key_data: KeyData, comment: impl Into<Comment>) -> Self {
         Self {
             key_data,
             comment: comment.into(),
@@ -152,7 +152,7 @@ impl PublicKey {
         #[cfg(not(feature = "alloc"))]
         let comment = "";
         #[cfg(feature = "alloc")]
-        let comment = self.comment_str_lossy();
+        let comment = self.comment.as_str_lossy();
 
         SshFormat::encode(self.algorithm().as_str(), &self.key_data, comment, out)
     }
@@ -164,7 +164,7 @@ impl PublicKey {
         SshFormat::encode_string(
             self.algorithm().as_str(),
             &self.key_data,
-            self.comment_str_lossy(),
+            self.comment.as_str_lossy(),
         )
     }
 
@@ -258,48 +258,11 @@ impl PublicKey {
     }
 
     /// Comment on the key (e.g. email address).
-    ///
-    /// This is a deprecated alias for [`PublicKey::comment_str_lossy`].
     #[cfg(feature = "alloc")]
-    #[deprecated(
-        since = "0.7.0",
-        note = "please use `comment_bytes`, `comment_str`, or `comment_str_lossy` instead"
-    )]
-    pub fn comment(&self) -> &str {
-        self.comment_str_lossy()
-    }
-
-    /// Comment on the key (e.g. email address).
-    ///
-    /// Since comments can contain arbitrary binary data when decoded from a
-    /// private key, this returns the raw bytes of the comment.
-    #[cfg(feature = "alloc")]
-    pub fn comment_bytes(&self) -> &[u8] {
+    pub fn comment(&self) -> &Comment {
         &self.comment
     }
 
-    /// Comment on the key (e.g. email address).
-    ///
-    /// This returns a UTF-8 interpretation of the comment when valid.
-    #[cfg(feature = "alloc")]
-    pub fn comment_str(&self) -> core::result::Result<&str, str::Utf8Error> {
-        str::from_utf8(&self.comment)
-    }
-
-    /// Comment on the key (e.g. email address).
-    ///
-    /// This returns as much data as can be interpreted as valid UTF-8.
-    #[cfg(feature = "alloc")]
-    pub fn comment_str_lossy(&self) -> &str {
-        for i in (1..=self.comment.len()).rev() {
-            if let Ok(s) = str::from_utf8(&self.comment[..i]) {
-                return s;
-            }
-        }
-
-        ""
-    }
-
     /// Public key data.
     pub fn key_data(&self) -> &KeyData {
         &self.key_data
@@ -314,7 +277,7 @@ impl PublicKey {
 
     /// Set the comment on the key.
     #[cfg(feature = "alloc")]
-    pub fn set_comment(&mut self, comment: impl Into<Vec<u8>>) {
+    pub fn set_comment(&mut self, comment: impl Into<Comment>) {
         self.comment = comment.into();
     }
 
@@ -330,7 +293,7 @@ impl PublicKey {
     /// Decode comment (e.g. email address)
     #[cfg(feature = "alloc")]
     pub(crate) fn decode_comment(&mut self, reader: &mut impl Reader) -> Result<()> {
-        self.comment = Vec::decode(reader)?;
+        self.comment = Comment::decode(reader)?;
         Ok(())
     }
 }
@@ -340,7 +303,7 @@ impl From<KeyData> for PublicKey {
         PublicKey {
             key_data,
             #[cfg(feature = "alloc")]
-            comment: Vec::new(),
+            comment: Comment::default(),
         }
     }
 }