Skip to content
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

PIV: Support AES management keys #330

Open
str4d opened this issue Nov 22, 2021 · 6 comments · May be fixed by #589
Open

PIV: Support AES management keys #330

str4d opened this issue Nov 22, 2021 · 6 comments · May be fixed by #589

Comments

@str4d
Copy link
Contributor

str4d commented Nov 22, 2021

Historically, YubiKey's PIV applet only supported 3DES management keys. However, YubiKeys with firmware 5.4 and up (produced starting from May 2021) support AES-128, AES-192, and AES-256 management keys, which are allowed at least as early as NIST SP 800-78-2 (released in 2010).

We should add support for AES management keys, to enable people who want to migrate away from the default 3DES keys to do so.

@archaelus
Copy link

Hi all, I'm interested in working on this. One of the first questions I have is: the MgmKey struct now needs to either be a DES key or an AES key. If I turn it into an enum, I then have to bifurcate every method on MgmKey - I think this is the way I went the last time I (privately) attempted this. It works, but doesn't feel tidy. Alternately, maybe MgmKey could be a trait, and DesKey/AesKey could implement that? I'd hope with that approach that the code for different key types would be neatly separate, but I have no idea what the downsides to this are.

Anyone have a recommendation? Or I can just wade in and see how it looks.

@tony-iqlusion
Copy link
Member

Could go either way, although the trait-based approach sounds a bit nicer to me

@str4d
Copy link
Contributor Author

str4d commented Dec 29, 2024

After reading through the conversation in #578 between @GregBowyer and @kwantam, I'm trying to figure out an alternative API that preserves MgmKey as supporting all possible management key types (i.e. runtime checking rather than #578's compile-time checking). Writing a comment here to help me figure it out 🙂

The current 3DES-only interface is:

impl MgmKey {
    /// Generate a random MGM key
    pub fn generate() -> Self { .. }

    /// Create an MGM key from byte slice.
    ///
    /// Returns an error if the slice is the wrong size or the key is weak.
    pub fn from_bytes(bytes: impl AsRef<[u8]>) -> Result<Self> { .. }

    /// Create an MGM key from the given byte array.
    ///
    /// Returns an error if the key is weak.
    pub fn new(key_bytes: [u8; DES_LEN_3DES]) -> Result<Self> { .. }

    /// Get derived management key (MGM)
    pub fn get_derived(yubikey: &mut YubiKey, pin: &[u8]) -> Result<Self> { .. }

    /// Get protected management key (MGM)
    pub fn get_protected(yubikey: &mut YubiKey) -> Result<Self> { .. }

    /// Resets the management key for the given YubiKey to the default value.
    ///
    /// This will wipe any metadata related to derived and PIN-protected management keys.
    pub fn set_default(yubikey: &mut YubiKey) -> Result<()> { .. }

    /// Configures the given YubiKey to use this management key.
    ///
    /// The management key must be stored by the user, and provided when performing key
    /// management operations.
    ///
    /// This will wipe any metadata related to derived and PIN-protected management keys.
    pub fn set_manual(&self, yubikey: &mut YubiKey, require_touch: bool) -> Result<()> { .. }

    /// Configures the given YubiKey to use this as a PIN-protected management key.
    ///
    /// This enables key management operations to be performed with access to the PIN.
    pub fn set_protected(&self, yubikey: &mut YubiKey) -> Result<()> { .. }

    /// Encrypt with 3DES key
    pub(crate) fn encrypt(&self, input: &[u8; DES_LEN_DES]) -> [u8; DES_LEN_DES] { .. }

    /// Decrypt with 3DES key
    pub(crate) fn decrypt(&self, input: &[u8; DES_LEN_DES]) -> [u8; DES_LEN_DES] { .. }
}

#578 currently makes all of these APIs usable by any one management key algorithm. However, looking at the API from a high level, only some of these make sense to be algorithm-specific, while others either require (or are less efficient without) knowledge of multiple algorithms, or only make sense in the context of the Yubikey firmware version.

Here's a sketch of how I think the APIs need to look like for the two different approaches. Note that there are some equivalent APIs on OneAlgoMgmKey and AllAlgosMgmKey that can't be shared via a trait due to the difference in where the generic parameter goes.

impl<C: Algo> OneAlgoMgmKey<C> {
    /// Generates a random MGM key for this algorithm.
    pub fn generate() -> Self { .. }

    /// Parses an MGM key from the given byte slice.
    ///
    /// Returns an error if the slice is the wrong size or the key is weak.
    pub fn from_bytes(bytes: impl AsRef<[u8]>) -> Result<Self> { .. }

    /// Creates an MGM key from the given key.
    ///
    /// Returns an error if the key is weak.
    pub fn new(key: Key<C>) -> Result<Self> { .. }

    /// Derives a management key (MGM) from a stored salt.
    ///
    /// TODO: Is this supported for AES?
    pub fn get_derived(yubikey: &mut YubiKey, pin: &[u8]) -> Result<Self> { .. }

    /// Encrypts a block with this management key.
    pub(crate) fn encrypt(&self, input: &Block<C>) -> Block<C> { .. }

    /// Decrypts a block with this management key.
    pub(crate) fn decrypt(&self, input: &Block<C>) -> Block<C> { .. }
}

impl AllAlgosMgmKey {
    /// Generates a random MGM key for the given algorithm.
    pub fn generate<C: Algo>() -> Self { .. }

    /// Generates a random MGM key using the preferred algorithm for the given Yubikey's
    /// firmware version.
    pub fn generate_for(yubikey: &YubiKey) -> Self { .. }

    /// Parses an MGM key from the given byte slice.
    ///
    /// Returns an error if the slice is the wrong size or the key is weak.
    ///
    /// TODO: Can we distinguish DES from AES-192? Or do we take `C` as a parameter and
    /// require the caller to know the type of the bytes they are parsing?
    pub fn from_bytes(bytes: impl AsRef<[u8]>) -> Result<Self> { .. }

    /// Creates an MGM key from the given key.
    ///
    /// Returns an error if the key is weak.
    pub fn new<C: Algo>(key: Key<C>) -> Result<Self> { .. }

    /// Derives a management key (MGM) with the given algorithm from a stored salt.
    ///
    /// TODO: Is this supported for AES? Is the algorithm supposed to be dynamic?
    pub fn get_derived<C: Algo>(yubikey: &mut YubiKey, pin: &[u8]) -> Result<Self> { .. }

    /// Resets the management key for the given YubiKey to the default value for that
    /// Yubikey's firmware version.
    ///
    /// This will wipe any metadata related to derived and PIN-protected management keys.
    pub fn set_default(yubikey: &mut YubiKey) -> Result<()> { .. }
}

/// Management key operations with APIs that can be safely unaware of the key's algorithm.
trait MgmKeyOps {
    /// Gets the protected management key (MGM)
    ///
    /// Returns an error if the protected management key's algorithm is not supported.
    ///
    /// TODO: How can we distinguish DES from AES-192?
    pub fn get_protected(yubikey: &mut YubiKey) -> Result<Self> { .. }

    /// Configures the given YubiKey to use this management key.
    ///
    /// The management key must be stored by the user, and provided when performing key
    /// management operations.
    ///
    /// This will wipe any metadata related to derived and PIN-protected management keys.
    ///
    /// Returns an error if the Yubikey's firmware doesn't support this management key
    /// algorithm.
    pub fn set_manual(&self, yubikey: &mut YubiKey, require_touch: bool) -> Result<()> { .. }

    /// Configures the given YubiKey to use this as a PIN-protected management key.
    ///
    /// This enables key management operations to be performed with access to the PIN.
    ///
    /// Returns an error if the Yubikey's firmware doesn't support this management key
    /// algorithm.
    pub fn set_protected(&self, yubikey: &mut YubiKey) -> Result<()> { .. }

    /// Given a challenge from a card, decrypts it and return the value
    pub(crate) fn card_challenge(&self, challenge: &[u8]) -> Result<Vec<u8>> { .. }

    /// Checks the authentication matches the challenge and auth data
    pub(crate) fn check_challenge(&self, challenge: &[u8], auth_data: &[u8]) -> Result<()> { .. }
}

impl<C: Algo> From<OneAlgoMgmKey<C>> for AllAlgosMgmKey { .. }
impl<C: Algo> MgmKeyOps for OneAlgoMgmKey<C> { .. }
impl MgmKeyOps for AllAlgosMgmKey { .. }

@str4d
Copy link
Contributor Author

str4d commented Dec 29, 2024

/// TODO: How can we distinguish DES from AES-192?

Answer: we can use GET METADATA (supported on firmware 5.3 and up; for prior firmwares we can safely just assume 3DES, as AES support was added in 5.4).

@str4d
Copy link
Contributor Author

str4d commented Jan 4, 2025

Opened #588 with a draft of my alternate proposed API. Draft because I haven't tested it locally yet.

@str4d str4d linked a pull request Jan 4, 2025 that will close this issue
@str4d
Copy link
Contributor Author

str4d commented Jan 4, 2025

Opened #589 with what AES management key support will look like if we go with #588.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants