Paul Wouters writes:
> Well, we have been passing domain names to forward via IKEv1 XAUTH for
> 10+ years and I've never heard of a problem with passing domain names
> as strings. I find this "security software authors might do dangerous
> string passing" a rather unconvincing argument.
Simple string is not an issue. Regex to check whether domain name is
leagl is quite simple, and even then people do that wrong.
Checking whether string is valid textual representation of the dns
record is harder, and you have to think things like, are comments
allowed, are newlines allowed. Also if you want to verify that the DS
record is actually for the real domain name, and not something else
you need to parse the string to get the first entry of it, and you
might need to know what $ORIGIN is to be able to know which suffix you
need to add to it if it does not have dot in the end etc...
I do not even know whether that format has some way of escaping
special characters, i.e., some kind of \x2e format or so.
Where is the formal specification for that text representation?
RFC4034 just specifies DS RR specific stuff, but it does not give any
indication for the generic format. For example where is it said that
"; key id = 60485" is comment?
I tried quickly look that up, but could not find it, so it would be
better to add direct reference to the rfc specifying the generic text
representation format of DNS RR records, not just the DS RR specific
parts which are in RFC4034.
> > Ifwe use string format the IKEv2 implementations might easily just
> > pass the string forward without any checking, and this might cause
> > security vulnerabilities like we had with shellshock or similar.
>
> We are talking about dedicated security software here, not /bin/bash. If
> your security software can't do string passing, you already are using
> a timb bomb.
You said that someone uses environment variables, and some uses it as
command line arguments. Those all might have issues with bash parsing
environment variables in specific ways, meaning that the security
software needs to make sure the things it passes to them are safe.
Yes, the easist way to do string passing that format is to first parse
it in IKEv2 from text to binary, and then reformat it back to text
format which I know cannot have anything funny in it, but before I can
do that I need to know the format I am about to parse, and for now I
do not have that information.
> > If the input format is some kind of binary format, and the IKEv2
> > library then constructs the string format out of that it will remove
> > ability to do that kind of attacks.
>
> Why? You don't think attackers can encode weird characters in binary
> which would be "blindly" decoded into ascii with the same dangerous
> characters inside?
No they cannot. If I encode the 8-bit number as 8-bit binary, when I
parse it, I will get 8-bit number, which I can then print with "%d" to
ascii if needed.
> > With string formats we need to make sure that there is nothing
> > dangerous in there ("$INCLUDE /etc/passwd" or "; `sh -c
> > /sbin/reboot`") and we have to specify what kind of checking
> > implementations should do on the format.
>
> Same for binary format?
We do not need to specify our binary format to include any of those
special things. There is no need for our binary format to allow
comments, or including other files or anything like that. We do not
need to specify binary format that includes all features the text
format has, only the features we need, which in our case is only to
encode the DS RR record, which consists exactly 4 fields.
> We either have to write our own routines (more likely) or link
> against a DNS library that has this functionality.
You can link against DNS library, I would write:
ret = ssh_decode_buffer(buffer,
SSH_DECODE_UINT8_STR(&name, &name_len),
SSH_DECODE_UINT16(&key_tag),
SSH_DECODE_CHAR(&alg),
SSH_DECODE_CHAR(&digest_type),
SSH_FORMAT_END);
if (ret == 0) { return SSH_IKEV2_INVALID_SYNTAX; }
digest = ssh_buffer_ptr(buffer) + ret;
digest_len = ssh_buffer_len(buffer) - ret;
which is most likely easier, than to find out the suitable location in
dns library where to call to get the rr-record decoded.
> Like using ldns, which would drag in so much code people more likely
> will copy the wire conversation functions from some dns library into
> their code and never update that code again, possibly leaving in
> security vulnerabilities.
Yes. And as we are talking about 10 lines of code, I think they should
write it specifically for this job, and not use huge library.
> > Saying that the contents is DS RDATA wire format as specified in the
> > RFC4034 section 5.1 would be much easier, and converting that to
> > string representation is quite easy, as it is just 1 16-bit field, two
> > 8-bit field, and then the digest:
>
> And now you have to understand DNS wire format and all their possible
> changes, use of compression, sneaking in multiple records, etc etc.
I was talking DS RDATA wire format as specifice in 5.1, and I do not
think there is anything like compression there, or multiple records,
as it is just one record. But if you like, we can just copy the fields
and their sizes to this specification and not use the RDATA format if
that makes you happy.
>
> > INTERNAL_DNS_DOMAIN(example.com,31406,8,2,
> > 0xF78CF3344F72137235098ECBBD08947C2C9001C7F6A085A17F518B5D8F6B916D)
>
> I understand you can do this, but it does add a string2wire() and
> wire2string() operation for what I believe is a red-herring for security
> reasons.
I disagree with you. I think it will add security, will make the
implementations easier to write, and smaller. Especially if you do
have your own resolver you are going to configure, and which might not
use textual format at all. In that case you suddenly need to implement
full textual format parser just to parse this one line of text.
> > It is not enough for the IPsec server to fetch KSK, as the client
> > might have fetched the TA information two months ago, when it
> > connected to the SGW. It might have done several IKEv2 rekeys after
> > that, but as it does configuration payloads only when it initially
> > connects it will not get new configuration information ever.
>
> Perhaps that needs clarification, but my idea was that you purge the
> trust anchor and the zone data underneath it when you tear down the
> IPsec SA.
IPsec SA? I assume you are meaning IKEv2 SA, not IPsec SA (Child SA),
as configuration parameters passed inside the configuration paylads
are tied to the lifetime of the IKEv2 SA, not Child SA.
> The VPN tunnel is not meant to be a replacement for RFC-5011
> style trust anchor updating. The split-DNS view can be changed at any
> time by the server/internal network and clients MUST NOT store this
> information as a permanent update. I will add language to clarify that.
But you do store it for few years, i.e. as long as your IKEv2 SA
happens to stay up... Not all IKEv2 SAs are short lived. Some of them
might stay up for months, or even years.
> > This might not be something that we actually need to fix here, it
> > might be enough just to add some words about that, and say that if
> > internal domain does KSK roll overs then the server might want to
> > request clients to do reauthentication (RFC4478) while doing roll over
> > so they will get new configuration information before the old one is
> > removed.
>
> Any internal KSK roll should be done slowly enough (days or weeks)
> so that any VPN client connecting can always pull a working DNSKEY
> RRset.
Why would client reconnect that often? I mean if I have IKEv2 up and
running, and I do rekeys every 24 hours, why should I need to
reconnect it ever unless the network between devices break down.
> Perhaps a re-key or re-auth event should allow to request/send a new
> CFG set to keep updated for very long lived tunnels?
IKEv2 rekey does not do that, as old configuration parameters are
inherited by new IKEv2 SA. Re-authentication (i.e. tearing down the
IKEv2 SA and creating new one) do make new configuration payload
exchange, thus might get new IP-address and will get new trust anchors
for DNSSEC.
And yes, the server can just send delete notification to the IKEv2 SA
when the old trust anchor is about to expire, thus forcing the client
to do reauthentication, and this will work even if the client does not
support RFC4478.
This might of course cause issues, as user might not be there to type
in the credentials need to reconnect, thus causing the connection to
stay down...
> > Internal server might be willing to resolve everything, and even
> > provide completely new DS for "." so you can do everything securely,
> > but the user might be bit suprised to learn that his facebook.com
> > requests go to the company dns server, and dnssec is happy with the
> > faked results pointing to the companys internal facebook proxy that
> > checks they do not post anything nasty about company to the
> > facebook...
>
> I am not sure if you are agreeing with the SHOULD or the MUST here? You
> seem to be arguing to leave it as MUST?
Not really, I am saying we should have warning saying that rogue
or badly configure server can cause all kind of issues especially by
sending the DS records for ".". The security considerations section do
have text saying that if vpn gateway A sends "example.com" and vpn
gateway B does the same then the IKE connection is terminated, but it
does not say that vpn gateway a might send "." as INTERNAL_DNS_DOMAIN
and give new TA for "." too, which will forward all DNS requests to
the vpn...
Also if one VPN gateway sends "example.com" and another sends "com"
which one of them is used? The one which the dns resolver you use
happens to use, or do we want to specifcy that resolvers should then
always use longest match when matching against the list of
resolvers...
> > So this is also security issue, i.e., how much configuration you
> > should trust from the server. Bad server can send you bad
> > configuration data, and client might want to limit the damage done by
> > the bad server.
>
> Which is why the client can send the list of domains it is willing to
> have overridden, or it can interpret the received list. And it can
> ignore "." if that is not in its locally accepted list.
But adding reasoning for that to the security considerations section
is needed...
--
[email protected]
_______________________________________________
IPsec mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/ipsec
