Skip to content

Commit

Permalink
Switch to mirage-crypto and refactor code
Browse files Browse the repository at this point in the history
  • Loading branch information
@reynir
reynir committed Mar 14, 2020
1 parent d74d5c0 commit 8988207
Show file tree
Hide file tree
Showing 11 changed files with 63 additions and 38 deletions.
2 changes: 1 addition & 1 deletion cmd/dune
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
(executable
(name ssh_add)
(libraries ssh-agent nocrypto nocrypto.unix ssh-agent-unix cmdliner))
(libraries ssh-agent mirage-crypto-pk mirage-crypto-rng.unix ssh-agent-unix cmdliner))
4 changes: 2 additions & 2 deletions cmd/ssh_add.ml
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@


let main bits key_comment =
let () = Nocrypto_entropy_unix.initialize () in
let () = Mirage_crypto_rng_unix.initialize () in
let sock_path =
match Sys.getenv "SSH_AUTH_SOCK" with
| path -> path
Expand All @@ -10,7 +10,7 @@ let main bits key_comment =
let () = Unix.connect fd Unix.(ADDR_UNIX sock_path) in
let ic = Unix.in_channel_of_descr fd in
let oc = Unix.out_channel_of_descr fd in
let privkey = Ssh_agent.Privkey.Ssh_rsa (Nocrypto.Rsa.generate bits) in
let privkey = Ssh_agent.Privkey.Ssh_rsa (Mirage_crypto_pk.Rsa.generate ~bits ()) in
let req = Ssh_agent.Ssh_agentc_add_identity { privkey; key_comment } in
match Ssh_agent_unix.request (ic, oc) req with
| Ok Ssh_agent.Ssh_agent_success ->
Expand Down
2 changes: 1 addition & 1 deletion src/dune
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,4 @@
(public_name ssh-agent)
(preprocess
(pps ppx_cstruct ppx_sexp_conv))
(libraries cstruct angstrom nocrypto faraday))
(libraries cstruct angstrom mirage-crypto mirage-crypto-pk faraday sexplib))
54 changes: 39 additions & 15 deletions src/parse.ml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ module Wire = struct
then return Z.zero
else take (Int32.to_int mpint_len)
>>= fun mpint ->
return (Nocrypto.Numeric.Z.of_cstruct_be (Cstruct.of_string mpint))
return (Mirage_crypto_pk.Z_extra.of_cstruct_be (Cstruct.of_string mpint))

let name_list =
string >>|
Expand All @@ -56,13 +56,21 @@ let pub_ssh_dss =
Wire.mpint >>= fun q ->
Wire.mpint >>= fun gg ->
Wire.mpint >>= fun y ->
return (Pubkey.Ssh_dss { p; q; gg; y })
match Mirage_crypto_pk.Dsa.pub ~p ~q ~gg ~y () with
| Error (`Msg e) ->
Angstrom.fail ("Mirage_crypto_pk.Dsa.pub: " ^ e)
| Ok pub ->
return pub

let pub_ssh_rsa =
let open Angstrom in
Wire.mpint >>= fun e ->
Wire.mpint >>= fun n ->
return (Pubkey.Ssh_rsa { e; n })
match Mirage_crypto_pk.Rsa.pub ~e ~n with
| Error (`Msg e) ->
Angstrom.fail ("Mirage_crypto_pk.Rsa.pub: " ^ e)
| Ok pub ->
return pub

let string_tuple =
let open Angstrom in
Expand All @@ -77,8 +85,7 @@ let pub_blob key_type =
let rec pub_ssh_rsa_cert () =
let open Angstrom in
Wire.string >>= fun nonce ->
Wire.mpint >>= fun e ->
Wire.mpint >>= fun n ->
pub_ssh_rsa >>= fun pubkey_to_be_signed ->
Wire.uint64 >>= fun serial ->
Wire.uint32 >>= fun typ ->
match Protocol_number.int_to_ssh_cert_type typ with
Expand All @@ -96,7 +103,7 @@ let rec pub_ssh_rsa_cert () =
return {
Pubkey.to_be_signed = {
Pubkey.nonce;
pubkey = { e; n };
pubkey = pubkey_to_be_signed;
serial;
typ;
key_id;
Expand All @@ -115,9 +122,11 @@ and pubkey can_be_cert =
let open Angstrom in
Wire.string >>= function
| "ssh-dss" ->
pub_ssh_dss
pub_ssh_dss >>= fun pubkey ->
return (Pubkey.Ssh_dss pubkey)
| "ssh-rsa" ->
pub_ssh_rsa
pub_ssh_rsa >>= fun pubkey ->
return (Pubkey.Ssh_rsa pubkey)
| "ssh-rsa-cert-v01@openssh.com" ->
if can_be_cert
then
Expand All @@ -134,7 +143,11 @@ let ssh_dss =
Wire.mpint >>= fun gg ->
Wire.mpint >>= fun y ->
Wire.mpint >>= fun x ->
return (Privkey.Ssh_dss { p; q; gg; y; x })
match Mirage_crypto_pk.Dsa.priv ~p ~q ~gg ~y ~x () with
| Error (`Msg e) ->
fail ("Mirage_crypto_pk.Dsa.priv: " ^ e)
| Ok priv ->
return priv

let ssh_rsa =
let open Angstrom in
Expand All @@ -144,8 +157,12 @@ let ssh_rsa =
Wire.mpint >>= fun _iqmp ->
Wire.mpint >>= fun p ->
Wire.mpint >>= fun q ->
(* FIXME: How do the parameters correspond to Nocrypto.Rsa.priv ? *)
return (Privkey.Ssh_rsa (Nocrypto.Rsa.priv_of_primes ~e ~p ~q))
(* FIXME: How do the parameters correspond to Mirage_crypto_pk.Rsa.priv ? *)

This comment has been minimized.

Sign in to view

Copy link
@hannesm

hannesm Mar 21, 2020

FWIW, do you have a reference at hand that describes the mpi in ssh_rsa? on a separate thought, what should the relation between awa-ssh and ocaml-ssh-agent be? (now that both use mirage-crypto, the former could be released as well, and I can see code being shared)

This comment has been minimized.

Sign in to view

Copy link
@reynir

reynir Mar 21, 2020

Author Owner

I'm not sure what you mean by mpi.
awa-ssh can use ocaml-ssh-agent to communicate with an ssh-agent. There is some code that can be shared. For example, the code for (de)serializing the wire format and for signing messages. They take different approaches to (de)serializing, though.

This comment has been minimized.

Sign in to view

Copy link
@hannesm

hannesm Mar 21, 2020

MPI = multi-precision integer
from the line in your code above, "How do the parameters correspond to Mirage_crypto_pk.Rsa.priv", I am curious about this as well, and would like to figure out how they are related ;)

This comment has been minimized.

Sign in to view

Copy link
@reynir

reynir Mar 21, 2020

Author Owner

Aha, so not message passing interface ;)
I think I used this definition, and it itself refers to FIPS.186-4: https://github.com/reynir/ocaml-ssh-agent/blob/master/docs/draft-miller-ssh-agent-00.txt#L290-L312

This comment has been minimized.

Sign in to view

Copy link
@hannesm

hannesm Mar 21, 2020

ok:
n, e, d are the same (public modulus, public exponent, private exponent)
iqmp is q' (q ^ -1 mod p)
q and p are the same (private pimes)

you could check in this code for equivalence thereof, or compute dp (d mod p) and dq (d mod q) and use Mirage_crypto_pk.Rsa.priv directly (maybe that function should take some arguments as option, and replace the various constructors? -- but maybe that is then a bit too magic)
match Mirage_crypto_pk.Rsa.priv_of_primes ~e ~p ~q with
| Error (`Msg e) ->
fail ("Mirage_crypto_pk.Rsa.priv_of_primes: " ^ e)
| Ok priv ->
return priv

let ssh_rsa_cert =
let open Angstrom in
Expand All @@ -160,7 +177,11 @@ let ssh_rsa_cert =
Wire.mpint >>= fun p ->
Wire.mpint >>= fun q ->
let e = cert.Pubkey.to_be_signed.Pubkey.pubkey.e in
return (Privkey.Ssh_rsa_cert (Nocrypto.Rsa.priv_of_primes ~e ~p ~q, cert))
match Mirage_crypto_pk.Rsa.priv_of_primes ~e ~p ~q with
| Error (`Msg e) ->
fail ("Mirage_crypto_pk.Rsa.priv_of_primes: " ^ e)
| Ok priv ->
return (priv, cert)

let blob key_type =
let open Angstrom in
Expand All @@ -171,11 +192,14 @@ let privkey =
let open Angstrom in
Wire.string >>= function
| "ssh-dss" ->
ssh_dss
ssh_dss >>= fun priv ->
return (Privkey.Ssh_dss priv)
| "ssh-rsa" ->
ssh_rsa
ssh_rsa >>= fun priv ->
return (Privkey.Ssh_rsa priv)
| "ssh-rsa-cert-v01@openssh.com" ->
ssh_rsa_cert
ssh_rsa_cert >>= fun (priv, cert) ->
return (Privkey.Ssh_rsa_cert (priv, cert))
| key_type ->
blob key_type

Expand Down
2 changes: 1 addition & 1 deletion src/serialize.ml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ module Wire = struct
if mpint = Z.zero
then write_uint32 t 0l
else
let mpint = Nocrypto.Numeric.Z.to_cstruct_be mpint in
let mpint = Mirage_crypto_pk.Z_extra.to_cstruct_be mpint in
let mpint_padded =
if Cstruct.get_uint8 mpint 0 land 0x80 <> 0
then Cstruct.append (Cstruct.of_string "\000") mpint
Expand Down
8 changes: 4 additions & 4 deletions src/ssh_agent.ml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,14 +62,14 @@ let sign priv (sign_flags : Protocol_number.sign_flag list) blob =
| Privkey.Ssh_rsa key | Privkey.Ssh_rsa_cert (key, _) ->
let alg_string, to_sign =
if List.mem Protocol_number.SSH_AGENT_RSA_SHA2_512 sign_flags
then let digest = Nocrypto.Hash.SHA512.digest (Cstruct.of_string blob) in
then let digest = Mirage_crypto.Hash.SHA512.digest (Cstruct.of_string blob) in
"rsa-sha2-512", Cstruct.append Util.id_sha512 digest
else if List.mem Protocol_number.SSH_AGENT_RSA_SHA2_256 sign_flags
then let digest = Nocrypto.Hash.SHA256.digest (Cstruct.of_string blob) in
then let digest = Mirage_crypto.Hash.SHA256.digest (Cstruct.of_string blob) in
"rsa-sha2-256", Cstruct.append Util.id_sha256 digest
else let digest = Nocrypto.Hash.SHA1.digest (Cstruct.of_string blob) in
else let digest = Mirage_crypto.Hash.SHA1.digest (Cstruct.of_string blob) in
"ssh-rsa", Cstruct.append Util.id_sha1 digest in
let signed = Nocrypto.Rsa.PKCS1.sig_encode ~key to_sign in
let signed = Mirage_crypto_pk.Rsa.PKCS1.sig_encode ~key to_sign in
Serialize.(with_faraday (fun t ->
Wire.write_string t alg_string;
Wire.write_string t (Cstruct.to_string signed)))
Expand Down
8 changes: 4 additions & 4 deletions src/ssh_agent.mli
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
module Pubkey : sig
type ssh_dss = Nocrypto.Dsa.pub
type ssh_dss = Mirage_crypto_pk.Dsa.pub
[@@deriving sexp_of]

type ssh_rsa = Nocrypto.Rsa.pub
type ssh_rsa = Mirage_crypto_pk.Rsa.pub
[@@deriving sexp_of]

type options = (string * string) list
Expand Down Expand Up @@ -65,10 +65,10 @@ module Pubkey : sig
end

module Privkey : sig
type ssh_dss = Nocrypto.Dsa.priv
type ssh_dss = Mirage_crypto_pk.Dsa.priv
[@@deriving sexp_of]

type ssh_rsa = Nocrypto.Rsa.priv
type ssh_rsa = Mirage_crypto_pk.Rsa.priv
[@@deriving sexp_of]

type t =
Expand Down
8 changes: 4 additions & 4 deletions src/types.ml
View file
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
open Sexplib.Conv

module Pubkey = struct
type ssh_dss = Nocrypto.Dsa.pub
type ssh_dss = Mirage_crypto_pk.Dsa.pub
[@@deriving sexp_of]

type ssh_rsa = Nocrypto.Rsa.pub
type ssh_rsa = Mirage_crypto_pk.Rsa.pub
[@@deriving sexp_of]

type options = (string * string) list
Expand Down Expand Up @@ -40,10 +40,10 @@ module Pubkey = struct
end

module Privkey = struct
type ssh_dss = Nocrypto.Dsa.priv
type ssh_dss = Mirage_crypto_pk.Dsa.priv
[@@deriving sexp_of]

type ssh_rsa = Nocrypto.Rsa.priv
type ssh_rsa = Mirage_crypto_pk.Rsa.priv
[@@deriving sexp_of]

type t =
Expand Down
5 changes: 3 additions & 2 deletions ssh-agent.opam
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,9 @@ depends: [
"ppx_sexp_conv"
"angstrom" {>= "0.10" & < "0.13"}
"faraday" {>= "0.6" & < "0.8"}
"nocrypto"
"mirage-crypto"
"mirage-crypto-pk"
"cstruct"
"sexplib"
"alcotest" {with-test}
"sexplib" {with-test}
]
2 changes: 1 addition & 1 deletion test/dune
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
(executable
(name test)
(libraries ssh-agent alcotest nocrypto.unix sexplib))
(libraries ssh-agent alcotest mirage-crypto-rng.unix sexplib))

(alias
(name runtest)
Expand Down
6 changes: 3 additions & 3 deletions test/test.ml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,9 +92,9 @@ let m_response = (module Response
: Alcotest.TESTABLE with type t = Ssh_agent.any_ssh_agent_response)


let () = Nocrypto_entropy_unix.initialize ()
let privkey = Nocrypto.Rsa.generate 1024
let pubkey = Nocrypto.Rsa.pub_of_priv privkey
let () = Mirage_crypto_rng_unix.initialize ()
let privkey = Mirage_crypto_pk.Rsa.generate ~bits:1024 ()
let pubkey = Mirage_crypto_pk.Rsa.pub_of_priv privkey
let privkey = Ssh_agent.Privkey.Ssh_rsa privkey
let pubkey = Ssh_agent.Pubkey.Ssh_rsa pubkey

Expand Down

0 comments on commit 8988207

Please sign in to comment.