From b41ff52857cda37f10c2861ecada8f8e9d49b28e Mon Sep 17 00:00:00 2001 From: Lars Date: Tue, 30 May 2023 17:19:19 +0200 Subject: [PATCH 1/3] Restored functionality to choose RSA hash algorithm Removed error when printing decoded certificate while trying to get private key fingerprint Updated docs Updated changelog --- README.md | 17 ++-- docs/cert.html | 32 ++++++-- docs/fields.html | 164 ++++++++++++++++++++++++------------- docs/index.html | 13 +-- docs/keys.html | 22 +++-- docs/utils.html | 83 ++++++++++++++++--- src/sshkey_tools/cert.py | 8 +- src/sshkey_tools/fields.py | 7 +- 8 files changed, 249 insertions(+), 97 deletions(-) diff --git a/README.md b/README.md index 0b9ccd5..325cbdf 100644 --- a/README.md +++ b/README.md @@ -22,11 +22,7 @@ Python package for managing OpenSSH keypairs and certificates ([protocol.CERTKEY - Export certificates to file, string or bytes # Roadmap -- [x] Rewrite certificate field functionality for simpler usage -- [ ] Re-add functionality for changing RSA hash method -- [ ] Add CLI functionality -- [ ] Convert to/from putty format (keys only) - +See issues for planned features and fixes # Installation @@ -49,6 +45,13 @@ pip3 install ./ # Documentation You can find the full documentation at [scheiblingco.github.io/sshkey-tools/](https://scheiblingco.github.io/sshkey-tools/) +## Building the documentation +```bash +pdoc3 src/sshkey_tools/ -o docs --html +cp -rf docs/sshkey_tools/* docs/ +rm -rf docs/sshkey_tools +``` + ## SSH Keypairs (generating, loading, exporting) ```python # Import the certificate classes @@ -308,6 +311,10 @@ certificate.sign() ``` ## Changelog +### 0.9.1 +- Updated documentation +- Fix for bug where exception would occur when trying to export a key without a comment set + ### 0.9 - Adjustments to certificate field handling for easier usage/syntax autocompletion - Updated testing diff --git a/docs/cert.html b/docs/cert.html index bba49f6..d14897f 100644 --- a/docs/cert.html +++ b/docs/cert.html @@ -540,9 +540,13 @@

Raises

bytes(self.header), bytes(self.fields), bytes(self.footer) ) - def sign(self) -> bool: + def sign(self, **kwargs) -> bool: """Sign the certificate + Args: + **kwargs: Arguments to pass to the signature signing method + ex. hash_alg for RSA signatures + Raises: _EX.NotSignedException: The certificate could not be signed @@ -550,7 +554,7 @@

Raises

bool: Whether successful """ if self.can_sign(): - self.footer.signature.sign(data=self.get_signable()) + self.footer.signature.sign(data=self.get_signable(), **kwargs) return True raise _EX.NotSignedException("There was an error while signing the certificate") @@ -2164,9 +2168,13 @@

Inherited members

bytes(self.header), bytes(self.fields), bytes(self.footer) ) - def sign(self) -> bool: + def sign(self, **kwargs) -> bool: """Sign the certificate + Args: + **kwargs: Arguments to pass to the signature signing method + ex. hash_alg for RSA signatures + Raises: _EX.NotSignedException: The certificate could not be signed @@ -2174,7 +2182,7 @@

Inherited members

bool: Whether successful """ if self.can_sign(): - self.footer.signature.sign(data=self.get_signable()) + self.footer.signature.sign(data=self.get_signable(), **kwargs) return True raise _EX.NotSignedException("There was an error while signing the certificate") @@ -2672,10 +2680,16 @@

Returns

-def sign(self) ‑> bool +def sign(self, **kwargs) ‑> bool

Sign the certificate

+

Args

+
+
**kwargs
+
Arguments to pass to the signature signing method +ex. hash_alg for RSA signatures
+

Raises

_EX.NotSignedException
@@ -2690,9 +2704,13 @@

Returns

Expand source code -
def sign(self) -> bool:
+
def sign(self, **kwargs) -> bool:
     """Sign the certificate
 
+    Args:
+        **kwargs: Arguments to pass to the signature signing method
+                  ex. hash_alg for RSA signatures
+
     Raises:
         _EX.NotSignedException: The certificate could not be signed
 
@@ -2700,7 +2718,7 @@ 
Returns

         bool: Whether successful
     """
     if self.can_sign():
-        self.footer.signature.sign(data=self.get_signable())
+        self.footer.signature.sign(data=self.get_signable(), **kwargs)
 
         return True
     raise _EX.NotSignedException("There was an error while signing the certificate")

diff --git a/docs/fields.html b/docs/fields.html
index 26ffaf1..7f078dd 100644
--- a/docs/fields.html
+++ b/docs/fields.html
@@ -66,7 +66,7 @@ 
Module sshkey_tools.fields

     long_to_bytes,
     random_keyid,
     random_serial,
-    str_to_timedelta,
+    str_to_time_delta,
 )
 
 NoneType = type(None)
@@ -168,7 +168,7 @@ 
Module sshkey_tools.fields

         """
         Validates if the field is set when required
         """
-        if self.DEFAULT == self.value is None:
+        if self.DEFAULT and self.value is None:
             return _EX.InvalidFieldDataException(
                 f"{self.get_name()} is a required field"
             )
@@ -294,22 +294,23 @@ 
Module sshkey_tools.fields

     DEFAULT = b""
 
     @classmethod
-    def encode(cls, value: bytes) -> bytes:
+    def encode(cls, value: bytes, encoding: str = "utf-8") -> bytes:
         """
         Encodes a string or bytestring into a packed byte string
 
         Args:
             value (Union[str, bytes]): The string/bytestring to encode
-            encoding (str): The encoding to user for the string
+            encoding (str): The optional encoding, not used when passing
+                            a byte value.
 
         Returns:
             bytes: Packed byte string containing the source data
         """
         cls.__validate_type__(value, True)
-        return pack(">I", len(value)) + ensure_bytestring(value)
+        return pack(">I", len(value)) + ensure_bytestring(value, encoding)
 
     @staticmethod
-    def decode(data: bytes) -> Tuple[bytes, bytes]:
+    def decode(data: bytes, encoding: str = None) -> Tuple[bytes, bytes]:
         """
         Unpacks the next string from a packed byte string
 
@@ -321,6 +322,10 @@ 
Module sshkey_tools.fields

                                   string and remainder of the data
         """
         length = unpack(">I", data[:4])[0] + 4
+
+        if encoding is not None:
+            return ensure_string(data[4:length], encoding), data[length:]
+
         return ensure_bytestring(data[4:length]), data[length:]
 
 
@@ -344,8 +349,7 @@ 
Module sshkey_tools.fields

         Returns:
             bytes: Packed byte string containing the source data
         """
-        cls.__validate_type__(value, True)
-        return BytestringField.encode(ensure_bytestring(value, encoding))
+        return super().encode(value, encoding)
 
     @staticmethod
     def decode(data: bytes, encoding: str = "utf-8") -> Tuple[str, bytes]:
@@ -359,9 +363,7 @@ 
Module sshkey_tools.fields

             tuple(bytes, bytes):  The next block of bytes from the packed byte
                                   string and remainder of the data
         """
-        value, data = BytestringField.decode(data)
-
-        return value.decode(encoding), data
+        return BytestringField.decode(data, encoding)
 
 
 class Integer32Field(CertificateField):
@@ -498,9 +500,9 @@ 
Module sshkey_tools.fields

 
         if isinstance(value, str):
             if value == "forever":
-                return Integer64Field.encode(MAX_INT64)
+                return Integer64Field.encode(MAX_INT64 - 1)
 
-            value = int(datetime.now() + str_to_timedelta(value))
+            value = int((datetime.now() + str_to_time_delta(value)).timestamp())
 
         if isinstance(value, datetime):
             value = int(value.timestamp())
@@ -779,7 +781,7 @@ 
Module sshkey_tools.fields

         return True
 
 
-class NonceField(StringField):
+class NonceField(BytestringField):
     """
     Contains the nonce for the certificate, randomly generated
     this protects the integrity of the private key, especially
@@ -1321,8 +1323,11 @@ 
Module sshkey_tools.fields

 
     def __table__(self) -> tuple:
         msg = "No signature"
-        if self.is_signed:
+        if self.is_signed and self.private_key is not None:
             msg = f"Signed with private key {self.private_key.get_fingerprint()}"
+        
+        if self.is_signed and self.private_key is None:
+            msg = "Signed with: See pubkey fingerprint above"
 
         return ("Signature", msg)
 
@@ -1379,7 +1384,7 @@ 
Module sshkey_tools.fields

         """
         return self.private_key is not None
 
-    def sign(self, data: bytes) -> None:
+    def sign(self, data: bytes, **kwargs) -> None:
         """
         Placeholder signing function
         """
@@ -1931,22 +1936,23 @@ 
Inherited members

     DEFAULT = b""
 
     @classmethod
-    def encode(cls, value: bytes) -> bytes:
+    def encode(cls, value: bytes, encoding: str = "utf-8") -> bytes:
         """
         Encodes a string or bytestring into a packed byte string
 
         Args:
             value (Union[str, bytes]): The string/bytestring to encode
-            encoding (str): The encoding to user for the string
+            encoding (str): The optional encoding, not used when passing
+                            a byte value.
 
         Returns:
             bytes: Packed byte string containing the source data
         """
         cls.__validate_type__(value, True)
-        return pack(">I", len(value)) + ensure_bytestring(value)
+        return pack(">I", len(value)) + ensure_bytestring(value, encoding)
 
     @staticmethod
-    def decode(data: bytes) -> Tuple[bytes, bytes]:
+    def decode(data: bytes, encoding: str = None) -> Tuple[bytes, bytes]:
         """
         Unpacks the next string from a packed byte string
 
@@ -1958,6 +1964,10 @@ 
Inherited members

                                   string and remainder of the data
         """
         length = unpack(">I", data[:4])[0] + 4
+
+        if encoding is not None:
+            return ensure_string(data[4:length], encoding), data[length:]
+
         return ensure_bytestring(data[4:length]), data[length:]

Ancestors

@@ -1968,6 +1978,7 @@

Subclasses

Class variables

@@ -1984,7 +1995,7 @@

Class variables

Static methods

-def decode(data: bytes) ‑> Tuple[bytes, bytes] +def decode(data: bytes, encoding: str = None) ‑> Tuple[bytes, bytes]

Unpacks the next string from a packed byte string

@@ -2002,7 +2013,7 @@

Returns

Expand source code 
@staticmethod
-def decode(data: bytes) -> Tuple[bytes, bytes]:
+def decode(data: bytes, encoding: str = None) -> Tuple[bytes, bytes]:
     """
     Unpacks the next string from a packed byte string
 
@@ -2014,11 +2025,15 @@ 
Returns

                               string and remainder of the data
     """
     length = unpack(">I", data[:4])[0] + 4
+
+    if encoding is not None:
+        return ensure_string(data[4:length], encoding), data[length:]
+
     return ensure_bytestring(data[4:length]), data[length:]
-def encode(value: bytes) ‑> bytes +def encode(value: bytes, encoding: str = 'utf-8') ‑> bytes

Encodes a string or bytestring into a packed byte string

@@ -2027,7 +2042,8 @@

Args

value : Union[str, bytes]
The string/bytestring to encode
encoding : str
-
The encoding to user for the string
+
The optional encoding, not used when passing +a byte value.

Returns

@@ -2039,19 +2055,20 @@

Returns

Expand source code 
@classmethod
-def encode(cls, value: bytes) -> bytes:
+def encode(cls, value: bytes, encoding: str = "utf-8") -> bytes:
     """
     Encodes a string or bytestring into a packed byte string
 
     Args:
         value (Union[str, bytes]): The string/bytestring to encode
-        encoding (str): The encoding to user for the string
+        encoding (str): The optional encoding, not used when passing
+                        a byte value.
 
     Returns:
         bytes: Packed byte string containing the source data
     """
     cls.__validate_type__(value, True)
-    return pack(">I", len(value)) + ensure_bytestring(value)
+ return pack(">I", len(value)) + ensure_bytestring(value, encoding)
@@ -2367,7 +2384,7 @@

Class variables

""" Validates if the field is set when required """ - if self.DEFAULT == self.value is None: + if self.DEFAULT and self.value is None: return _EX.InvalidFieldDataException( f"{self.get_name()} is a required field" ) @@ -2893,9 +2910,9 @@

Inherited members

if isinstance(value, str): if value == "forever": - return Integer64Field.encode(MAX_INT64) + return Integer64Field.encode(MAX_INT64 - 1) - value = int(datetime.now() + str_to_timedelta(value)) + value = int((datetime.now() + str_to_time_delta(value)).timestamp()) if isinstance(value, datetime): value = int(value.timestamp()) @@ -3033,9 +3050,9 @@

Returns

if isinstance(value, str): if value == "forever": - return Integer64Field.encode(MAX_INT64) + return Integer64Field.encode(MAX_INT64 - 1) - value = int(datetime.now() + str_to_timedelta(value)) + value = int((datetime.now() + str_to_time_delta(value)).timestamp()) if isinstance(value, datetime): value = int(value.timestamp()) @@ -4761,7 +4778,7 @@

Inherited members

  • StringField:
    • diff --git a/src/sshkey_tools/cert.py b/src/sshkey_tools/cert.py index 4a893a2..36d71f5 100644 --- a/src/sshkey_tools/cert.py +++ b/src/sshkey_tools/cert.py @@ -495,9 +495,13 @@ def get_signable(self) -> bytes: bytes(self.header), bytes(self.fields), bytes(self.footer) ) - def sign(self) -> bool: + def sign(self, **kwargs) -> bool: """Sign the certificate + Args: + **kwargs: Arguments to pass to the signature signing method + ex. hash_alg for RSA signatures + Raises: _EX.NotSignedException: The certificate could not be signed @@ -505,7 +509,7 @@ def sign(self) -> bool: bool: Whether successful """ if self.can_sign(): - self.footer.signature.sign(data=self.get_signable()) + self.footer.signature.sign(data=self.get_signable(), **kwargs) return True raise _EX.NotSignedException("There was an error while signing the certificate") diff --git a/src/sshkey_tools/fields.py b/src/sshkey_tools/fields.py index 72a029c..dd2fc41 100644 --- a/src/sshkey_tools/fields.py +++ b/src/sshkey_tools/fields.py @@ -1294,8 +1294,11 @@ def __init__(self, private_key: PrivateKey = None, signature: bytes = None): def __table__(self) -> tuple: msg = "No signature" - if self.is_signed: + if self.is_signed and self.private_key is not None: msg = f"Signed with private key {self.private_key.get_fingerprint()}" + + if self.is_signed and self.private_key is None: + msg = "Signed with: See pubkey fingerprint above" return ("Signature", msg) @@ -1352,7 +1355,7 @@ def can_sign(self): """ return self.private_key is not None - def sign(self, data: bytes) -> None: + def sign(self, data: bytes, **kwargs) -> None: """ Placeholder signing function """ From a0721588880d55df93c416e5766ea1a5cdfec418 Mon Sep 17 00:00:00 2001 From: Lars Date: Tue, 30 May 2023 17:30:56 +0200 Subject: [PATCH 2/3] Added Rsa alg choice to documentation Added note about DSA deprecation and ECDSA flaws --- README.md | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 325cbdf..729b8f4 100644 --- a/README.md +++ b/README.md @@ -2,9 +2,20 @@ Python package for managing OpenSSH keypairs and certificates ([protocol.CERTKEYS](https://github.com/openssh/openssh-portable/blob/master/PROTOCOL.certkeys)). Supported functionality includes: +# Notice +The DSA algorithm is considered deprecated and will be removed in a future version. If possible, use RSA, [(ECDSA)](https://billatnapier.medium.com/ecdsa-weakness-where-nonces-are-reused-2be63856a01a) or ED25519 as a first-hand choice. + +Notice from OpenSSH: +``` +OpenSSH 7.0 and greater similarly disable the ssh-dss (DSA) public key algorithm. It too is weak and we recommend against its use. It can be re-enabled using the HostKeyAlgorithms configuration option: sshd_config(5) HostKeyAlgorithms +``` + +[ECDSA has some flaws](https://billatnapier.medium.com/ecdsa-weakness-where-nonces-are-reused-2be63856a01a), especially when using short nonces or re-using nonces, it can still be used but exercise some caution in regards to nonces/re-signing identical data multiple times. + + # Features ### SSH Keys -- Supports RSA, DSA, ECDSA and ED25519 keys +- Supports RSA, DSA (Note: Deprecated), ECDSA and ED25519 keys - Import existing keys from file, string, byte data or [pyca/cryptography](https://github.com/pyca/cryptography) class - Generate new keys - Get public key from private keys @@ -127,6 +138,7 @@ b"\0xc\0a\........" The loaded private key objects can be used to sign bytestrings, and the public keys can be used to verify signatures on those ```python from sshkey_tools.keys import RsaPrivateKey, RsaPublicKey +from sshkey_tools.fields import RsaAlgs signable_data = b'This is a message that will be signed' @@ -136,6 +148,10 @@ pubkey = RsaPrivateKey.public_key # Sign the data signature = privkey.sign(signable_data) +# When using an RSA key for the signature, you can specify the hashing algorithm +# The default algorithm is SHA512 +signature = privkey.sign(signable_data, RsaAlgs.SHA512) + # Verify the signature (Throws exception if invalid) pubkey.verify(signable_data, signature) ``` From 50f13cedd4d28e348621e206820a546dadaffdc2 Mon Sep 17 00:00:00 2001 From: Lars Date: Tue, 30 May 2023 17:35:04 +0200 Subject: [PATCH 3/3] Linting --- src/sshkey_tools/fields.py | 2 +- src/sshkey_tools/keys.py | 2 +- src/sshkey_tools/utils.py | 3 ++- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/sshkey_tools/fields.py b/src/sshkey_tools/fields.py index dd2fc41..81fd844 100644 --- a/src/sshkey_tools/fields.py +++ b/src/sshkey_tools/fields.py @@ -1296,7 +1296,7 @@ def __table__(self) -> tuple: msg = "No signature" if self.is_signed and self.private_key is not None: msg = f"Signed with private key {self.private_key.get_fingerprint()}" - + if self.is_signed and self.private_key is None: msg = "Signed with: See pubkey fingerprint above" diff --git a/src/sshkey_tools/keys.py b/src/sshkey_tools/keys.py index 46293cb..3c286a4 100644 --- a/src/sshkey_tools/keys.py +++ b/src/sshkey_tools/keys.py @@ -129,7 +129,7 @@ def __init__( _SERIALIZATION.Encoding.OpenSSH, _SERIALIZATION.PublicFormat.OpenSSH, ] - + # Ensure comment is not None self.comment = nullsafe_getattr(self, "comment", "") diff --git a/src/sshkey_tools/utils.py b/src/sshkey_tools/utils.py index 4b21af2..1eef0a1 100644 --- a/src/sshkey_tools/utils.py +++ b/src/sshkey_tools/utils.py @@ -246,9 +246,10 @@ def nullsafe_getattr(obj, attr: str, default): att = getattr(obj, attr, default) if att is None: att = default - + return att + def join_dicts(*dicts) -> dict: """ Joins two or more dictionaries together.