haskell-tls / hs-certificate

Certificate and Key Reader/Writer in haskell
60 stars 57 forks source link

Use 'IP' instead of raw 'ByteString' for AltNameIP constructor #102

Open KtorZ opened 5 years ago

KtorZ commented 5 years ago

This leaves the encoding / decoding of the IP address to the x509 library. This encoding is quite specific for x509 and there's no reason why the caller wouldn't like to leverage a higher-level type here.

Note, I've tested it in a different commit by exposing the ipFromBS and ipToBS constructors:

instance Arbitrary IP where
  arbitrary = oneof
    [ ipv4 <$> arbitrary <*> arbitrary <*> arbitrary <*> arbitrary
    , ipv6 <$> arbitrary <*> arbitrary <*> arbitrary <*> arbitrary <*> arbitrary <*> arbitrary <*> arbitrary <*> arbitrary
    ]

property_roundtrip_ip :: IP -> Bool
property_roundtrip_ip ip =
  case ipFromBS (ipToBS ip) of
    Left err -> error err
    Right ip' | ip' == ip -> True
              | otherwise -> error ("expected " ++ show ip ++ " got: " ++ show ip')
    ip:                   OK
      +++ OK, passed 100 tests.

Though, I am not sure you want to expose those as part of the main Data.X509 API (since Data.X509.Internal isn't exposed).

Let me know.

KtorZ commented 5 years ago

Switched from ip to foundation and defined a custom data IP to unify both Foundation.Network.IPv4 and Foundation.Network.IPv6 :+1:

ocheron commented 5 years ago

I can understand the need for higher-level type but moving away from the ASN.1 definition also has downside. It increases the risk of parse failure.

If ever needed I implemented the reverse approach where content is kept as bytestring and parsed only in x509-validation.

Also in the Name Constraints extension, the same GeneralName datatype is used but with different content. The bytestring is the concatenation of address and mask.

KtorZ commented 5 years ago

Hey! I am not sure to understand your point :thinking_face: ?

The encoding for IP addresses of x509 certs is well-defined through RFC so there's not much freedom given to developers when it comes to specifying an IP SAN.

This PR removes the hassle of doing the encoding ourselves.