Skip to content

Conversation

@johnmaguire
Copy link
Member

No description provided.

@johnmaguire johnmaguire merged commit 1d1f008 into main Dec 12, 2024
2 checks passed
@johnmaguire johnmaguire deleted the helper-func branch December 12, 2024 20:56
@nbrownus
Copy link

This is fine, I was curious if we would want to know the type so we could assert any incompatible type situations

@johnmaguire
Copy link
Member Author

johnmaguire commented Dec 12, 2024

@nbrownus What do you mean by "any incompatible type situations"? This is just a wrapper function around some existing unmarshal functions that makes it more convenient to load keys from disk to be passed back to dnapi when you're storing them marshalled as PEM in a single field. PrivateKey.Unwrap() gives you the underlying type.

dnclient didn't need/want this function because it stores the cert in either a p256 or an ed field. This is mostly just because the existing JSON field was called ed_pubkey and I didn't want to stick P256 in it. And because we didn't used to store the private key as a PEM in dnclient - just raw bytes. Mobile Nebula on the other hand has always used PEM, but it used the wrong header before (handled in the mobile_nebula repo.)

@nbrownus
Copy link

I haven't reviewed the usage of the function but I see we are losing context on which curve is contained in the pem bytes.

If that curve doesn't match the public keys curve then we have some problems and the crypto might not fail (ed25519 mixed with x25519)

@johnmaguire
Copy link
Member Author

johnmaguire commented Dec 12, 2024

@nbrownus NewPrivateKey returns an implementation of Ed25519PrivateKey or P256PrivateKey. The PrivateKey interface Unwrap() returns the underlying key (which can be type-matched.) I don't think we lose context. :) It's just that this context isn't necessarily useful to consumers of dnapi, but functions like MarshalPEM are useful to call w/o switching on types.

@johnmaguire
Copy link
Member Author

dnapi/keys/keys.go

Lines 110 to 119 in 9f2241d

func NewPrivateKey(k any) (PrivateKey, error) {
switch k := k.(type) {
case *ecdsa.PrivateKey:
return P256PrivateKey{k}, nil
case ed25519.PrivateKey:
return Ed25519PrivateKey{k}, nil
default:
return nil, fmt.Errorf("unsupported private key type: %T", k)
}
}

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 this pull request may close these issues.

4 participants