* [Patch] NetworkPkg/IpSecDxe: Generate SPI randomly and correct IKE_SPI_BASE value
@ 2016-09-06 3:39 Jiaxin Wu
2016-09-06 3:39 ` [Patch] NetworkPkg/DnsDxe: Handle CNAME type responded from the name server Jiaxin Wu
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Jiaxin Wu @ 2016-09-06 3:39 UTC (permalink / raw)
To: edk2-devel; +Cc: Ye Ting, Fu Siyuan
This path made the following update:
* Generate SPI randomly.
* Correct IKE_SPI_BASE value according RFC 4302/4303.
Cc: Ye Ting <ting.ye@intel.com>
Cc: Fu Siyuan <siyuan.fu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiaxin Wu <jiaxin.wu@intel.com>
---
NetworkPkg/IpSecDxe/IkeCommon.c | 102 +++++++++++++++++++++++++++++++-----
NetworkPkg/IpSecDxe/IkeCommon.h | 20 ++++---
NetworkPkg/IpSecDxe/Ikev2/Utility.c | 11 +++-
3 files changed, 112 insertions(+), 21 deletions(-)
diff --git a/NetworkPkg/IpSecDxe/IkeCommon.c b/NetworkPkg/IpSecDxe/IkeCommon.c
index 6fc7c06..b1e4321 100644
--- a/NetworkPkg/IpSecDxe/IkeCommon.c
+++ b/NetworkPkg/IpSecDxe/IkeCommon.c
@@ -1,9 +1,9 @@
/** @file
Common operation of the IKE
- Copyright (c) 2010 - 2015, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2010 - 2016, Intel Corporation. All rights reserved.<BR>
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD License
which accompanies this distribution. The full text of the license may be found at
http://opensource.org/licenses/bsd-license.php.
@@ -16,14 +16,56 @@
#include "Ike.h"
#include "IkeCommon.h"
#include "IpSecConfigImpl.h"
#include "IpSecDebug.h"
-//
-// Initial the SPI
-//
-UINT32 mNextSpi = IKE_SPI_BASE;
+/**
+ Check whether the new generated Spi has existed.
+
+ @param[in] IkeSaSession Pointer to the Child SA Session.
+ @param[in] SpiValue SPI Value.
+
+ @retval TRUE This SpiValue has existed in the Child SA Session
+ @retval FALSE This SpiValue doesn't exist in the Child SA Session.
+
+**/
+BOOLEAN
+IkeSpiValueExisted (
+ IN IKEV2_SA_SESSION *IkeSaSession,
+ IN UINT32 SpiValue
+ )
+{
+ LIST_ENTRY *Entry;
+ LIST_ENTRY *Next;
+ IKEV2_CHILD_SA_SESSION *SaSession;
+
+ Entry = NULL;
+ Next = NULL;
+ SaSession = NULL;
+
+ //
+ // Check whether the SPI value has existed in ChildSaEstablishSessionList.
+ //
+ NET_LIST_FOR_EACH_SAFE (Entry, Next, &IkeSaSession->ChildSaEstablishSessionList) {
+ SaSession= IKEV2_CHILD_SA_SESSION_BY_IKE_SA (Entry);
+ if (SaSession->LocalPeerSpi == SpiValue) {
+ return TRUE;
+ }
+ }
+
+ //
+ // Check whether the SPI value has existed in ChildSaSessionList.
+ //
+ NET_LIST_FOR_EACH_SAFE (Entry, Next, &IkeSaSession->ChildSaSessionList) {
+ SaSession= IKEV2_CHILD_SA_SESSION_BY_IKE_SA (Entry);
+ if (SaSession->LocalPeerSpi == SpiValue) {
+ return TRUE;
+ }
+ }
+
+ return FALSE;
+}
/**
Call Crypto Lib to generate a random value with eight-octet length.
@return the 64 byte vaule.
@@ -156,23 +198,57 @@ IkePayloadFree (
FreePool (IkePayload);
}
/**
Generate an new SPI.
-
- @return a SPI in 4 bytes.
+
+ @param[in] IkeSaSession Pointer to IKEV2_SA_SESSION related to this Child SA
+ Session.
+ @param[in out] SpiValue Pointer to the new generated SPI value.
+
+ @retval EFI_SUCCESS The operation performs successfully.
+ @retval Otherwise The operation is failed.
**/
-UINT32
+EFI_STATUS
IkeGenerateSpi (
- VOID
+ IN IKEV2_SA_SESSION *IkeSaSession,
+ OUT UINT32 *SpiValue
)
{
- //
- // TODO: should generate SPI randomly to avoid security issue
- //
- return mNextSpi++;
+ EFI_STATUS Status;
+
+ Status = EFI_SUCCESS;
+
+ while (TRUE) {
+ //
+ // Generate SPI randomly
+ //
+ Status = IpSecCryptoIoGenerateRandomBytes ((UINT8 *)SpiValue, sizeof (UINT32));
+ if (EFI_ERROR (Status)) {
+ break;
+ }
+
+ //
+ // The set of SPI values in the range 1 through 255 are reserved by the
+ // Internet Assigned Numbers Authority (IANA) for future use; a reserved
+ // SPI value will not normally be assigned by IANA unless the use of the
+ // assigned SPI value is specified in an RFC.
+ //
+ if (*SpiValue < IKE_SPI_BASE) {
+ *SpiValue += IKE_SPI_BASE;
+ }
+
+ //
+ // Check whether the new generated SPI has existed.
+ //
+ if (!IkeSpiValueExisted (IkeSaSession, *SpiValue)) {
+ break;
+ }
+ }
+
+ return Status;
}
/**
Generate a random data for IV
diff --git a/NetworkPkg/IpSecDxe/IkeCommon.h b/NetworkPkg/IpSecDxe/IkeCommon.h
index 714ecaa..7f7fd4d 100644
--- a/NetworkPkg/IpSecDxe/IkeCommon.h
+++ b/NetworkPkg/IpSecDxe/IkeCommon.h
@@ -1,9 +1,9 @@
/** @file
Common operation of the IKE.
- Copyright (c) 2010 - 2015, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2010 - 2016, Intel Corporation. All rights reserved.<BR>
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD License
which accompanies this distribution. The full text of the license may be found at
http://opensource.org/licenses/bsd-license.php.
@@ -37,11 +37,11 @@
#define IKE_DEFAULT_PORT 500
#define IKE_DEFAULT_TIMEOUT_INTERVAL 10000 // 10s
#define IKE_NONCE_SIZE 16
#define IKE_MAX_RETRY 4
-#define IKE_SPI_BASE 0x10000
+#define IKE_SPI_BASE 0x100
#define IKE_PAYLOAD_SIGNATURE SIGNATURE_32('I','K','E','P')
#define IKE_PAYLOAD_BY_PACKET(a) CR(a,IKE_PAYLOAD,ByPacket,IKE_PAYLOAD_SIGNATURE)
#define IKE_PACKET_APPEND_PAYLOAD(IkePacket,IkePayload) \
@@ -128,18 +128,24 @@ VOID
IkePayloadFree (
IN IKE_PAYLOAD *IkePayload
);
/**
- Generate an unused SPI
-
- @return a SPI in 4 bytes.
+ Generate an new SPI.
+
+ @param[in] IkeSaSession Pointer to IKEV2_SA_SESSION related to this Child SA
+ Session.
+ @param[in out] SpiValue Pointer to the new generated SPI value.
+
+ @retval EFI_SUCCESS The operation performs successfully.
+ @retval Otherwise The operation is failed.
**/
-UINT32
+EFI_STATUS
IkeGenerateSpi (
- VOID
+ IN IKEV2_SA_SESSION *IkeSaSession,
+ OUT UINT32 *SpiValue
);
/**
Generate a random data for IV
diff --git a/NetworkPkg/IpSecDxe/Ikev2/Utility.c b/NetworkPkg/IpSecDxe/Ikev2/Utility.c
index 5b26ba1..c365532 100644
--- a/NetworkPkg/IpSecDxe/Ikev2/Utility.c
+++ b/NetworkPkg/IpSecDxe/Ikev2/Utility.c
@@ -523,11 +523,20 @@ Ikev2ChildSaSessionAlloc (
// Initialize the fields of ChildSaSession and its SessionCommon.
//
ChildSaSession->Signature = IKEV2_CHILD_SA_SESSION_SIGNATURE;
ChildSaSession->IkeSaSession = IkeSaSession;
ChildSaSession->MessageId = IkeSaSession->MessageId;
- ChildSaSession->LocalPeerSpi = IkeGenerateSpi ();
+
+ //
+ // Generate an new SPI.
+ //
+ Status = IkeGenerateSpi (IkeSaSession, &(ChildSaSession->LocalPeerSpi));
+ if (EFI_ERROR (Status)) {
+ FreePool (ChildSaSession);
+ return NULL;
+ }
+
ChildSaCommon = &ChildSaSession->SessionCommon;
ChildSaCommon->UdpService = UdpService;
ChildSaCommon->Private = IkeSaSession->SessionCommon.Private;
ChildSaCommon->IkeSessionType = IkeSessionTypeChildSa;
ChildSaCommon->IkeVer = 2;
--
1.9.5.msysgit.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [Patch] NetworkPkg/DnsDxe: Handle CNAME type responded from the name server
2016-09-06 3:39 [Patch] NetworkPkg/IpSecDxe: Generate SPI randomly and correct IKE_SPI_BASE value Jiaxin Wu
@ 2016-09-06 3:39 ` Jiaxin Wu
2016-09-06 3:55 ` Fu, Siyuan
2016-09-06 3:55 ` [Patch] NetworkPkg/IpSecDxe: Generate SPI randomly and correct IKE_SPI_BASE value Fu, Siyuan
2016-09-06 6:37 ` Ye, Ting
2 siblings, 1 reply; 8+ messages in thread
From: Jiaxin Wu @ 2016-09-06 3:39 UTC (permalink / raw)
To: edk2-devel; +Cc: Hegde Nagaraj P, Fu Siyuan, Ye Ting
According RFC 1034 - 3.6.2, if the query name is an alias, the name server
will include the CNAME record in the response and restart the query at the
domain name specified in the data field of the CNAME record. RFC also provides
one example server action when A query received:
Suppose a name server was processing a query with for USCISIC.ARPA, asking for
type A information, and had the following resource records:
USC-ISIC.ARPA IN CNAME C.ISI.EDU
C.ISI.EDU IN A 10.0.0.52
Both of these RRs would be returned in the response to the type A query.
Currently, DnsDxe driver doesn't handle the CNAME type response, which will cause
any exception result. The driver need continue the packet parsing while CNAME type
record parsed. So, this patch is used to handle it correctly.
Cc: Hegde Nagaraj P <nagaraj-p.hegde@hpe.com>
Cc: Fu Siyuan <siyuan.fu@intel.com>
Cc: Ye Ting <ting.ye@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiaxin Wu <jiaxin.wu@intel.com>
---
NetworkPkg/DnsDxe/DnsImpl.c | 60 ++++++++++++++++++++++++++-------------------
1 file changed, 35 insertions(+), 25 deletions(-)
diff --git a/NetworkPkg/DnsDxe/DnsImpl.c b/NetworkPkg/DnsDxe/DnsImpl.c
index 360f68e..c68ec88 100644
--- a/NetworkPkg/DnsDxe/DnsImpl.c
+++ b/NetworkPkg/DnsDxe/DnsImpl.c
@@ -1231,17 +1231,10 @@ ParseDnsResponse (
if (DnsHeader->Flags.Bits.RCode != DNS_FLAGS_RCODE_NO_ERROR || DnsHeader->AnswersNum < 1 || \
DnsHeader->Flags.Bits.QR != DNS_FLAGS_QR_RESPONSE) {
Status = EFI_ABORTED;
goto ON_EXIT;
}
-
- //
- // Free the sending packet.
- //
- if (Item->Value != NULL) {
- NetbufFree ((NET_BUF *) (Item->Value));
- }
//
// Do some buffer allocations.
//
if (Instance->Service->IpVersion == IP_VERSION_4) {
@@ -1288,40 +1281,42 @@ ParseDnsResponse (
//
// It's the GeneralLookUp querying.
//
Dns6TokenEntry->Token->RspData.GLookupData = AllocatePool (sizeof (DNS_RESOURCE_RECORD));
if (Dns6TokenEntry->Token->RspData.GLookupData == NULL) {
- Status = EFI_UNSUPPORTED;
+ Status = EFI_OUT_OF_RESOURCES;
goto ON_EXIT;
}
Dns6TokenEntry->Token->RspData.GLookupData->RRList = AllocatePool (DnsHeader->AnswersNum * sizeof (DNS_RESOURCE_RECORD));
if (Dns6TokenEntry->Token->RspData.GLookupData->RRList == NULL) {
- Status = EFI_UNSUPPORTED;
+ Status = EFI_OUT_OF_RESOURCES;
goto ON_EXIT;
}
} else {
//
// It's not the GeneralLookUp querying. Check the Query type.
//
if (QuerySection->Type == DNS_TYPE_AAAA) {
Dns6TokenEntry->Token->RspData.H2AData = AllocatePool (sizeof (DNS6_HOST_TO_ADDR_DATA));
if (Dns6TokenEntry->Token->RspData.H2AData == NULL) {
- Status = EFI_UNSUPPORTED;
+ Status = EFI_OUT_OF_RESOURCES;
goto ON_EXIT;
}
Dns6TokenEntry->Token->RspData.H2AData->IpList = AllocatePool (DnsHeader->AnswersNum * sizeof (EFI_IPv6_ADDRESS));
if (Dns6TokenEntry->Token->RspData.H2AData->IpList == NULL) {
- Status = EFI_UNSUPPORTED;
+ Status = EFI_OUT_OF_RESOURCES;
goto ON_EXIT;
}
} else {
Status = EFI_UNSUPPORTED;
goto ON_EXIT;
}
}
}
+ Status = EFI_NOT_FOUND;
+
//
// Processing AnswerSection.
//
while (AnswerSectionNum < DnsHeader->AnswersNum) {
//
@@ -1348,51 +1343,53 @@ ParseDnsResponse (
//
// Fill the ResourceRecord.
//
Dns4RR[RRCount].QName = AllocateZeroPool (AsciiStrLen (QueryName) + 1);
if (Dns4RR[RRCount].QName == NULL) {
- Status = EFI_UNSUPPORTED;
+ Status = EFI_OUT_OF_RESOURCES;
goto ON_EXIT;
}
CopyMem (Dns4RR[RRCount].QName, QueryName, AsciiStrLen (QueryName));
Dns4RR[RRCount].QType = AnswerSection->Type;
Dns4RR[RRCount].QClass = AnswerSection->Class;
Dns4RR[RRCount].TTL = AnswerSection->Ttl;
Dns4RR[RRCount].DataLength = AnswerSection->DataLength;
Dns4RR[RRCount].RData = AllocateZeroPool (Dns4RR[RRCount].DataLength);
if (Dns4RR[RRCount].RData == NULL) {
- Status = EFI_UNSUPPORTED;
+ Status = EFI_OUT_OF_RESOURCES;
goto ON_EXIT;
}
CopyMem (Dns4RR[RRCount].RData, AnswerData, Dns4RR[RRCount].DataLength);
RRCount ++;
+ Status = EFI_SUCCESS;
} else if (Instance->Service->IpVersion == IP_VERSION_6 && Dns6TokenEntry->GeneralLookUp) {
Dns6RR = Dns6TokenEntry->Token->RspData.GLookupData->RRList;
AnswerData = (UINT8 *) AnswerSection + sizeof (*AnswerSection);
//
// Fill the ResourceRecord.
//
Dns6RR[RRCount].QName = AllocateZeroPool (AsciiStrLen (QueryName) + 1);
if (Dns6RR[RRCount].QName == NULL) {
- Status = EFI_UNSUPPORTED;
+ Status = EFI_OUT_OF_RESOURCES;
goto ON_EXIT;
}
CopyMem (Dns6RR[RRCount].QName, QueryName, AsciiStrLen (QueryName));
Dns6RR[RRCount].QType = AnswerSection->Type;
Dns6RR[RRCount].QClass = AnswerSection->Class;
Dns6RR[RRCount].TTL = AnswerSection->Ttl;
Dns6RR[RRCount].DataLength = AnswerSection->DataLength;
Dns6RR[RRCount].RData = AllocateZeroPool (Dns6RR[RRCount].DataLength);
if (Dns6RR[RRCount].RData == NULL) {
- Status = EFI_UNSUPPORTED;
+ Status = EFI_OUT_OF_RESOURCES;
goto ON_EXIT;
}
CopyMem (Dns6RR[RRCount].RData, AnswerData, Dns6RR[RRCount].DataLength);
RRCount ++;
+ Status = EFI_SUCCESS;
} else {
//
// It's not the GeneralLookUp querying.
// Check the Query type, parse the response packet.
//
@@ -1425,30 +1422,31 @@ ParseDnsResponse (
//
// Allocate new CacheEntry pool.
//
Dns4CacheEntry = AllocateZeroPool (sizeof (EFI_DNS4_CACHE_ENTRY));
if (Dns4CacheEntry == NULL) {
- Status = EFI_UNSUPPORTED;
+ Status = EFI_OUT_OF_RESOURCES;
goto ON_EXIT;
}
Dns4CacheEntry->HostName = AllocateZeroPool (2 * (StrLen(Dns4TokenEntry->QueryHostName) + 1));
if (Dns4CacheEntry->HostName == NULL) {
- Status = EFI_UNSUPPORTED;
+ Status = EFI_OUT_OF_RESOURCES;
goto ON_EXIT;
}
CopyMem (Dns4CacheEntry->HostName, Dns4TokenEntry->QueryHostName, 2 * (StrLen(Dns4TokenEntry->QueryHostName) + 1));
Dns4CacheEntry->IpAddress = AllocateZeroPool (sizeof (EFI_IPv4_ADDRESS));
if (Dns4CacheEntry->IpAddress == NULL) {
- Status = EFI_UNSUPPORTED;
+ Status = EFI_OUT_OF_RESOURCES;
goto ON_EXIT;
}
CopyMem (Dns4CacheEntry->IpAddress, AnswerData, sizeof (EFI_IPv4_ADDRESS));
Dns4CacheEntry->Timeout = AnswerSection->Ttl;
UpdateDns4Cache (&mDriverData->Dns4CacheList, FALSE, TRUE, *Dns4CacheEntry);
- IpCount ++;
+ IpCount ++;
+ Status = EFI_SUCCESS;
break;
case DNS_TYPE_AAAA:
//
// This is address entry, get Data.
//
@@ -1476,30 +1474,38 @@ ParseDnsResponse (
//
// Allocate new CacheEntry pool.
//
Dns6CacheEntry = AllocateZeroPool (sizeof (EFI_DNS6_CACHE_ENTRY));
if (Dns6CacheEntry == NULL) {
- Status = EFI_UNSUPPORTED;
+ Status = EFI_OUT_OF_RESOURCES;
goto ON_EXIT;
}
Dns6CacheEntry->HostName = AllocateZeroPool (2 * (StrLen(Dns6TokenEntry->QueryHostName) + 1));
if (Dns6CacheEntry->HostName == NULL) {
- Status = EFI_UNSUPPORTED;
+ Status = EFI_OUT_OF_RESOURCES;
goto ON_EXIT;
}
CopyMem (Dns6CacheEntry->HostName, Dns6TokenEntry->QueryHostName, 2 * (StrLen(Dns6TokenEntry->QueryHostName) + 1));
Dns6CacheEntry->IpAddress = AllocateZeroPool (sizeof (EFI_IPv6_ADDRESS));
if (Dns6CacheEntry->IpAddress == NULL) {
- Status = EFI_UNSUPPORTED;
+ Status = EFI_OUT_OF_RESOURCES;
goto ON_EXIT;
}
CopyMem (Dns6CacheEntry->IpAddress, AnswerData, sizeof (EFI_IPv6_ADDRESS));
Dns6CacheEntry->Timeout = AnswerSection->Ttl;
UpdateDns6Cache (&mDriverData->Dns6CacheList, FALSE, TRUE, *Dns6CacheEntry);
IpCount ++;
+ Status = EFI_SUCCESS;
+ break;
+ case DNS_TYPE_CNAME:
+ //
+ // According RFC 1034 - 3.6.2, if the query name is an alias, the name server will include the CNAME
+ // record in the response and restart the query at the domain name specified in the data field of the
+ // CNAME record. So, let's skip this CNAME record parsing and continue the next one.
+ //
break;
default:
Status = EFI_UNSUPPORTED;
goto ON_EXIT;
}
@@ -1539,24 +1545,28 @@ ParseDnsResponse (
}
}
}
//
- // Parsing is complete, SignalEvent here.
+ // Parsing is complete, free the sending packet and signal Event here.
//
+ if (Item != NULL && Item->Value != NULL) {
+ NetbufFree ((NET_BUF *) (Item->Value));
+ }
+
if (Instance->Service->IpVersion == IP_VERSION_4) {
ASSERT (Dns4TokenEntry != NULL);
Dns4RemoveTokenEntry (&Instance->Dns4TxTokens, Dns4TokenEntry);
- Dns4TokenEntry->Token->Status = EFI_SUCCESS;
+ Dns4TokenEntry->Token->Status = Status;
if (Dns4TokenEntry->Token->Event != NULL) {
gBS->SignalEvent (Dns4TokenEntry->Token->Event);
DispatchDpc ();
}
} else {
ASSERT (Dns6TokenEntry != NULL);
Dns6RemoveTokenEntry (&Instance->Dns6TxTokens, Dns6TokenEntry);
- Dns6TokenEntry->Token->Status = EFI_SUCCESS;
+ Dns6TokenEntry->Token->Status = Status;
if (Dns6TokenEntry->Token->Event != NULL) {
gBS->SignalEvent (Dns6TokenEntry->Token->Event);
DispatchDpc ();
}
}
--
1.9.5.msysgit.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [Patch] NetworkPkg/DnsDxe: Handle CNAME type responded from the name server
2016-09-06 3:39 ` [Patch] NetworkPkg/DnsDxe: Handle CNAME type responded from the name server Jiaxin Wu
@ 2016-09-06 3:55 ` Fu, Siyuan
2016-09-06 7:05 ` Wu, Jiaxin
0 siblings, 1 reply; 8+ messages in thread
From: Fu, Siyuan @ 2016-09-06 3:55 UTC (permalink / raw)
To: Wu, Jiaxin, edk2-devel@lists.01.org; +Cc: Hegde Nagaraj P, Ye, Ting
Hi, Jiaxin
I have 2 comments for the patch.
1. I think we can add the CNAME record to the DNS cache, just like the A and AAAA records, so the driver won't need to repeat the query if user query the host again.
2. We should hold the original query packet until the response is parsed successfully, otherwise the system will crash again if a retransmit is needed.
Best Regards
Siyuan
> -----Original Message-----
> From: Wu, Jiaxin
> Sent: Tuesday, September 6, 2016 11:39 AM
> To: edk2-devel@lists.01.org
> Cc: Hegde Nagaraj P <nagaraj-p.hegde@hpe.com>; Fu, Siyuan
> <siyuan.fu@intel.com>; Ye, Ting <ting.ye@intel.com>
> Subject: [Patch] NetworkPkg/DnsDxe: Handle CNAME type responded from the
> name server
>
> According RFC 1034 - 3.6.2, if the query name is an alias, the name server
> will include the CNAME record in the response and restart the query at the
> domain name specified in the data field of the CNAME record. RFC also
> provides
> one example server action when A query received:
>
> Suppose a name server was processing a query with for USCISIC.ARPA, asking
> for
> type A information, and had the following resource records:
> USC-ISIC.ARPA IN CNAME C.ISI.EDU
> C.ISI.EDU IN A 10.0.0.52
> Both of these RRs would be returned in the response to the type A query.
>
> Currently, DnsDxe driver doesn't handle the CNAME type response, which
> will cause
> any exception result. The driver need continue the packet parsing while
> CNAME type
> record parsed. So, this patch is used to handle it correctly.
>
> Cc: Hegde Nagaraj P <nagaraj-p.hegde@hpe.com>
> Cc: Fu Siyuan <siyuan.fu@intel.com>
> Cc: Ye Ting <ting.ye@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jiaxin Wu <jiaxin.wu@intel.com>
> ---
> NetworkPkg/DnsDxe/DnsImpl.c | 60 ++++++++++++++++++++++++++--------------
> -----
> 1 file changed, 35 insertions(+), 25 deletions(-)
>
> diff --git a/NetworkPkg/DnsDxe/DnsImpl.c b/NetworkPkg/DnsDxe/DnsImpl.c
> index 360f68e..c68ec88 100644
> --- a/NetworkPkg/DnsDxe/DnsImpl.c
> +++ b/NetworkPkg/DnsDxe/DnsImpl.c
> @@ -1231,17 +1231,10 @@ ParseDnsResponse (
> if (DnsHeader->Flags.Bits.RCode != DNS_FLAGS_RCODE_NO_ERROR ||
> DnsHeader->AnswersNum < 1 || \
> DnsHeader->Flags.Bits.QR != DNS_FLAGS_QR_RESPONSE) {
> Status = EFI_ABORTED;
> goto ON_EXIT;
> }
> -
> - //
> - // Free the sending packet.
> - //
> - if (Item->Value != NULL) {
> - NetbufFree ((NET_BUF *) (Item->Value));
> - }
>
> //
> // Do some buffer allocations.
> //
> if (Instance->Service->IpVersion == IP_VERSION_4) {
> @@ -1288,40 +1281,42 @@ ParseDnsResponse (
> //
> // It's the GeneralLookUp querying.
> //
> Dns6TokenEntry->Token->RspData.GLookupData = AllocatePool (sizeof
> (DNS_RESOURCE_RECORD));
> if (Dns6TokenEntry->Token->RspData.GLookupData == NULL) {
> - Status = EFI_UNSUPPORTED;
> + Status = EFI_OUT_OF_RESOURCES;
> goto ON_EXIT;
> }
> Dns6TokenEntry->Token->RspData.GLookupData->RRList = AllocatePool
> (DnsHeader->AnswersNum * sizeof (DNS_RESOURCE_RECORD));
> if (Dns6TokenEntry->Token->RspData.GLookupData->RRList == NULL) {
> - Status = EFI_UNSUPPORTED;
> + Status = EFI_OUT_OF_RESOURCES;
> goto ON_EXIT;
> }
> } else {
> //
> // It's not the GeneralLookUp querying. Check the Query type.
> //
> if (QuerySection->Type == DNS_TYPE_AAAA) {
> Dns6TokenEntry->Token->RspData.H2AData = AllocatePool (sizeof
> (DNS6_HOST_TO_ADDR_DATA));
> if (Dns6TokenEntry->Token->RspData.H2AData == NULL) {
> - Status = EFI_UNSUPPORTED;
> + Status = EFI_OUT_OF_RESOURCES;
> goto ON_EXIT;
> }
> Dns6TokenEntry->Token->RspData.H2AData->IpList = AllocatePool
> (DnsHeader->AnswersNum * sizeof (EFI_IPv6_ADDRESS));
> if (Dns6TokenEntry->Token->RspData.H2AData->IpList == NULL) {
> - Status = EFI_UNSUPPORTED;
> + Status = EFI_OUT_OF_RESOURCES;
> goto ON_EXIT;
> }
> } else {
> Status = EFI_UNSUPPORTED;
> goto ON_EXIT;
> }
> }
> }
>
> + Status = EFI_NOT_FOUND;
> +
> //
> // Processing AnswerSection.
> //
> while (AnswerSectionNum < DnsHeader->AnswersNum) {
> //
> @@ -1348,51 +1343,53 @@ ParseDnsResponse (
> //
> // Fill the ResourceRecord.
> //
> Dns4RR[RRCount].QName = AllocateZeroPool (AsciiStrLen (QueryName) +
> 1);
> if (Dns4RR[RRCount].QName == NULL) {
> - Status = EFI_UNSUPPORTED;
> + Status = EFI_OUT_OF_RESOURCES;
> goto ON_EXIT;
> }
> CopyMem (Dns4RR[RRCount].QName, QueryName, AsciiStrLen (QueryName));
> Dns4RR[RRCount].QType = AnswerSection->Type;
> Dns4RR[RRCount].QClass = AnswerSection->Class;
> Dns4RR[RRCount].TTL = AnswerSection->Ttl;
> Dns4RR[RRCount].DataLength = AnswerSection->DataLength;
> Dns4RR[RRCount].RData = AllocateZeroPool
> (Dns4RR[RRCount].DataLength);
> if (Dns4RR[RRCount].RData == NULL) {
> - Status = EFI_UNSUPPORTED;
> + Status = EFI_OUT_OF_RESOURCES;
> goto ON_EXIT;
> }
> CopyMem (Dns4RR[RRCount].RData, AnswerData,
> Dns4RR[RRCount].DataLength);
>
> RRCount ++;
> + Status = EFI_SUCCESS;
> } else if (Instance->Service->IpVersion == IP_VERSION_6 &&
> Dns6TokenEntry->GeneralLookUp) {
> Dns6RR = Dns6TokenEntry->Token->RspData.GLookupData->RRList;
> AnswerData = (UINT8 *) AnswerSection + sizeof (*AnswerSection);
>
> //
> // Fill the ResourceRecord.
> //
> Dns6RR[RRCount].QName = AllocateZeroPool (AsciiStrLen (QueryName) +
> 1);
> if (Dns6RR[RRCount].QName == NULL) {
> - Status = EFI_UNSUPPORTED;
> + Status = EFI_OUT_OF_RESOURCES;
> goto ON_EXIT;
> }
> CopyMem (Dns6RR[RRCount].QName, QueryName, AsciiStrLen (QueryName));
> Dns6RR[RRCount].QType = AnswerSection->Type;
> Dns6RR[RRCount].QClass = AnswerSection->Class;
> Dns6RR[RRCount].TTL = AnswerSection->Ttl;
> Dns6RR[RRCount].DataLength = AnswerSection->DataLength;
> Dns6RR[RRCount].RData = AllocateZeroPool
> (Dns6RR[RRCount].DataLength);
> if (Dns6RR[RRCount].RData == NULL) {
> - Status = EFI_UNSUPPORTED;
> + Status = EFI_OUT_OF_RESOURCES;
> goto ON_EXIT;
> }
> CopyMem (Dns6RR[RRCount].RData, AnswerData,
> Dns6RR[RRCount].DataLength);
>
> RRCount ++;
> + Status = EFI_SUCCESS;
> } else {
> //
> // It's not the GeneralLookUp querying.
> // Check the Query type, parse the response packet.
> //
> @@ -1425,30 +1422,31 @@ ParseDnsResponse (
> //
> // Allocate new CacheEntry pool.
> //
> Dns4CacheEntry = AllocateZeroPool (sizeof (EFI_DNS4_CACHE_ENTRY));
> if (Dns4CacheEntry == NULL) {
> - Status = EFI_UNSUPPORTED;
> + Status = EFI_OUT_OF_RESOURCES;
> goto ON_EXIT;
> }
> Dns4CacheEntry->HostName = AllocateZeroPool (2 *
> (StrLen(Dns4TokenEntry->QueryHostName) + 1));
> if (Dns4CacheEntry->HostName == NULL) {
> - Status = EFI_UNSUPPORTED;
> + Status = EFI_OUT_OF_RESOURCES;
> goto ON_EXIT;
> }
> CopyMem (Dns4CacheEntry->HostName, Dns4TokenEntry->QueryHostName,
> 2 * (StrLen(Dns4TokenEntry->QueryHostName) + 1));
> Dns4CacheEntry->IpAddress = AllocateZeroPool (sizeof
> (EFI_IPv4_ADDRESS));
> if (Dns4CacheEntry->IpAddress == NULL) {
> - Status = EFI_UNSUPPORTED;
> + Status = EFI_OUT_OF_RESOURCES;
> goto ON_EXIT;
> }
> CopyMem (Dns4CacheEntry->IpAddress, AnswerData, sizeof
> (EFI_IPv4_ADDRESS));
> Dns4CacheEntry->Timeout = AnswerSection->Ttl;
>
> UpdateDns4Cache (&mDriverData->Dns4CacheList, FALSE, TRUE,
> *Dns4CacheEntry);
>
> - IpCount ++;
> + IpCount ++;
> + Status = EFI_SUCCESS;
> break;
> case DNS_TYPE_AAAA:
> //
> // This is address entry, get Data.
> //
> @@ -1476,30 +1474,38 @@ ParseDnsResponse (
> //
> // Allocate new CacheEntry pool.
> //
> Dns6CacheEntry = AllocateZeroPool (sizeof (EFI_DNS6_CACHE_ENTRY));
> if (Dns6CacheEntry == NULL) {
> - Status = EFI_UNSUPPORTED;
> + Status = EFI_OUT_OF_RESOURCES;
> goto ON_EXIT;
> }
> Dns6CacheEntry->HostName = AllocateZeroPool (2 *
> (StrLen(Dns6TokenEntry->QueryHostName) + 1));
> if (Dns6CacheEntry->HostName == NULL) {
> - Status = EFI_UNSUPPORTED;
> + Status = EFI_OUT_OF_RESOURCES;
> goto ON_EXIT;
> }
> CopyMem (Dns6CacheEntry->HostName, Dns6TokenEntry->QueryHostName,
> 2 * (StrLen(Dns6TokenEntry->QueryHostName) + 1));
> Dns6CacheEntry->IpAddress = AllocateZeroPool (sizeof
> (EFI_IPv6_ADDRESS));
> if (Dns6CacheEntry->IpAddress == NULL) {
> - Status = EFI_UNSUPPORTED;
> + Status = EFI_OUT_OF_RESOURCES;
> goto ON_EXIT;
> }
> CopyMem (Dns6CacheEntry->IpAddress, AnswerData, sizeof
> (EFI_IPv6_ADDRESS));
> Dns6CacheEntry->Timeout = AnswerSection->Ttl;
>
> UpdateDns6Cache (&mDriverData->Dns6CacheList, FALSE, TRUE,
> *Dns6CacheEntry);
>
> IpCount ++;
> + Status = EFI_SUCCESS;
> + break;
> + case DNS_TYPE_CNAME:
> + //
> + // According RFC 1034 - 3.6.2, if the query name is an alias, the
> name server will include the CNAME
> + // record in the response and restart the query at the domain
> name specified in the data field of the
> + // CNAME record. So, let's skip this CNAME record parsing and
> continue the next one.
> + //
> break;
> default:
> Status = EFI_UNSUPPORTED;
> goto ON_EXIT;
> }
> @@ -1539,24 +1545,28 @@ ParseDnsResponse (
> }
> }
> }
>
> //
> - // Parsing is complete, SignalEvent here.
> + // Parsing is complete, free the sending packet and signal Event here.
> //
> + if (Item != NULL && Item->Value != NULL) {
> + NetbufFree ((NET_BUF *) (Item->Value));
> + }
> +
> if (Instance->Service->IpVersion == IP_VERSION_4) {
> ASSERT (Dns4TokenEntry != NULL);
> Dns4RemoveTokenEntry (&Instance->Dns4TxTokens, Dns4TokenEntry);
> - Dns4TokenEntry->Token->Status = EFI_SUCCESS;
> + Dns4TokenEntry->Token->Status = Status;
> if (Dns4TokenEntry->Token->Event != NULL) {
> gBS->SignalEvent (Dns4TokenEntry->Token->Event);
> DispatchDpc ();
> }
> } else {
> ASSERT (Dns6TokenEntry != NULL);
> Dns6RemoveTokenEntry (&Instance->Dns6TxTokens, Dns6TokenEntry);
> - Dns6TokenEntry->Token->Status = EFI_SUCCESS;
> + Dns6TokenEntry->Token->Status = Status;
> if (Dns6TokenEntry->Token->Event != NULL) {
> gBS->SignalEvent (Dns6TokenEntry->Token->Event);
> DispatchDpc ();
> }
> }
> --
> 1.9.5.msysgit.1
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Patch] NetworkPkg/IpSecDxe: Generate SPI randomly and correct IKE_SPI_BASE value
2016-09-06 3:39 [Patch] NetworkPkg/IpSecDxe: Generate SPI randomly and correct IKE_SPI_BASE value Jiaxin Wu
2016-09-06 3:39 ` [Patch] NetworkPkg/DnsDxe: Handle CNAME type responded from the name server Jiaxin Wu
@ 2016-09-06 3:55 ` Fu, Siyuan
2016-09-06 6:37 ` Ye, Ting
2 siblings, 0 replies; 8+ messages in thread
From: Fu, Siyuan @ 2016-09-06 3:55 UTC (permalink / raw)
To: Wu, Jiaxin, edk2-devel@lists.01.org; +Cc: Ye, Ting
Reviewed-by: Fu Siyuan <siyuan.fu@intel.com>
> -----Original Message-----
> From: Wu, Jiaxin
> Sent: Tuesday, September 6, 2016 11:39 AM
> To: edk2-devel@lists.01.org
> Cc: Ye, Ting <ting.ye@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>
> Subject: [Patch] NetworkPkg/IpSecDxe: Generate SPI randomly and correct
> IKE_SPI_BASE value
>
> This path made the following update:
> * Generate SPI randomly.
> * Correct IKE_SPI_BASE value according RFC 4302/4303.
>
> Cc: Ye Ting <ting.ye@intel.com>
> Cc: Fu Siyuan <siyuan.fu@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jiaxin Wu <jiaxin.wu@intel.com>
> ---
> NetworkPkg/IpSecDxe/IkeCommon.c | 102
> +++++++++++++++++++++++++++++++-----
> NetworkPkg/IpSecDxe/IkeCommon.h | 20 ++++---
> NetworkPkg/IpSecDxe/Ikev2/Utility.c | 11 +++-
> 3 files changed, 112 insertions(+), 21 deletions(-)
>
> diff --git a/NetworkPkg/IpSecDxe/IkeCommon.c
> b/NetworkPkg/IpSecDxe/IkeCommon.c
> index 6fc7c06..b1e4321 100644
> --- a/NetworkPkg/IpSecDxe/IkeCommon.c
> +++ b/NetworkPkg/IpSecDxe/IkeCommon.c
> @@ -1,9 +1,9 @@
> /** @file
> Common operation of the IKE
>
> - Copyright (c) 2010 - 2015, Intel Corporation. All rights reserved.<BR>
> + Copyright (c) 2010 - 2016, Intel Corporation. All rights reserved.<BR>
>
> This program and the accompanying materials
> are licensed and made available under the terms and conditions of the
> BSD License
> which accompanies this distribution. The full text of the license may
> be found at
> http://opensource.org/licenses/bsd-license.php.
> @@ -16,14 +16,56 @@
> #include "Ike.h"
> #include "IkeCommon.h"
> #include "IpSecConfigImpl.h"
> #include "IpSecDebug.h"
>
> -//
> -// Initial the SPI
> -//
> -UINT32 mNextSpi = IKE_SPI_BASE;
> +/**
> + Check whether the new generated Spi has existed.
> +
> + @param[in] IkeSaSession Pointer to the Child SA Session.
> + @param[in] SpiValue SPI Value.
> +
> + @retval TRUE This SpiValue has existed in the Child SA Session
> + @retval FALSE This SpiValue doesn't exist in the Child SA Session.
> +
> +**/
> +BOOLEAN
> +IkeSpiValueExisted (
> + IN IKEV2_SA_SESSION *IkeSaSession,
> + IN UINT32 SpiValue
> + )
> +{
> + LIST_ENTRY *Entry;
> + LIST_ENTRY *Next;
> + IKEV2_CHILD_SA_SESSION *SaSession;
> +
> + Entry = NULL;
> + Next = NULL;
> + SaSession = NULL;
> +
> + //
> + // Check whether the SPI value has existed in
> ChildSaEstablishSessionList.
> + //
> + NET_LIST_FOR_EACH_SAFE (Entry, Next, &IkeSaSession-
> >ChildSaEstablishSessionList) {
> + SaSession= IKEV2_CHILD_SA_SESSION_BY_IKE_SA (Entry);
> + if (SaSession->LocalPeerSpi == SpiValue) {
> + return TRUE;
> + }
> + }
> +
> + //
> + // Check whether the SPI value has existed in ChildSaSessionList.
> + //
> + NET_LIST_FOR_EACH_SAFE (Entry, Next, &IkeSaSession->ChildSaSessionList)
> {
> + SaSession= IKEV2_CHILD_SA_SESSION_BY_IKE_SA (Entry);
> + if (SaSession->LocalPeerSpi == SpiValue) {
> + return TRUE;
> + }
> + }
> +
> + return FALSE;
> +}
>
> /**
> Call Crypto Lib to generate a random value with eight-octet length.
>
> @return the 64 byte vaule.
> @@ -156,23 +198,57 @@ IkePayloadFree (
> FreePool (IkePayload);
> }
>
> /**
> Generate an new SPI.
> -
> - @return a SPI in 4 bytes.
> +
> + @param[in] IkeSaSession Pointer to IKEV2_SA_SESSION related to
> this Child SA
> + Session.
> + @param[in out] SpiValue Pointer to the new generated SPI value.
> +
> + @retval EFI_SUCCESS The operation performs successfully.
> + @retval Otherwise The operation is failed.
>
> **/
> -UINT32
> +EFI_STATUS
> IkeGenerateSpi (
> - VOID
> + IN IKEV2_SA_SESSION *IkeSaSession,
> + OUT UINT32 *SpiValue
> )
> {
> - //
> - // TODO: should generate SPI randomly to avoid security issue
> - //
> - return mNextSpi++;
> + EFI_STATUS Status;
> +
> + Status = EFI_SUCCESS;
> +
> + while (TRUE) {
> + //
> + // Generate SPI randomly
> + //
> + Status = IpSecCryptoIoGenerateRandomBytes ((UINT8 *)SpiValue, sizeof
> (UINT32));
> + if (EFI_ERROR (Status)) {
> + break;
> + }
> +
> + //
> + // The set of SPI values in the range 1 through 255 are reserved by
> the
> + // Internet Assigned Numbers Authority (IANA) for future use; a
> reserved
> + // SPI value will not normally be assigned by IANA unless the use of
> the
> + // assigned SPI value is specified in an RFC.
> + //
> + if (*SpiValue < IKE_SPI_BASE) {
> + *SpiValue += IKE_SPI_BASE;
> + }
> +
> + //
> + // Check whether the new generated SPI has existed.
> + //
> + if (!IkeSpiValueExisted (IkeSaSession, *SpiValue)) {
> + break;
> + }
> + }
> +
> + return Status;
> }
>
> /**
> Generate a random data for IV
>
> diff --git a/NetworkPkg/IpSecDxe/IkeCommon.h
> b/NetworkPkg/IpSecDxe/IkeCommon.h
> index 714ecaa..7f7fd4d 100644
> --- a/NetworkPkg/IpSecDxe/IkeCommon.h
> +++ b/NetworkPkg/IpSecDxe/IkeCommon.h
> @@ -1,9 +1,9 @@
> /** @file
> Common operation of the IKE.
>
> - Copyright (c) 2010 - 2015, Intel Corporation. All rights reserved.<BR>
> + Copyright (c) 2010 - 2016, Intel Corporation. All rights reserved.<BR>
>
> This program and the accompanying materials
> are licensed and made available under the terms and conditions of the
> BSD License
> which accompanies this distribution. The full text of the license may
> be found at
> http://opensource.org/licenses/bsd-license.php.
> @@ -37,11 +37,11 @@
>
> #define IKE_DEFAULT_PORT 500
> #define IKE_DEFAULT_TIMEOUT_INTERVAL 10000 // 10s
> #define IKE_NONCE_SIZE 16
> #define IKE_MAX_RETRY 4
> -#define IKE_SPI_BASE 0x10000
> +#define IKE_SPI_BASE 0x100
> #define IKE_PAYLOAD_SIGNATURE SIGNATURE_32('I','K','E','P')
> #define IKE_PAYLOAD_BY_PACKET(a)
> CR(a,IKE_PAYLOAD,ByPacket,IKE_PAYLOAD_SIGNATURE)
>
>
> #define IKE_PACKET_APPEND_PAYLOAD(IkePacket,IkePayload) \
> @@ -128,18 +128,24 @@ VOID
> IkePayloadFree (
> IN IKE_PAYLOAD *IkePayload
> );
>
> /**
> - Generate an unused SPI
> -
> - @return a SPI in 4 bytes.
> + Generate an new SPI.
> +
> + @param[in] IkeSaSession Pointer to IKEV2_SA_SESSION related to
> this Child SA
> + Session.
> + @param[in out] SpiValue Pointer to the new generated SPI value.
> +
> + @retval EFI_SUCCESS The operation performs successfully.
> + @retval Otherwise The operation is failed.
>
> **/
> -UINT32
> +EFI_STATUS
> IkeGenerateSpi (
> - VOID
> + IN IKEV2_SA_SESSION *IkeSaSession,
> + OUT UINT32 *SpiValue
> );
>
> /**
> Generate a random data for IV
>
> diff --git a/NetworkPkg/IpSecDxe/Ikev2/Utility.c
> b/NetworkPkg/IpSecDxe/Ikev2/Utility.c
> index 5b26ba1..c365532 100644
> --- a/NetworkPkg/IpSecDxe/Ikev2/Utility.c
> +++ b/NetworkPkg/IpSecDxe/Ikev2/Utility.c
> @@ -523,11 +523,20 @@ Ikev2ChildSaSessionAlloc (
> // Initialize the fields of ChildSaSession and its SessionCommon.
> //
> ChildSaSession->Signature = IKEV2_CHILD_SA_SESSION_SIGNATURE;
> ChildSaSession->IkeSaSession = IkeSaSession;
> ChildSaSession->MessageId = IkeSaSession->MessageId;
> - ChildSaSession->LocalPeerSpi = IkeGenerateSpi ();
> +
> + //
> + // Generate an new SPI.
> + //
> + Status = IkeGenerateSpi (IkeSaSession, &(ChildSaSession->LocalPeerSpi));
> + if (EFI_ERROR (Status)) {
> + FreePool (ChildSaSession);
> + return NULL;
> + }
> +
> ChildSaCommon = &ChildSaSession->SessionCommon;
> ChildSaCommon->UdpService = UdpService;
> ChildSaCommon->Private = IkeSaSession-
> >SessionCommon.Private;
> ChildSaCommon->IkeSessionType = IkeSessionTypeChildSa;
> ChildSaCommon->IkeVer = 2;
> --
> 1.9.5.msysgit.1
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Patch] NetworkPkg/IpSecDxe: Generate SPI randomly and correct IKE_SPI_BASE value
2016-09-06 3:39 [Patch] NetworkPkg/IpSecDxe: Generate SPI randomly and correct IKE_SPI_BASE value Jiaxin Wu
2016-09-06 3:39 ` [Patch] NetworkPkg/DnsDxe: Handle CNAME type responded from the name server Jiaxin Wu
2016-09-06 3:55 ` [Patch] NetworkPkg/IpSecDxe: Generate SPI randomly and correct IKE_SPI_BASE value Fu, Siyuan
@ 2016-09-06 6:37 ` Ye, Ting
2 siblings, 0 replies; 8+ messages in thread
From: Ye, Ting @ 2016-09-06 6:37 UTC (permalink / raw)
To: Wu, Jiaxin, edk2-devel@lists.01.org; +Cc: Fu, Siyuan
Reviewed-by: Ye Ting <ting.ye@intel.com>
-----Original Message-----
From: Wu, Jiaxin
Sent: Tuesday, September 06, 2016 11:39 AM
To: edk2-devel@lists.01.org
Cc: Ye, Ting <ting.ye@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>
Subject: [Patch] NetworkPkg/IpSecDxe: Generate SPI randomly and correct IKE_SPI_BASE value
This path made the following update:
* Generate SPI randomly.
* Correct IKE_SPI_BASE value according RFC 4302/4303.
Cc: Ye Ting <ting.ye@intel.com>
Cc: Fu Siyuan <siyuan.fu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiaxin Wu <jiaxin.wu@intel.com>
---
NetworkPkg/IpSecDxe/IkeCommon.c | 102 +++++++++++++++++++++++++++++++-----
NetworkPkg/IpSecDxe/IkeCommon.h | 20 ++++---
NetworkPkg/IpSecDxe/Ikev2/Utility.c | 11 +++-
3 files changed, 112 insertions(+), 21 deletions(-)
diff --git a/NetworkPkg/IpSecDxe/IkeCommon.c b/NetworkPkg/IpSecDxe/IkeCommon.c index 6fc7c06..b1e4321 100644
--- a/NetworkPkg/IpSecDxe/IkeCommon.c
+++ b/NetworkPkg/IpSecDxe/IkeCommon.c
@@ -1,9 +1,9 @@
/** @file
Common operation of the IKE
- Copyright (c) 2010 - 2015, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2010 - 2016, Intel Corporation. All rights
+ reserved.<BR>
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD License
which accompanies this distribution. The full text of the license may be found at
http://opensource.org/licenses/bsd-license.php.
@@ -16,14 +16,56 @@
#include "Ike.h"
#include "IkeCommon.h"
#include "IpSecConfigImpl.h"
#include "IpSecDebug.h"
-//
-// Initial the SPI
-//
-UINT32 mNextSpi = IKE_SPI_BASE;
+/**
+ Check whether the new generated Spi has existed.
+
+ @param[in] IkeSaSession Pointer to the Child SA Session.
+ @param[in] SpiValue SPI Value.
+
+ @retval TRUE This SpiValue has existed in the Child SA Session
+ @retval FALSE This SpiValue doesn't exist in the Child SA Session.
+
+**/
+BOOLEAN
+IkeSpiValueExisted (
+ IN IKEV2_SA_SESSION *IkeSaSession,
+ IN UINT32 SpiValue
+ )
+{
+ LIST_ENTRY *Entry;
+ LIST_ENTRY *Next;
+ IKEV2_CHILD_SA_SESSION *SaSession;
+
+ Entry = NULL;
+ Next = NULL;
+ SaSession = NULL;
+
+ //
+ // Check whether the SPI value has existed in ChildSaEstablishSessionList.
+ //
+ NET_LIST_FOR_EACH_SAFE (Entry, Next, &IkeSaSession->ChildSaEstablishSessionList) {
+ SaSession= IKEV2_CHILD_SA_SESSION_BY_IKE_SA (Entry);
+ if (SaSession->LocalPeerSpi == SpiValue) {
+ return TRUE;
+ }
+ }
+
+ //
+ // Check whether the SPI value has existed in ChildSaSessionList.
+ //
+ NET_LIST_FOR_EACH_SAFE (Entry, Next, &IkeSaSession->ChildSaSessionList) {
+ SaSession= IKEV2_CHILD_SA_SESSION_BY_IKE_SA (Entry);
+ if (SaSession->LocalPeerSpi == SpiValue) {
+ return TRUE;
+ }
+ }
+
+ return FALSE;
+}
/**
Call Crypto Lib to generate a random value with eight-octet length.
@return the 64 byte vaule.
@@ -156,23 +198,57 @@ IkePayloadFree (
FreePool (IkePayload);
}
/**
Generate an new SPI.
-
- @return a SPI in 4 bytes.
+
+ @param[in] IkeSaSession Pointer to IKEV2_SA_SESSION related to this Child SA
+ Session.
+ @param[in out] SpiValue Pointer to the new generated SPI value.
+
+ @retval EFI_SUCCESS The operation performs successfully.
+ @retval Otherwise The operation is failed.
**/
-UINT32
+EFI_STATUS
IkeGenerateSpi (
- VOID
+ IN IKEV2_SA_SESSION *IkeSaSession,
+ OUT UINT32 *SpiValue
)
{
- //
- // TODO: should generate SPI randomly to avoid security issue
- //
- return mNextSpi++;
+ EFI_STATUS Status;
+
+ Status = EFI_SUCCESS;
+
+ while (TRUE) {
+ //
+ // Generate SPI randomly
+ //
+ Status = IpSecCryptoIoGenerateRandomBytes ((UINT8 *)SpiValue, sizeof (UINT32));
+ if (EFI_ERROR (Status)) {
+ break;
+ }
+
+ //
+ // The set of SPI values in the range 1 through 255 are reserved by the
+ // Internet Assigned Numbers Authority (IANA) for future use; a reserved
+ // SPI value will not normally be assigned by IANA unless the use of the
+ // assigned SPI value is specified in an RFC.
+ //
+ if (*SpiValue < IKE_SPI_BASE) {
+ *SpiValue += IKE_SPI_BASE;
+ }
+
+ //
+ // Check whether the new generated SPI has existed.
+ //
+ if (!IkeSpiValueExisted (IkeSaSession, *SpiValue)) {
+ break;
+ }
+ }
+
+ return Status;
}
/**
Generate a random data for IV
diff --git a/NetworkPkg/IpSecDxe/IkeCommon.h b/NetworkPkg/IpSecDxe/IkeCommon.h index 714ecaa..7f7fd4d 100644
--- a/NetworkPkg/IpSecDxe/IkeCommon.h
+++ b/NetworkPkg/IpSecDxe/IkeCommon.h
@@ -1,9 +1,9 @@
/** @file
Common operation of the IKE.
- Copyright (c) 2010 - 2015, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2010 - 2016, Intel Corporation. All rights
+ reserved.<BR>
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD License
which accompanies this distribution. The full text of the license may be found at
http://opensource.org/licenses/bsd-license.php.
@@ -37,11 +37,11 @@
#define IKE_DEFAULT_PORT 500
#define IKE_DEFAULT_TIMEOUT_INTERVAL 10000 // 10s
#define IKE_NONCE_SIZE 16
#define IKE_MAX_RETRY 4
-#define IKE_SPI_BASE 0x10000
+#define IKE_SPI_BASE 0x100
#define IKE_PAYLOAD_SIGNATURE SIGNATURE_32('I','K','E','P')
#define IKE_PAYLOAD_BY_PACKET(a) CR(a,IKE_PAYLOAD,ByPacket,IKE_PAYLOAD_SIGNATURE)
#define IKE_PACKET_APPEND_PAYLOAD(IkePacket,IkePayload) \
@@ -128,18 +128,24 @@ VOID
IkePayloadFree (
IN IKE_PAYLOAD *IkePayload
);
/**
- Generate an unused SPI
-
- @return a SPI in 4 bytes.
+ Generate an new SPI.
+
+ @param[in] IkeSaSession Pointer to IKEV2_SA_SESSION related to this Child SA
+ Session.
+ @param[in out] SpiValue Pointer to the new generated SPI value.
+
+ @retval EFI_SUCCESS The operation performs successfully.
+ @retval Otherwise The operation is failed.
**/
-UINT32
+EFI_STATUS
IkeGenerateSpi (
- VOID
+ IN IKEV2_SA_SESSION *IkeSaSession,
+ OUT UINT32 *SpiValue
);
/**
Generate a random data for IV
diff --git a/NetworkPkg/IpSecDxe/Ikev2/Utility.c b/NetworkPkg/IpSecDxe/Ikev2/Utility.c
index 5b26ba1..c365532 100644
--- a/NetworkPkg/IpSecDxe/Ikev2/Utility.c
+++ b/NetworkPkg/IpSecDxe/Ikev2/Utility.c
@@ -523,11 +523,20 @@ Ikev2ChildSaSessionAlloc (
// Initialize the fields of ChildSaSession and its SessionCommon.
//
ChildSaSession->Signature = IKEV2_CHILD_SA_SESSION_SIGNATURE;
ChildSaSession->IkeSaSession = IkeSaSession;
ChildSaSession->MessageId = IkeSaSession->MessageId;
- ChildSaSession->LocalPeerSpi = IkeGenerateSpi ();
+
+ //
+ // Generate an new SPI.
+ //
+ Status = IkeGenerateSpi (IkeSaSession,
+ &(ChildSaSession->LocalPeerSpi)); if (EFI_ERROR (Status)) {
+ FreePool (ChildSaSession);
+ return NULL;
+ }
+
ChildSaCommon = &ChildSaSession->SessionCommon;
ChildSaCommon->UdpService = UdpService;
ChildSaCommon->Private = IkeSaSession->SessionCommon.Private;
ChildSaCommon->IkeSessionType = IkeSessionTypeChildSa;
ChildSaCommon->IkeVer = 2;
--
1.9.5.msysgit.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [Patch] NetworkPkg/DnsDxe: Handle CNAME type responded from the name server
2016-09-06 3:55 ` Fu, Siyuan
@ 2016-09-06 7:05 ` Wu, Jiaxin
2016-09-06 7:13 ` Fu, Siyuan
0 siblings, 1 reply; 8+ messages in thread
From: Wu, Jiaxin @ 2016-09-06 7:05 UTC (permalink / raw)
To: Fu, Siyuan, edk2-devel@lists.01.org; +Cc: Hegde Nagaraj P, Ye, Ting
Hi Siyuan,
> I have 2 comments for the patch.
> 1. I think we can add the CNAME record to the DNS cache, just like the A and
> AAAA records, so the driver won't need to repeat the query if user query the
> host again.
In my opinion , the CNAME is unnecessary to cache to avoid the repeat the query. Because if DnsCache is enabled, HostName and the corresponding IpAddress will be retrieved directly from EFI_DNS4_CACHE_ENTRY.
> 2. We should hold the original query packet until the response is parsed
> successfully, otherwise the system will crash again if a retransmit is needed.
This patch already did the update to avoid the crash issue during retransmit by removing the below code after the parsing is complete.
//
// Parsing is complete, free the sending packet and signal Event here.
//
if (Item != NULL && Item->Value != NULL) {
NetbufFree ((NET_BUF *) (Item->Value));
}
So, the original query packet is safe until the response is parsed.
>
> Best Regards
> Siyuan
>
>
Thanks,
Jiaxin
> > -----Original Message-----
> > From: Wu, Jiaxin
> > Sent: Tuesday, September 6, 2016 11:39 AM
> > To: edk2-devel@lists.01.org
> > Cc: Hegde Nagaraj P <nagaraj-p.hegde@hpe.com>; Fu, Siyuan
> > <siyuan.fu@intel.com>; Ye, Ting <ting.ye@intel.com>
> > Subject: [Patch] NetworkPkg/DnsDxe: Handle CNAME type responded from
> > the name server
> >
> > According RFC 1034 - 3.6.2, if the query name is an alias, the name
> > server will include the CNAME record in the response and restart the
> > query at the domain name specified in the data field of the CNAME
> > record. RFC also provides one example server action when A query
> > received:
> >
> > Suppose a name server was processing a query with for USCISIC.ARPA,
> > asking for type A information, and had the following resource records:
> > USC-ISIC.ARPA IN CNAME C.ISI.EDU
> > C.ISI.EDU IN A 10.0.0.52
> > Both of these RRs would be returned in the response to the type A query.
> >
> > Currently, DnsDxe driver doesn't handle the CNAME type response, which
> > will cause any exception result. The driver need continue the packet
> > parsing while CNAME type record parsed. So, this patch is used to
> > handle it correctly.
> >
> > Cc: Hegde Nagaraj P <nagaraj-p.hegde@hpe.com>
> > Cc: Fu Siyuan <siyuan.fu@intel.com>
> > Cc: Ye Ting <ting.ye@intel.com>
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Jiaxin Wu <jiaxin.wu@intel.com>
> > ---
> > NetworkPkg/DnsDxe/DnsImpl.c | 60
> > ++++++++++++++++++++++++++--------------
> > -----
> > 1 file changed, 35 insertions(+), 25 deletions(-)
> >
> > diff --git a/NetworkPkg/DnsDxe/DnsImpl.c
> b/NetworkPkg/DnsDxe/DnsImpl.c
> > index 360f68e..c68ec88 100644
> > --- a/NetworkPkg/DnsDxe/DnsImpl.c
> > +++ b/NetworkPkg/DnsDxe/DnsImpl.c
> > @@ -1231,17 +1231,10 @@ ParseDnsResponse (
> > if (DnsHeader->Flags.Bits.RCode != DNS_FLAGS_RCODE_NO_ERROR ||
> > DnsHeader->AnswersNum < 1 || \
> > DnsHeader->Flags.Bits.QR != DNS_FLAGS_QR_RESPONSE) {
> > Status = EFI_ABORTED;
> > goto ON_EXIT;
> > }
> > -
> > - //
> > - // Free the sending packet.
> > - //
> > - if (Item->Value != NULL) {
> > - NetbufFree ((NET_BUF *) (Item->Value));
> > - }
> >
> > //
> > // Do some buffer allocations.
> > //
> > if (Instance->Service->IpVersion == IP_VERSION_4) { @@ -1288,40
> > +1281,42 @@ ParseDnsResponse (
> > //
> > // It's the GeneralLookUp querying.
> > //
> > Dns6TokenEntry->Token->RspData.GLookupData = AllocatePool
> > (sizeof (DNS_RESOURCE_RECORD));
> > if (Dns6TokenEntry->Token->RspData.GLookupData == NULL) {
> > - Status = EFI_UNSUPPORTED;
> > + Status = EFI_OUT_OF_RESOURCES;
> > goto ON_EXIT;
> > }
> > Dns6TokenEntry->Token->RspData.GLookupData->RRList =
> > AllocatePool (DnsHeader->AnswersNum * sizeof
> (DNS_RESOURCE_RECORD));
> > if (Dns6TokenEntry->Token->RspData.GLookupData->RRList == NULL) {
> > - Status = EFI_UNSUPPORTED;
> > + Status = EFI_OUT_OF_RESOURCES;
> > goto ON_EXIT;
> > }
> > } else {
> > //
> > // It's not the GeneralLookUp querying. Check the Query type.
> > //
> > if (QuerySection->Type == DNS_TYPE_AAAA) {
> > Dns6TokenEntry->Token->RspData.H2AData = AllocatePool (sizeof
> > (DNS6_HOST_TO_ADDR_DATA));
> > if (Dns6TokenEntry->Token->RspData.H2AData == NULL) {
> > - Status = EFI_UNSUPPORTED;
> > + Status = EFI_OUT_OF_RESOURCES;
> > goto ON_EXIT;
> > }
> > Dns6TokenEntry->Token->RspData.H2AData->IpList = AllocatePool
> > (DnsHeader->AnswersNum * sizeof (EFI_IPv6_ADDRESS));
> > if (Dns6TokenEntry->Token->RspData.H2AData->IpList == NULL) {
> > - Status = EFI_UNSUPPORTED;
> > + Status = EFI_OUT_OF_RESOURCES;
> > goto ON_EXIT;
> > }
> > } else {
> > Status = EFI_UNSUPPORTED;
> > goto ON_EXIT;
> > }
> > }
> > }
> >
> > + Status = EFI_NOT_FOUND;
> > +
> > //
> > // Processing AnswerSection.
> > //
> > while (AnswerSectionNum < DnsHeader->AnswersNum) {
> > //
> > @@ -1348,51 +1343,53 @@ ParseDnsResponse (
> > //
> > // Fill the ResourceRecord.
> > //
> > Dns4RR[RRCount].QName = AllocateZeroPool (AsciiStrLen
> > (QueryName) + 1);
> > if (Dns4RR[RRCount].QName == NULL) {
> > - Status = EFI_UNSUPPORTED;
> > + Status = EFI_OUT_OF_RESOURCES;
> > goto ON_EXIT;
> > }
> > CopyMem (Dns4RR[RRCount].QName, QueryName, AsciiStrLen
> (QueryName));
> > Dns4RR[RRCount].QType = AnswerSection->Type;
> > Dns4RR[RRCount].QClass = AnswerSection->Class;
> > Dns4RR[RRCount].TTL = AnswerSection->Ttl;
> > Dns4RR[RRCount].DataLength = AnswerSection->DataLength;
> > Dns4RR[RRCount].RData = AllocateZeroPool
> > (Dns4RR[RRCount].DataLength);
> > if (Dns4RR[RRCount].RData == NULL) {
> > - Status = EFI_UNSUPPORTED;
> > + Status = EFI_OUT_OF_RESOURCES;
> > goto ON_EXIT;
> > }
> > CopyMem (Dns4RR[RRCount].RData, AnswerData,
> > Dns4RR[RRCount].DataLength);
> >
> > RRCount ++;
> > + Status = EFI_SUCCESS;
> > } else if (Instance->Service->IpVersion == IP_VERSION_6 &&
> > Dns6TokenEntry->GeneralLookUp) {
> > Dns6RR = Dns6TokenEntry->Token->RspData.GLookupData->RRList;
> > AnswerData = (UINT8 *) AnswerSection + sizeof (*AnswerSection);
> >
> > //
> > // Fill the ResourceRecord.
> > //
> > Dns6RR[RRCount].QName = AllocateZeroPool (AsciiStrLen
> > (QueryName) + 1);
> > if (Dns6RR[RRCount].QName == NULL) {
> > - Status = EFI_UNSUPPORTED;
> > + Status = EFI_OUT_OF_RESOURCES;
> > goto ON_EXIT;
> > }
> > CopyMem (Dns6RR[RRCount].QName, QueryName, AsciiStrLen
> (QueryName));
> > Dns6RR[RRCount].QType = AnswerSection->Type;
> > Dns6RR[RRCount].QClass = AnswerSection->Class;
> > Dns6RR[RRCount].TTL = AnswerSection->Ttl;
> > Dns6RR[RRCount].DataLength = AnswerSection->DataLength;
> > Dns6RR[RRCount].RData = AllocateZeroPool
> > (Dns6RR[RRCount].DataLength);
> > if (Dns6RR[RRCount].RData == NULL) {
> > - Status = EFI_UNSUPPORTED;
> > + Status = EFI_OUT_OF_RESOURCES;
> > goto ON_EXIT;
> > }
> > CopyMem (Dns6RR[RRCount].RData, AnswerData,
> > Dns6RR[RRCount].DataLength);
> >
> > RRCount ++;
> > + Status = EFI_SUCCESS;
> > } else {
> > //
> > // It's not the GeneralLookUp querying.
> > // Check the Query type, parse the response packet.
> > //
> > @@ -1425,30 +1422,31 @@ ParseDnsResponse (
> > //
> > // Allocate new CacheEntry pool.
> > //
> > Dns4CacheEntry = AllocateZeroPool (sizeof
> (EFI_DNS4_CACHE_ENTRY));
> > if (Dns4CacheEntry == NULL) {
> > - Status = EFI_UNSUPPORTED;
> > + Status = EFI_OUT_OF_RESOURCES;
> > goto ON_EXIT;
> > }
> > Dns4CacheEntry->HostName = AllocateZeroPool (2 *
> > (StrLen(Dns4TokenEntry->QueryHostName) + 1));
> > if (Dns4CacheEntry->HostName == NULL) {
> > - Status = EFI_UNSUPPORTED;
> > + Status = EFI_OUT_OF_RESOURCES;
> > goto ON_EXIT;
> > }
> > CopyMem (Dns4CacheEntry->HostName,
> > Dns4TokenEntry->QueryHostName,
> > 2 * (StrLen(Dns4TokenEntry->QueryHostName) + 1));
> > Dns4CacheEntry->IpAddress = AllocateZeroPool (sizeof
> > (EFI_IPv4_ADDRESS));
> > if (Dns4CacheEntry->IpAddress == NULL) {
> > - Status = EFI_UNSUPPORTED;
> > + Status = EFI_OUT_OF_RESOURCES;
> > goto ON_EXIT;
> > }
> > CopyMem (Dns4CacheEntry->IpAddress, AnswerData, sizeof
> > (EFI_IPv4_ADDRESS));
> > Dns4CacheEntry->Timeout = AnswerSection->Ttl;
> >
> > UpdateDns4Cache (&mDriverData->Dns4CacheList, FALSE, TRUE,
> > *Dns4CacheEntry);
> >
> > - IpCount ++;
> > + IpCount ++;
> > + Status = EFI_SUCCESS;
> > break;
> > case DNS_TYPE_AAAA:
> > //
> > // This is address entry, get Data.
> > //
> > @@ -1476,30 +1474,38 @@ ParseDnsResponse (
> > //
> > // Allocate new CacheEntry pool.
> > //
> > Dns6CacheEntry = AllocateZeroPool (sizeof
> (EFI_DNS6_CACHE_ENTRY));
> > if (Dns6CacheEntry == NULL) {
> > - Status = EFI_UNSUPPORTED;
> > + Status = EFI_OUT_OF_RESOURCES;
> > goto ON_EXIT;
> > }
> > Dns6CacheEntry->HostName = AllocateZeroPool (2 *
> > (StrLen(Dns6TokenEntry->QueryHostName) + 1));
> > if (Dns6CacheEntry->HostName == NULL) {
> > - Status = EFI_UNSUPPORTED;
> > + Status = EFI_OUT_OF_RESOURCES;
> > goto ON_EXIT;
> > }
> > CopyMem (Dns6CacheEntry->HostName,
> > Dns6TokenEntry->QueryHostName,
> > 2 * (StrLen(Dns6TokenEntry->QueryHostName) + 1));
> > Dns6CacheEntry->IpAddress = AllocateZeroPool (sizeof
> > (EFI_IPv6_ADDRESS));
> > if (Dns6CacheEntry->IpAddress == NULL) {
> > - Status = EFI_UNSUPPORTED;
> > + Status = EFI_OUT_OF_RESOURCES;
> > goto ON_EXIT;
> > }
> > CopyMem (Dns6CacheEntry->IpAddress, AnswerData, sizeof
> > (EFI_IPv6_ADDRESS));
> > Dns6CacheEntry->Timeout = AnswerSection->Ttl;
> >
> > UpdateDns6Cache (&mDriverData->Dns6CacheList, FALSE, TRUE,
> > *Dns6CacheEntry);
> >
> > IpCount ++;
> > + Status = EFI_SUCCESS;
> > + break;
> > + case DNS_TYPE_CNAME:
> > + //
> > + // According RFC 1034 - 3.6.2, if the query name is an alias,
> > + the
> > name server will include the CNAME
> > + // record in the response and restart the query at the domain
> > name specified in the data field of the
> > + // CNAME record. So, let's skip this CNAME record parsing and
> > continue the next one.
> > + //
> > break;
> > default:
> > Status = EFI_UNSUPPORTED;
> > goto ON_EXIT;
> > }
> > @@ -1539,24 +1545,28 @@ ParseDnsResponse (
> > }
> > }
> > }
> >
> > //
> > - // Parsing is complete, SignalEvent here.
> > + // Parsing is complete, free the sending packet and signal Event here.
> > //
> > + if (Item != NULL && Item->Value != NULL) {
> > + NetbufFree ((NET_BUF *) (Item->Value)); }
> > +
> > if (Instance->Service->IpVersion == IP_VERSION_4) {
> > ASSERT (Dns4TokenEntry != NULL);
> > Dns4RemoveTokenEntry (&Instance->Dns4TxTokens, Dns4TokenEntry);
> > - Dns4TokenEntry->Token->Status = EFI_SUCCESS;
> > + Dns4TokenEntry->Token->Status = Status;
> > if (Dns4TokenEntry->Token->Event != NULL) {
> > gBS->SignalEvent (Dns4TokenEntry->Token->Event);
> > DispatchDpc ();
> > }
> > } else {
> > ASSERT (Dns6TokenEntry != NULL);
> > Dns6RemoveTokenEntry (&Instance->Dns6TxTokens, Dns6TokenEntry);
> > - Dns6TokenEntry->Token->Status = EFI_SUCCESS;
> > + Dns6TokenEntry->Token->Status = Status;
> > if (Dns6TokenEntry->Token->Event != NULL) {
> > gBS->SignalEvent (Dns6TokenEntry->Token->Event);
> > DispatchDpc ();
> > }
> > }
> > --
> > 1.9.5.msysgit.1
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Patch] NetworkPkg/DnsDxe: Handle CNAME type responded from the name server
2016-09-06 7:05 ` Wu, Jiaxin
@ 2016-09-06 7:13 ` Fu, Siyuan
2016-09-06 7:54 ` Wu, Jiaxin
0 siblings, 1 reply; 8+ messages in thread
From: Fu, Siyuan @ 2016-09-06 7:13 UTC (permalink / raw)
To: Wu, Jiaxin, edk2-devel@lists.01.org; +Cc: Hegde Nagaraj P, Ye, Ting
Jiaxin,
For #2, it's ok, I didn't notice the change in your patch.
For #1, I think we shouldn't mix the CNAME RR and A RR, they are 2 different answers and may have different TTL.
Best Regards
Siyuan
From: Wu, Jiaxin
Sent: Tuesday, September 6, 2016 3:05 PM
To: Fu, Siyuan <siyuan.fu@intel.com>; edk2-devel@lists.01.org
Cc: Hegde Nagaraj P <nagaraj-p.hegde@hpe.com>; Ye, Ting <ting.ye@intel.com>
Subject: RE: [Patch] NetworkPkg/DnsDxe: Handle CNAME type responded from the name server
Hi Siyuan,
> I have 2 comments for the patch.
> 1. I think we can add the CNAME record to the DNS cache, just like the A and
> AAAA records, so the driver won't need to repeat the query if user query the
> host again.
In my opinion , the CNAME is unnecessary to cache to avoid the repeat the query. Because if DnsCache is enabled, HostName and the corresponding IpAddress will be retrieved directly from EFI_DNS4_CACHE_ENTRY.
> 2. We should hold the original query packet until the response is parsed
> successfully, otherwise the system will crash again if a retransmit is needed.
This patch already did the update to avoid the crash issue during retransmit by removing the below code after the parsing is complete.
//
// Parsing is complete, free the sending packet and signal Event here.
//
if (Item != NULL && Item->Value != NULL) {
NetbufFree ((NET_BUF *) (Item->Value));
}
So, the original query packet is safe until the response is parsed.
>
> Best Regards
> Siyuan
>
>
Thanks,
Jiaxin
> > -----Original Message-----
> > From: Wu, Jiaxin
> > Sent: Tuesday, September 6, 2016 11:39 AM
> > To: edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
> > Cc: Hegde Nagaraj P <nagaraj-p.hegde@hpe.com<mailto:nagaraj-p.hegde@hpe.com>>; Fu, Siyuan
> > <siyuan.fu@intel.com<mailto:siyuan.fu@intel.com>>; Ye, Ting <ting.ye@intel.com<mailto:ting.ye@intel.com>>
> > Subject: [Patch] NetworkPkg/DnsDxe: Handle CNAME type responded from
> > the name server
> >
> > According RFC 1034 - 3.6.2, if the query name is an alias, the name
> > server will include the CNAME record in the response and restart the
> > query at the domain name specified in the data field of the CNAME
> > record. RFC also provides one example server action when A query
> > received:
> >
> > Suppose a name server was processing a query with for USCISIC.ARPA,
> > asking for type A information, and had the following resource records:
> > USC-ISIC.ARPA IN CNAME C.ISI.EDU
> > C.ISI.EDU IN A 10.0.0.52
> > Both of these RRs would be returned in the response to the type A query.
> >
> > Currently, DnsDxe driver doesn't handle the CNAME type response, which
> > will cause any exception result. The driver need continue the packet
> > parsing while CNAME type record parsed. So, this patch is used to
> > handle it correctly.
> >
> > Cc: Hegde Nagaraj P <nagaraj-p.hegde@hpe.com<mailto:nagaraj-p.hegde@hpe.com>>
> > Cc: Fu Siyuan <siyuan.fu@intel.com<mailto:siyuan.fu@intel.com>>
> > Cc: Ye Ting <ting.ye@intel.com<mailto:ting.ye@intel.com>>
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Jiaxin Wu <jiaxin.wu@intel.com<mailto:jiaxin.wu@intel.com>>
> > ---
> > NetworkPkg/DnsDxe/DnsImpl.c | 60
> > ++++++++++++++++++++++++++--------------
> > -----
> > 1 file changed, 35 insertions(+), 25 deletions(-)
> >
> > diff --git a/NetworkPkg/DnsDxe/DnsImpl.c
> b/NetworkPkg/DnsDxe/DnsImpl.c
> > index 360f68e..c68ec88 100644
> > --- a/NetworkPkg/DnsDxe/DnsImpl.c
> > +++ b/NetworkPkg/DnsDxe/DnsImpl.c
> > @@ -1231,17 +1231,10 @@ ParseDnsResponse (
> > if (DnsHeader->Flags.Bits.RCode != DNS_FLAGS_RCODE_NO_ERROR ||
> > DnsHeader->AnswersNum < 1 || \
> > DnsHeader->Flags.Bits.QR != DNS_FLAGS_QR_RESPONSE) {
> > Status = EFI_ABORTED;
> > goto ON_EXIT;
> > }
> > -
> > - //
> > - // Free the sending packet.
> > - //
> > - if (Item->Value != NULL) {
> > - NetbufFree ((NET_BUF *) (Item->Value));
> > - }
> >
> > //
> > // Do some buffer allocations.
> > //
> > if (Instance->Service->IpVersion == IP_VERSION_4) { @@ -1288,40
> > +1281,42 @@ ParseDnsResponse (
> > //
> > // It's the GeneralLookUp querying.
> > //
> > Dns6TokenEntry->Token->RspData.GLookupData = AllocatePool
> > (sizeof (DNS_RESOURCE_RECORD));
> > if (Dns6TokenEntry->Token->RspData.GLookupData == NULL) {
> > - Status = EFI_UNSUPPORTED;
> > + Status = EFI_OUT_OF_RESOURCES;
> > goto ON_EXIT;
> > }
> > Dns6TokenEntry->Token->RspData.GLookupData->RRList =
> > AllocatePool (DnsHeader->AnswersNum * sizeof
> (DNS_RESOURCE_RECORD));
> > if (Dns6TokenEntry->Token->RspData.GLookupData->RRList == NULL) {
> > - Status = EFI_UNSUPPORTED;
> > + Status = EFI_OUT_OF_RESOURCES;
> > goto ON_EXIT;
> > }
> > } else {
> > //
> > // It's not the GeneralLookUp querying. Check the Query type.
> > //
> > if (QuerySection->Type == DNS_TYPE_AAAA) {
> > Dns6TokenEntry->Token->RspData.H2AData = AllocatePool (sizeof
> > (DNS6_HOST_TO_ADDR_DATA));
> > if (Dns6TokenEntry->Token->RspData.H2AData == NULL) {
> > - Status = EFI_UNSUPPORTED;
> > + Status = EFI_OUT_OF_RESOURCES;
> > goto ON_EXIT;
> > }
> > Dns6TokenEntry->Token->RspData.H2AData->IpList = AllocatePool
> > (DnsHeader->AnswersNum * sizeof (EFI_IPv6_ADDRESS));
> > if (Dns6TokenEntry->Token->RspData.H2AData->IpList == NULL) {
> > - Status = EFI_UNSUPPORTED;
> > + Status = EFI_OUT_OF_RESOURCES;
> > goto ON_EXIT;
> > }
> > } else {
> > Status = EFI_UNSUPPORTED;
> > goto ON_EXIT;
> > }
> > }
> > }
> >
> > + Status = EFI_NOT_FOUND;
> > +
> > //
> > // Processing AnswerSection.
> > //
> > while (AnswerSectionNum < DnsHeader->AnswersNum) {
> > //
> > @@ -1348,51 +1343,53 @@ ParseDnsResponse (
> > //
> > // Fill the ResourceRecord.
> > //
> > Dns4RR[RRCount].QName = AllocateZeroPool (AsciiStrLen
> > (QueryName) + 1);
> > if (Dns4RR[RRCount].QName == NULL) {
> > - Status = EFI_UNSUPPORTED;
> > + Status = EFI_OUT_OF_RESOURCES;
> > goto ON_EXIT;
> > }
> > CopyMem (Dns4RR[RRCount].QName, QueryName, AsciiStrLen
> (QueryName));
> > Dns4RR[RRCount].QType = AnswerSection->Type;
> > Dns4RR[RRCount].QClass = AnswerSection->Class;
> > Dns4RR[RRCount].TTL = AnswerSection->Ttl;
> > Dns4RR[RRCount].DataLength = AnswerSection->DataLength;
> > Dns4RR[RRCount].RData = AllocateZeroPool
> > (Dns4RR[RRCount].DataLength);
> > if (Dns4RR[RRCount].RData == NULL) {
> > - Status = EFI_UNSUPPORTED;
> > + Status = EFI_OUT_OF_RESOURCES;
> > goto ON_EXIT;
> > }
> > CopyMem (Dns4RR[RRCount].RData, AnswerData,
> > Dns4RR[RRCount].DataLength);
> >
> > RRCount ++;
> > + Status = EFI_SUCCESS;
> > } else if (Instance->Service->IpVersion == IP_VERSION_6 &&
> > Dns6TokenEntry->GeneralLookUp) {
> > Dns6RR = Dns6TokenEntry->Token->RspData.GLookupData->RRList;
> > AnswerData = (UINT8 *) AnswerSection + sizeof (*AnswerSection);
> >
> > //
> > // Fill the ResourceRecord.
> > //
> > Dns6RR[RRCount].QName = AllocateZeroPool (AsciiStrLen
> > (QueryName) + 1);
> > if (Dns6RR[RRCount].QName == NULL) {
> > - Status = EFI_UNSUPPORTED;
> > + Status = EFI_OUT_OF_RESOURCES;
> > goto ON_EXIT;
> > }
> > CopyMem (Dns6RR[RRCount].QName, QueryName, AsciiStrLen
> (QueryName));
> > Dns6RR[RRCount].QType = AnswerSection->Type;
> > Dns6RR[RRCount].QClass = AnswerSection->Class;
> > Dns6RR[RRCount].TTL = AnswerSection->Ttl;
> > Dns6RR[RRCount].DataLength = AnswerSection->DataLength;
> > Dns6RR[RRCount].RData = AllocateZeroPool
> > (Dns6RR[RRCount].DataLength);
> > if (Dns6RR[RRCount].RData == NULL) {
> > - Status = EFI_UNSUPPORTED;
> > + Status = EFI_OUT_OF_RESOURCES;
> > goto ON_EXIT;
> > }
> > CopyMem (Dns6RR[RRCount].RData, AnswerData,
> > Dns6RR[RRCount].DataLength);
> >
> > RRCount ++;
> > + Status = EFI_SUCCESS;
> > } else {
> > //
> > // It's not the GeneralLookUp querying.
> > // Check the Query type, parse the response packet.
> > //
> > @@ -1425,30 +1422,31 @@ ParseDnsResponse (
> > //
> > // Allocate new CacheEntry pool.
> > //
> > Dns4CacheEntry = AllocateZeroPool (sizeof
> (EFI_DNS4_CACHE_ENTRY));
> > if (Dns4CacheEntry == NULL) {
> > - Status = EFI_UNSUPPORTED;
> > + Status = EFI_OUT_OF_RESOURCES;
> > goto ON_EXIT;
> > }
> > Dns4CacheEntry->HostName = AllocateZeroPool (2 *
> > (StrLen(Dns4TokenEntry->QueryHostName) + 1));
> > if (Dns4CacheEntry->HostName == NULL) {
> > - Status = EFI_UNSUPPORTED;
> > + Status = EFI_OUT_OF_RESOURCES;
> > goto ON_EXIT;
> > }
> > CopyMem (Dns4CacheEntry->HostName,
> > Dns4TokenEntry->QueryHostName,
> > 2 * (StrLen(Dns4TokenEntry->QueryHostName) + 1));
> > Dns4CacheEntry->IpAddress = AllocateZeroPool (sizeof
> > (EFI_IPv4_ADDRESS));
> > if (Dns4CacheEntry->IpAddress == NULL) {
> > - Status = EFI_UNSUPPORTED;
> > + Status = EFI_OUT_OF_RESOURCES;
> > goto ON_EXIT;
> > }
> > CopyMem (Dns4CacheEntry->IpAddress, AnswerData, sizeof
> > (EFI_IPv4_ADDRESS));
> > Dns4CacheEntry->Timeout = AnswerSection->Ttl;
> >
> > UpdateDns4Cache (&mDriverData->Dns4CacheList, FALSE, TRUE,
> > *Dns4CacheEntry);
> >
> > - IpCount ++;
> > + IpCount ++;
> > + Status = EFI_SUCCESS;
> > break;
> > case DNS_TYPE_AAAA:
> > //
> > // This is address entry, get Data.
> > //
> > @@ -1476,30 +1474,38 @@ ParseDnsResponse (
> > //
> > // Allocate new CacheEntry pool.
> > //
> > Dns6CacheEntry = AllocateZeroPool (sizeof
> (EFI_DNS6_CACHE_ENTRY));
> > if (Dns6CacheEntry == NULL) {
> > - Status = EFI_UNSUPPORTED;
> > + Status = EFI_OUT_OF_RESOURCES;
> > goto ON_EXIT;
> > }
> > Dns6CacheEntry->HostName = AllocateZeroPool (2 *
> > (StrLen(Dns6TokenEntry->QueryHostName) + 1));
> > if (Dns6CacheEntry->HostName == NULL) {
> > - Status = EFI_UNSUPPORTED;
> > + Status = EFI_OUT_OF_RESOURCES;
> > goto ON_EXIT;
> > }
> > CopyMem (Dns6CacheEntry->HostName,
> > Dns6TokenEntry->QueryHostName,
> > 2 * (StrLen(Dns6TokenEntry->QueryHostName) + 1));
> > Dns6CacheEntry->IpAddress = AllocateZeroPool (sizeof
> > (EFI_IPv6_ADDRESS));
> > if (Dns6CacheEntry->IpAddress == NULL) {
> > - Status = EFI_UNSUPPORTED;
> > + Status = EFI_OUT_OF_RESOURCES;
> > goto ON_EXIT;
> > }
> > CopyMem (Dns6CacheEntry->IpAddress, AnswerData, sizeof
> > (EFI_IPv6_ADDRESS));
> > Dns6CacheEntry->Timeout = AnswerSection->Ttl;
> >
> > UpdateDns6Cache (&mDriverData->Dns6CacheList, FALSE, TRUE,
> > *Dns6CacheEntry);
> >
> > IpCount ++;
> > + Status = EFI_SUCCESS;
> > + break;
> > + case DNS_TYPE_CNAME:
> > + //
> > + // According RFC 1034 - 3.6.2, if the query name is an alias,
> > + the
> > name server will include the CNAME
> > + // record in the response and restart the query at the domain
> > name specified in the data field of the
> > + // CNAME record. So, let's skip this CNAME record parsing and
> > continue the next one.
> > + //
> > break;
> > default:
> > Status = EFI_UNSUPPORTED;
> > goto ON_EXIT;
> > }
> > @@ -1539,24 +1545,28 @@ ParseDnsResponse (
> > }
> > }
> > }
> >
> > //
> > - // Parsing is complete, SignalEvent here.
> > + // Parsing is complete, free the sending packet and signal Event here.
> > //
> > + if (Item != NULL && Item->Value != NULL) {
> > + NetbufFree ((NET_BUF *) (Item->Value)); }
> > +
> > if (Instance->Service->IpVersion == IP_VERSION_4) {
> > ASSERT (Dns4TokenEntry != NULL);
> > Dns4RemoveTokenEntry (&Instance->Dns4TxTokens, Dns4TokenEntry);
> > - Dns4TokenEntry->Token->Status = EFI_SUCCESS;
> > + Dns4TokenEntry->Token->Status = Status;
> > if (Dns4TokenEntry->Token->Event != NULL) {
> > gBS->SignalEvent (Dns4TokenEntry->Token->Event);
> > DispatchDpc ();
> > }
> > } else {
> > ASSERT (Dns6TokenEntry != NULL);
> > Dns6RemoveTokenEntry (&Instance->Dns6TxTokens, Dns6TokenEntry);
> > - Dns6TokenEntry->Token->Status = EFI_SUCCESS;
> > + Dns6TokenEntry->Token->Status = Status;
> > if (Dns6TokenEntry->Token->Event != NULL) {
> > gBS->SignalEvent (Dns6TokenEntry->Token->Event);
> > DispatchDpc ();
> > }
> > }
> > --
> > 1.9.5.msysgit.1
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Patch] NetworkPkg/DnsDxe: Handle CNAME type responded from the name server
2016-09-06 7:13 ` Fu, Siyuan
@ 2016-09-06 7:54 ` Wu, Jiaxin
0 siblings, 0 replies; 8+ messages in thread
From: Wu, Jiaxin @ 2016-09-06 7:54 UTC (permalink / raw)
To: Fu, Siyuan, edk2-devel@lists.01.org; +Cc: Hegde Nagaraj P, Ye, Ting
Yes, take the minimum value of TTL is meaningful.
From: Fu, Siyuan
Sent: Tuesday, September 6, 2016 3:13 PM
To: Wu, Jiaxin <jiaxin.wu@intel.com>; edk2-devel@lists.01.org
Cc: Hegde Nagaraj P <nagaraj-p.hegde@hpe.com>; Ye, Ting <ting.ye@intel.com>
Subject: RE: [Patch] NetworkPkg/DnsDxe: Handle CNAME type responded from the name server
Jiaxin,
For #2, it's ok, I didn't notice the change in your patch.
For #1, I think we shouldn't mix the CNAME RR and A RR, they are 2 different answers and may have different TTL.
Best Regards
Siyuan
From: Wu, Jiaxin
Sent: Tuesday, September 6, 2016 3:05 PM
To: Fu, Siyuan <siyuan.fu@intel.com<mailto:siyuan.fu@intel.com>>; edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
Cc: Hegde Nagaraj P <nagaraj-p.hegde@hpe.com<mailto:nagaraj-p.hegde@hpe.com>>; Ye, Ting <ting.ye@intel.com<mailto:ting.ye@intel.com>>
Subject: RE: [Patch] NetworkPkg/DnsDxe: Handle CNAME type responded from the name server
Hi Siyuan,
> I have 2 comments for the patch.
> 1. I think we can add the CNAME record to the DNS cache, just like the A and
> AAAA records, so the driver won't need to repeat the query if user query the
> host again.
In my opinion , the CNAME is unnecessary to cache to avoid the repeat the query. Because if DnsCache is enabled, HostName and the corresponding IpAddress will be retrieved directly from EFI_DNS4_CACHE_ENTRY.
> 2. We should hold the original query packet until the response is parsed
> successfully, otherwise the system will crash again if a retransmit is needed.
This patch already did the update to avoid the crash issue during retransmit by removing the below code after the parsing is complete.
//
// Parsing is complete, free the sending packet and signal Event here.
//
if (Item != NULL && Item->Value != NULL) {
NetbufFree ((NET_BUF *) (Item->Value));
}
So, the original query packet is safe until the response is parsed.
>
> Best Regards
> Siyuan
>
>
Thanks,
Jiaxin
> > -----Original Message-----
> > From: Wu, Jiaxin
> > Sent: Tuesday, September 6, 2016 11:39 AM
> > To: edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
> > Cc: Hegde Nagaraj P <nagaraj-p.hegde@hpe.com<mailto:nagaraj-p.hegde@hpe.com>>; Fu, Siyuan
> > <siyuan.fu@intel.com<mailto:siyuan.fu@intel.com>>; Ye, Ting <ting.ye@intel.com<mailto:ting.ye@intel.com>>
> > Subject: [Patch] NetworkPkg/DnsDxe: Handle CNAME type responded from
> > the name server
> >
> > According RFC 1034 - 3.6.2, if the query name is an alias, the name
> > server will include the CNAME record in the response and restart the
> > query at the domain name specified in the data field of the CNAME
> > record. RFC also provides one example server action when A query
> > received:
> >
> > Suppose a name server was processing a query with for USCISIC.ARPA,
> > asking for type A information, and had the following resource records:
> > USC-ISIC.ARPA IN CNAME C.ISI.EDU
> > C.ISI.EDU IN A 10.0.0.52
> > Both of these RRs would be returned in the response to the type A query.
> >
> > Currently, DnsDxe driver doesn't handle the CNAME type response, which
> > will cause any exception result. The driver need continue the packet
> > parsing while CNAME type record parsed. So, this patch is used to
> > handle it correctly.
> >
> > Cc: Hegde Nagaraj P <nagaraj-p.hegde@hpe.com<mailto:nagaraj-p.hegde@hpe.com>>
> > Cc: Fu Siyuan <siyuan.fu@intel.com<mailto:siyuan.fu@intel.com>>
> > Cc: Ye Ting <ting.ye@intel.com<mailto:ting.ye@intel.com>>
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Jiaxin Wu <jiaxin.wu@intel.com<mailto:jiaxin.wu@intel.com>>
> > ---
> > NetworkPkg/DnsDxe/DnsImpl.c | 60
> > ++++++++++++++++++++++++++--------------
> > -----
> > 1 file changed, 35 insertions(+), 25 deletions(-)
> >
> > diff --git a/NetworkPkg/DnsDxe/DnsImpl.c
> b/NetworkPkg/DnsDxe/DnsImpl.c
> > index 360f68e..c68ec88 100644
> > --- a/NetworkPkg/DnsDxe/DnsImpl.c
> > +++ b/NetworkPkg/DnsDxe/DnsImpl.c
> > @@ -1231,17 +1231,10 @@ ParseDnsResponse (
> > if (DnsHeader->Flags.Bits.RCode != DNS_FLAGS_RCODE_NO_ERROR ||
> > DnsHeader->AnswersNum < 1 || \
> > DnsHeader->Flags.Bits.QR != DNS_FLAGS_QR_RESPONSE) {
> > Status = EFI_ABORTED;
> > goto ON_EXIT;
> > }
> > -
> > - //
> > - // Free the sending packet.
> > - //
> > - if (Item->Value != NULL) {
> > - NetbufFree ((NET_BUF *) (Item->Value));
> > - }
> >
> > //
> > // Do some buffer allocations.
> > //
> > if (Instance->Service->IpVersion == IP_VERSION_4) { @@ -1288,40
> > +1281,42 @@ ParseDnsResponse (
> > //
> > // It's the GeneralLookUp querying.
> > //
> > Dns6TokenEntry->Token->RspData.GLookupData = AllocatePool
> > (sizeof (DNS_RESOURCE_RECORD));
> > if (Dns6TokenEntry->Token->RspData.GLookupData == NULL) {
> > - Status = EFI_UNSUPPORTED;
> > + Status = EFI_OUT_OF_RESOURCES;
> > goto ON_EXIT;
> > }
> > Dns6TokenEntry->Token->RspData.GLookupData->RRList =
> > AllocatePool (DnsHeader->AnswersNum * sizeof
> (DNS_RESOURCE_RECORD));
> > if (Dns6TokenEntry->Token->RspData.GLookupData->RRList == NULL) {
> > - Status = EFI_UNSUPPORTED;
> > + Status = EFI_OUT_OF_RESOURCES;
> > goto ON_EXIT;
> > }
> > } else {
> > //
> > // It's not the GeneralLookUp querying. Check the Query type.
> > //
> > if (QuerySection->Type == DNS_TYPE_AAAA) {
> > Dns6TokenEntry->Token->RspData.H2AData = AllocatePool (sizeof
> > (DNS6_HOST_TO_ADDR_DATA));
> > if (Dns6TokenEntry->Token->RspData.H2AData == NULL) {
> > - Status = EFI_UNSUPPORTED;
> > + Status = EFI_OUT_OF_RESOURCES;
> > goto ON_EXIT;
> > }
> > Dns6TokenEntry->Token->RspData.H2AData->IpList = AllocatePool
> > (DnsHeader->AnswersNum * sizeof (EFI_IPv6_ADDRESS));
> > if (Dns6TokenEntry->Token->RspData.H2AData->IpList == NULL) {
> > - Status = EFI_UNSUPPORTED;
> > + Status = EFI_OUT_OF_RESOURCES;
> > goto ON_EXIT;
> > }
> > } else {
> > Status = EFI_UNSUPPORTED;
> > goto ON_EXIT;
> > }
> > }
> > }
> >
> > + Status = EFI_NOT_FOUND;
> > +
> > //
> > // Processing AnswerSection.
> > //
> > while (AnswerSectionNum < DnsHeader->AnswersNum) {
> > //
> > @@ -1348,51 +1343,53 @@ ParseDnsResponse (
> > //
> > // Fill the ResourceRecord.
> > //
> > Dns4RR[RRCount].QName = AllocateZeroPool (AsciiStrLen
> > (QueryName) + 1);
> > if (Dns4RR[RRCount].QName == NULL) {
> > - Status = EFI_UNSUPPORTED;
> > + Status = EFI_OUT_OF_RESOURCES;
> > goto ON_EXIT;
> > }
> > CopyMem (Dns4RR[RRCount].QName, QueryName, AsciiStrLen
> (QueryName));
> > Dns4RR[RRCount].QType = AnswerSection->Type;
> > Dns4RR[RRCount].QClass = AnswerSection->Class;
> > Dns4RR[RRCount].TTL = AnswerSection->Ttl;
> > Dns4RR[RRCount].DataLength = AnswerSection->DataLength;
> > Dns4RR[RRCount].RData = AllocateZeroPool
> > (Dns4RR[RRCount].DataLength);
> > if (Dns4RR[RRCount].RData == NULL) {
> > - Status = EFI_UNSUPPORTED;
> > + Status = EFI_OUT_OF_RESOURCES;
> > goto ON_EXIT;
> > }
> > CopyMem (Dns4RR[RRCount].RData, AnswerData,
> > Dns4RR[RRCount].DataLength);
> >
> > RRCount ++;
> > + Status = EFI_SUCCESS;
> > } else if (Instance->Service->IpVersion == IP_VERSION_6 &&
> > Dns6TokenEntry->GeneralLookUp) {
> > Dns6RR = Dns6TokenEntry->Token->RspData.GLookupData->RRList;
> > AnswerData = (UINT8 *) AnswerSection + sizeof (*AnswerSection);
> >
> > //
> > // Fill the ResourceRecord.
> > //
> > Dns6RR[RRCount].QName = AllocateZeroPool (AsciiStrLen
> > (QueryName) + 1);
> > if (Dns6RR[RRCount].QName == NULL) {
> > - Status = EFI_UNSUPPORTED;
> > + Status = EFI_OUT_OF_RESOURCES;
> > goto ON_EXIT;
> > }
> > CopyMem (Dns6RR[RRCount].QName, QueryName, AsciiStrLen
> (QueryName));
> > Dns6RR[RRCount].QType = AnswerSection->Type;
> > Dns6RR[RRCount].QClass = AnswerSection->Class;
> > Dns6RR[RRCount].TTL = AnswerSection->Ttl;
> > Dns6RR[RRCount].DataLength = AnswerSection->DataLength;
> > Dns6RR[RRCount].RData = AllocateZeroPool
> > (Dns6RR[RRCount].DataLength);
> > if (Dns6RR[RRCount].RData == NULL) {
> > - Status = EFI_UNSUPPORTED;
> > + Status = EFI_OUT_OF_RESOURCES;
> > goto ON_EXIT;
> > }
> > CopyMem (Dns6RR[RRCount].RData, AnswerData,
> > Dns6RR[RRCount].DataLength);
> >
> > RRCount ++;
> > + Status = EFI_SUCCESS;
> > } else {
> > //
> > // It's not the GeneralLookUp querying.
> > // Check the Query type, parse the response packet.
> > //
> > @@ -1425,30 +1422,31 @@ ParseDnsResponse (
> > //
> > // Allocate new CacheEntry pool.
> > //
> > Dns4CacheEntry = AllocateZeroPool (sizeof
> (EFI_DNS4_CACHE_ENTRY));
> > if (Dns4CacheEntry == NULL) {
> > - Status = EFI_UNSUPPORTED;
> > + Status = EFI_OUT_OF_RESOURCES;
> > goto ON_EXIT;
> > }
> > Dns4CacheEntry->HostName = AllocateZeroPool (2 *
> > (StrLen(Dns4TokenEntry->QueryHostName) + 1));
> > if (Dns4CacheEntry->HostName == NULL) {
> > - Status = EFI_UNSUPPORTED;
> > + Status = EFI_OUT_OF_RESOURCES;
> > goto ON_EXIT;
> > }
> > CopyMem (Dns4CacheEntry->HostName,
> > Dns4TokenEntry->QueryHostName,
> > 2 * (StrLen(Dns4TokenEntry->QueryHostName) + 1));
> > Dns4CacheEntry->IpAddress = AllocateZeroPool (sizeof
> > (EFI_IPv4_ADDRESS));
> > if (Dns4CacheEntry->IpAddress == NULL) {
> > - Status = EFI_UNSUPPORTED;
> > + Status = EFI_OUT_OF_RESOURCES;
> > goto ON_EXIT;
> > }
> > CopyMem (Dns4CacheEntry->IpAddress, AnswerData, sizeof
> > (EFI_IPv4_ADDRESS));
> > Dns4CacheEntry->Timeout = AnswerSection->Ttl;
> >
> > UpdateDns4Cache (&mDriverData->Dns4CacheList, FALSE, TRUE,
> > *Dns4CacheEntry);
> >
> > - IpCount ++;
> > + IpCount ++;
> > + Status = EFI_SUCCESS;
> > break;
> > case DNS_TYPE_AAAA:
> > //
> > // This is address entry, get Data.
> > //
> > @@ -1476,30 +1474,38 @@ ParseDnsResponse (
> > //
> > // Allocate new CacheEntry pool.
> > //
> > Dns6CacheEntry = AllocateZeroPool (sizeof
> (EFI_DNS6_CACHE_ENTRY));
> > if (Dns6CacheEntry == NULL) {
> > - Status = EFI_UNSUPPORTED;
> > + Status = EFI_OUT_OF_RESOURCES;
> > goto ON_EXIT;
> > }
> > Dns6CacheEntry->HostName = AllocateZeroPool (2 *
> > (StrLen(Dns6TokenEntry->QueryHostName) + 1));
> > if (Dns6CacheEntry->HostName == NULL) {
> > - Status = EFI_UNSUPPORTED;
> > + Status = EFI_OUT_OF_RESOURCES;
> > goto ON_EXIT;
> > }
> > CopyMem (Dns6CacheEntry->HostName,
> > Dns6TokenEntry->QueryHostName,
> > 2 * (StrLen(Dns6TokenEntry->QueryHostName) + 1));
> > Dns6CacheEntry->IpAddress = AllocateZeroPool (sizeof
> > (EFI_IPv6_ADDRESS));
> > if (Dns6CacheEntry->IpAddress == NULL) {
> > - Status = EFI_UNSUPPORTED;
> > + Status = EFI_OUT_OF_RESOURCES;
> > goto ON_EXIT;
> > }
> > CopyMem (Dns6CacheEntry->IpAddress, AnswerData, sizeof
> > (EFI_IPv6_ADDRESS));
> > Dns6CacheEntry->Timeout = AnswerSection->Ttl;
> >
> > UpdateDns6Cache (&mDriverData->Dns6CacheList, FALSE, TRUE,
> > *Dns6CacheEntry);
> >
> > IpCount ++;
> > + Status = EFI_SUCCESS;
> > + break;
> > + case DNS_TYPE_CNAME:
> > + //
> > + // According RFC 1034 - 3.6.2, if the query name is an alias,
> > + the
> > name server will include the CNAME
> > + // record in the response and restart the query at the domain
> > name specified in the data field of the
> > + // CNAME record. So, let's skip this CNAME record parsing and
> > continue the next one.
> > + //
> > break;
> > default:
> > Status = EFI_UNSUPPORTED;
> > goto ON_EXIT;
> > }
> > @@ -1539,24 +1545,28 @@ ParseDnsResponse (
> > }
> > }
> > }
> >
> > //
> > - // Parsing is complete, SignalEvent here.
> > + // Parsing is complete, free the sending packet and signal Event here.
> > //
> > + if (Item != NULL && Item->Value != NULL) {
> > + NetbufFree ((NET_BUF *) (Item->Value)); }
> > +
> > if (Instance->Service->IpVersion == IP_VERSION_4) {
> > ASSERT (Dns4TokenEntry != NULL);
> > Dns4RemoveTokenEntry (&Instance->Dns4TxTokens, Dns4TokenEntry);
> > - Dns4TokenEntry->Token->Status = EFI_SUCCESS;
> > + Dns4TokenEntry->Token->Status = Status;
> > if (Dns4TokenEntry->Token->Event != NULL) {
> > gBS->SignalEvent (Dns4TokenEntry->Token->Event);
> > DispatchDpc ();
> > }
> > } else {
> > ASSERT (Dns6TokenEntry != NULL);
> > Dns6RemoveTokenEntry (&Instance->Dns6TxTokens, Dns6TokenEntry);
> > - Dns6TokenEntry->Token->Status = EFI_SUCCESS;
> > + Dns6TokenEntry->Token->Status = Status;
> > if (Dns6TokenEntry->Token->Event != NULL) {
> > gBS->SignalEvent (Dns6TokenEntry->Token->Event);
> > DispatchDpc ();
> > }
> > }
> > --
> > 1.9.5.msysgit.1
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2016-09-06 7:54 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-09-06 3:39 [Patch] NetworkPkg/IpSecDxe: Generate SPI randomly and correct IKE_SPI_BASE value Jiaxin Wu
2016-09-06 3:39 ` [Patch] NetworkPkg/DnsDxe: Handle CNAME type responded from the name server Jiaxin Wu
2016-09-06 3:55 ` Fu, Siyuan
2016-09-06 7:05 ` Wu, Jiaxin
2016-09-06 7:13 ` Fu, Siyuan
2016-09-06 7:54 ` Wu, Jiaxin
2016-09-06 3:55 ` [Patch] NetworkPkg/IpSecDxe: Generate SPI randomly and correct IKE_SPI_BASE value Fu, Siyuan
2016-09-06 6:37 ` Ye, Ting
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox